Hi Xuelei, Bernd, On 01/07/19 11:31 PM, Xuelei Fan wrote: > On 7/1/2019 10:51 AM, Bernd Eckenfels wrote: >> Also the `is*` prefix would point to a boolean, that’s maybe a >> cleaner data type than a case sensitive string? >> > I agreed. The "isTimedout" is also used to for strings other than > "YES"/"No". I don't think it is a good code convention. > isTimedout = "Invalidated before timeout"; > > As Jaikiran had taken care of the issue in a previous thread: > > https://mail.openjdk.java.net/pipermail/security-dev/2019-June/020307.html > > > I will leave it to Jaikiran if we want to make more improvement in > that code review thread.
I can take up the cleanup of this testcase as a separate task. I will need to understand the current semantics[1] of some of the APIs being used in this test and how it impacts this test. [1] http://mail.openjdk.java.net/pipermail/security-dev/2019-July/020308.html -Jaikiran > > Thanks, > Xuelei > >> >> -- >> http://bernd.eckenfels.net >> ------------------------------------------------------------------------ >> *Von:* security-dev <security-dev-boun...@openjdk.java.net> im >> Auftrag von Xuelei Fan <xuelei....@oracle.com> >> *Gesendet:* Montag, Juli 1, 2019 6:44 PM >> *An:* security-dev@openjdk.java.net >> *Betreff:* Request for Review [14] JDK-8226976, SessionTimeOutTests >> uses == operator for String value check >> Hi, >> >> In the following test case, "==" is used to compare two strings. As is >> not a comment coding convention. I would like to use "equals()" method >> instead. >> >> Thanks, >> Xuelei >> >> >> $ hg diff test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java >> diff -r 73f1c84ca264 >> test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java >> --- a/test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java >> Thu Jun 27 22:03:19 2019 +0200 >> +++ b/test/jdk/javax/net/ssl/SSLSession/SessionTimeOutTests.java >> Mon Jul 01 09:29:23 2019 -0700 >> @@ -283,7 +283,7 @@ >> } >> System.out.print(sess + " " + lifetime); >> if (((timeout == 0) || (lifetime < timeout)) && >> - (isTimedout == "YES")) { >> + isTimedout.equals("YES")) { >> isTimedout = "Invalidated before timeout"; >> }
