[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8234935: JdwpListenTest.java and JdwpAttachTest.java getting bind failures on Windows 2
From:       Alex Menkov <alexey.menkov () oracle ! com>
Date:       2020-02-10 21:49:20
Message-ID: 7d56589c-76b8-c17e-f1e6-1073a15b57d1 () oracle ! com
[Download RAW message or body]

Thanks, will do

--alex

On 02/10/2020 11:43, Chris Plummer wrote:
> Hi Alex,
> 
> The changes look good. Please up the copyright in JdwpListenTest.java.
> 
> thanks,
> 
> Chris
> 
> On 2/7/20 2:06 PM, Alex Menkov wrote:
>> 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
>>>>>
>>>>>
>>>
>>>
> 
> 
[prev in list] [next in list] [prev in thread] [next in thread]