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

[Chris Plummer]

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