On Mon, 30 Jan 2023 14:08:49 GMT, Daniel Jeliński <[email protected]> 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
