Thanks, will make all values 0x.. before push
--alex
On 02/10/2020 12:56, serguei.spit...@oracle.com wrote:
Hi Alex,
It looks okay to me.
Minor:
+ return bytes[0] == 0x20 && bytes[1] == 0x01 && bytes[2] == 00 &&
bytes[3] == 0; '00' looks strange, maybe you want something like this: +
return bytes[0] == 0x20 && bytes[1] == 0x01 && bytes[2] == 0x0 &&
bytes[3] == 0x0;
Thanks,
Serguei
On 2/7/20 14:06, 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
