Re: RFR: 8252117: com/sun/jdi/BadHandshakeTest.java failed with "ConnectException: Connection refused: connect" [v2]

2020-10-23 Thread Alex Menkov
> Please review the fix for BadHandshakeTest.
> Summary:
> The test verifies that bad (testcase1) or incomplete (testcase2) handshake 
> does not cause debuggee termination.
> To check this it tries to attach to the debuggee again (connect in testcase2 
> is also verification for testcase1)
> 
> The fix adds reply logic to the last attach (similar to retry logic in 
> testcase2)

Alex Menkov has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix the comment

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/789/files
  - new: https://git.openjdk.java.net/jdk/pull/789/files/26c5e4be..b16810b6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=789=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=789=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/789.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/789/head:pull/789

PR: https://git.openjdk.java.net/jdk/pull/789


Re: RFR: 8252117: com/sun/jdi/BadHandshakeTest.java failed with "ConnectException: Connection refused: connect"

2020-10-23 Thread Chris Plummer
On Thu, 22 Oct 2020 00:00:17 GMT, Alex Menkov  wrote:

> Please review the fix for BadHandshakeTest.
> Summary:
> The test verifies that bad (testcase1) or incomplete (testcase2) handshake 
> does not cause debuggee termination.
> To check this it tries to attach to the debuggee again (connect in testcase2 
> is also verification for testcase1)
> 
> The fix adds reply logic to the last attach (similar to retry logic in 
> testcase2)

Other than the one comment I noted, the changes look good. Was the failure rate 
high enough that you've been able to prove your fix works?

test/jdk/com/sun/jdi/BadHandshakeTest.java line 159:

> 157: }
> 158: 
> 159: // get the debuggee some time to exit before forcibly 
> terminate it

"give", not "get". Also, "terminating", not "terminate".

-

Marked as reviewed by cjplummer (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/789


Re: RFR: 8253939: [TESTBUG] Increase coverage of the cgroups detection code [v3]

2020-10-23 Thread Severin Gehwolf
> Test only change. With 
> [JDK-8253435](https://bugs.openjdk.java.net/browse/JDK-8253435) a test has 
> been added on the hotspot side, but nothing for the Java Metrics code. Same 
> for [JDK-8252359](https://bugs.openjdk.java.net/browse/JDK-8252359).
> 
> When JDK-8217766 got fixed cgroup factories with the detection logic didn't 
> exist so were harder to test. This patch adds tests for them too.
> 
> Thoughts?

Severin Gehwolf has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  8253939: [TESTBUG] Increase coverage of the cgroups detection code

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/609/files
  - new: https://git.openjdk.java.net/jdk/pull/609/files/a122958f..44362211

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=609=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=609=01-02

  Stats: 107 lines in 2 files changed: 107 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/609/head:pull/609

PR: https://git.openjdk.java.net/jdk/pull/609


Re: RFR: 8253939: [TESTBUG] Increase coverage of the cgroups detection code for Java [v2]

2020-10-23 Thread Severin Gehwolf
> Test only change. With 
> [JDK-8253435](https://bugs.openjdk.java.net/browse/JDK-8253435) a test has 
> been added on the hotspot side, but nothing for the Java Metrics code. Same 
> for [JDK-8252359](https://bugs.openjdk.java.net/browse/JDK-8252359). This 
> patch alleviates that. 
> 
> Thoughts?

Severin Gehwolf has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  8253939: [TESTBUG] Increase coverage of the cgroups detection code for Java

-

Changes: https://git.openjdk.java.net/jdk/pull/609/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=609=01
  Stats: 40 lines in 1 file changed: 27 ins; 0 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/609/head:pull/609

PR: https://git.openjdk.java.net/jdk/pull/609


Re: RFR: 8255072: [TESTBUG] com/sun/jdi/EATests.java should not fail if expected VMOutOfMemoryException is not thrown [v3]

2020-10-23 Thread Richard Reingruber
On Fri, 23 Oct 2020 08:15:34 GMT, Chris Plummer  wrote:

>>> 
>>> 
>>> Looks good.
>> 
>> Thank you. I'll wait for a second review assuming it's required.
>
>> Thank you. I'll wait for a second review assuming it's required.
> 
> You might want to add the compiler and/or gc teams to the review

Following @plummercj advice to add compiler/gc teams.

-

PR: https://git.openjdk.java.net/jdk/pull/775


Re: RFR: 8255072: [TESTBUG] com/sun/jdi/EATests.java should not fail if expected VMOutOfMemoryException is not thrown [v3]

2020-10-23 Thread Chris Plummer
On Fri, 23 Oct 2020 08:00:49 GMT, Richard Reingruber  wrote:

> Thank you. I'll wait for a second review assuming it's required.

You might want to add the compiler and/or gc teams to the review

-

PR: https://git.openjdk.java.net/jdk/pull/775


Re: RFR: 8255072: [TESTBUG] com/sun/jdi/EATests.java should not fail if expected VMOutOfMemoryException is not thrown [v3]

2020-10-23 Thread Richard Reingruber
On Thu, 22 Oct 2020 23:06:10 GMT, Chris Plummer  wrote:

> 
> 
> Looks good.

Thank you. I'll wait for a second review assuming it's required.

-

PR: https://git.openjdk.java.net/jdk/pull/775