RFR: JDK-8234935: JdwpListenTest.java and JdwpAttachTest.java getting bind failures on Windows 2016 hosts

[Alex Menkov]

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
> 
>