On Wed, 20 Sep 2023 16:28:00 GMT, Liam Miller-Cushon <cus...@openjdk.org> wrote:
>> Please consider this fix for >> [JDK-8313804](https://bugs.openjdk.org/browse/JDK-8313804), which adds >> support to JDWP for `-Djava.net.preferIPv6Addresses=system`. Previously it >> only handled `-Djava.net.preferIPv6Addresses=true` and >> `-Djava.net.preferIPv6Addresses=false`. > > Liam Miller-Cushon has updated the pull request incrementally with one > additional commit since the last revision: > > Update workaround for JDK-8250630 to check if preferIPv4Stack is false It seems like the "IPv6-only system" part of the test should include more test cases, both passing and failing. I'm not sure why it was lmitted to just the one: // IPv6-only system - expected to fail on IPv4 address new ListenTest("localhost", ipv6Address) .preferIPv4Stack(true) .run(TestResult.ListenFailed); test/jdk/com/sun/jdi/JdwpNetProps.java line 73: > 71: .preferIPv4Stack(true) > 72: .preferIPv6Addresses("system") > 73: .run(TestResult.Success); Does this test guarantee that "system" was honored, or only verify that there was no failure to attach? In other words, wouldn't this pass even if the debug agent ignored the use of "system"? What happens if using "system" with `preferIPv4Stack(false)`? I'm not sure why you don't have a test case for this. I guess maybe the reason is we don't know if the test case would pass or not because we don't know if "system" will result in using v4 (pass) or v6 (fail). Is there a way to determine that in advanced? test/jdk/com/sun/jdi/JdwpNetProps.java line 82: > 80: // - listen on IPv4 > 81: new ListenTest("localhost", ipv6Address) > 82: .preferIPv6Addresses("false") It seems we should also have a test case that omits using `preferIPv6Addresses()`. I think like this test case, it should fail because the debug agent will default to v4. ------------- PR Review: https://git.openjdk.org/jdk/pull/15796#pullrequestreview-1636443176 PR Review Comment: https://git.openjdk.org/jdk/pull/15796#discussion_r1332140962 PR Review Comment: https://git.openjdk.org/jdk/pull/15796#discussion_r1332142864
