Updated webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/JdwpTestsTeredo/webrev.02/
I decided to go 2nd way.
--alex
On 02/06/2020 17:31, Chris Plummer wrote:
Either is fine by me.
Chris
On 2/6/20 3:40 PM, Alex Menkov wrote:
Hi Chris,
Thank you for the review.
So we have 2 ways - create new RFE for refactoring and then fix this
bug in updated code.
or just fix this 2 tests without refactoring (the changes in the tests
will be identical).
Do you think it makes sense to go #1 or just do #2?
Regarding using Platform.isWindows - it's good for the case, I'll fix
it in the next iteration.
--alex
On 02/06/2020 15:01, Chris Plummer wrote:
Hi Alex,
When refactoring is big and the bug fix is small, I prefer to see the
refactoring done first. It just keeps things cleaner and makes it
easier for the reviewer to see the important changes. It also helps
anyone looking at this bug or these tests in the future to better
recognize what the actual bug fix was, and what was just refactoring.
Think if there was another test with this issue, and someone was
looking at the diff of this fix to see how to apply it to the other
test.
BTW, there is already a Platform.isWindows() API. It should probably
be used rather than the check the test is using. It is a slightly
different test however, testing for a prefix of "win" rather than
"windows" anywhere in the string.
thanks,
Chris
On 2/6/20 1:14 PM, Alex Menkov wrote:
Hi all,
Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8234935
webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/JdwpTestsTeredo/webrev/
The failures are caused by Teredo clients
(https://en.wikipedia.org/wiki/Teredo_tunneling).
The fix filters out corresponding addresses.
JdwpListenTest and JdwpAttachTest use the same way to get addresses
for testing. As this is not the 1st time the algorithm is updated I
decided to deduplicate the code and move shared code to new base class.
So actual change is the addition of
71 // Teredo clients cause intermittent errors on listen ("bind
failed")
72 // and attach ("no route to host").
73 // Teredo is supposed to be a temporary measure, but some test
machines have it.
74 if (isTeredo(addr6)) {
75 continue;
76 }
and isTeredo method implementation.
--alex
