RFR: 8301214: Adjust handshakeTimeout value in test HandshakeTimeout.java after 8189338

[Vyom Tewari]

On Mon, 30 Jan 2023 14:08:49 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> test/jdk/java/rmi/transport/handshakeTimeout/HandshakeTimeout.java line 59:
>> 
>>> 57:     public static void main(String[] args) throws Exception {
>>> 58: 
>>> 59:         System.setProperty("sun.rmi.transport.tcp.handshakeTimeout", "1");
>> 
>> I can see that this test uses "TIMEOUT" down in test 
>> /*
>>          * Wait for call attempt to finished, and analyze result.
>>          */
>>         t.join(TIMEOUT);
>> 
>> i will suggest you to reduce the "TIMEOUT"  constant instead of hard coding it to "1" second.
>
>> I can see that this test uses "TIMEOUT" down in test
> 
> Right, this is why I didn't remove the TIMEOUT constant
>> i will suggest you to reduce the "TIMEOUT" constant instead of hard coding it to "1" second.
> 
> I can't; this would make the test fail more often. I could introduce a separate timeout constant for `handshakeTimeout` to make it clear that the two are not related.
> 
> Note that this test uses 2 timeouts:
> - the `handshakeTimeout`, which is the time limit for socket operations; when this amount of time passes waiting for a response from the server, an exception is thrown
> - the `TIMEOUT`, which is the amount of time after which we expect to get the exception. This includes the `handshakeTimeout`, but also includes all other operations performed during the handshake, like class loading, initialization, TLS handshake etc.
> 
> Alternatively I could change the test descriptor to:
> 
>  * @run main/othervm/timeout=10 -Dsun.rmi.transport.tcp.handshakeTimeout=1 HandshakeTimeout
> 
> and drop both constants from the code.
> What do you think?

setting System property(sun.rmi.transport.tcp.handshakeTimeout) as argument is good idea and this will increase the code readability as well.

-------------

PR: https://git.openjdk.org/jdk/pull/12292