Hi Xuelei,
Thank you for review. Please see inline.
On 06/21/2016 05:57 PM, Xuelei Fan wrote:
Looks more like a issue of "Unsupported or unrecognized SSL message".
This issue might be impacted by other test cases that running in the
same time that using free socket port. It's good to dump the SSL/TLS
record for further evaluation.
Minor comments:
test/javax/net/ssl/TLS/TestJSSE.java
36 System.setProperty("javax.net.debug", "ssl");
What do you think if dump the TLS record, too?
System.setProperty("javax.net.debug", "ssl,record");
Sure, "ssl,record" is much better. I checked that it doesn't result to
"overflow output".
test/javax/net/ssl/TLS/CipherTestUtils.java
line 431-433: Looks like you missed the print the string array.
Sure, thank you!
line 379, 498, 509: may exceed 80 characters each line. Please also
look at other updated that may exceed 80 characters each line.
Right, a couple of lines are about 81-85 characters. I am not a fan of
long lines, but I think they may exceed 80 characters if it makes them
more readable. I know that some people use vi, so in some environment
they may have problems with lines more than 80 characters.
If I recall correctly, there was a discussion about new Java coding
guidelines, but I am not sure it has been done. I know about the
following draft, but not sure if it is official
http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html
It has a recommendation about line lengths, but it allows lines more
than 80 characters if it makes it more readable.
Anyway, I avoided lines more than 80 characters (mostly I renamed
variables).
Please take a look at updated webrev:
http://cr.openjdk.java.net/~asmotrak/8152745/webrev.01/
Artem
On 6/22/2016 1:20 AM, Artem Smotrakov wrote:
Hello,
Please review the patch below for javax/net/ssl/TLS/TestJSSE.java test.
The test has been observed to fail intermittently with "Unsupported or
unrecognized SSL" error. But I couldn't reproduce it manually while
running the test in a loop for a couple of days on Linux x64. For now
it's not clear why it failed.
The patch updates the test with the following:
- enabled debug output, so that we can have more info if the test fails
again
- splitted the test to several files to avoid jtreg's "output overflow"
Hm, a sort of hack code as these files are not actually 'java' code.
But it should work, I think.
Thanks,
Xuelei
- updated the test to close sockets
- some refactoring to make the code/logs more readable (although I
believe the test needs even more refactoring)
Webrev: http://cr.openjdk.java.net/~asmotrak/8152745/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8152745
Artem
