fix(node): increase timeout in TlsCertificateTest to prevent flaky fa… #5064
Conversation
…ilures This change increases the timeout from 50s to 120s in TlsCertificateTest.test.ts. This prevents 'Exceeded timeout of 50000 ms for a hook' errors during cluster creation on slower environments, addressing Issue valkey-io#4989. Signed-off-by: hank95179 <[email protected]>
|
120,000 ms maybe a little excessive. Did you try any other values before this one? Can you post the script you used for verification? |
| } from "./TestUtilities"; | ||
|
|
||
| const TIMEOUT = 50000; | ||
| const TIMEOUT = 120000; |
There was a problem hiding this comment.
Is this timeout limit too much? It is 120 seconds.
| } from "./TestUtilities"; | ||
|
|
||
| const TIMEOUT = 50000; | ||
| const TIMEOUT = 120000; |
There was a problem hiding this comment.
This TIMEOUT value is used across all tests in this class.
The issue occurs only in the BeforeAll action, where the Valkey servers take longer to be created. Increasing the timeout from 50000 to 120000 is excessive and unnecessary for the test cases themselves.
Please introduce a separate variable with an extended timeout value specifically for the BeforeAll action.
This PR should be of use: #4701
As this PR has been open with feedback for some time without any response from @hank95179, I will take over the PR if no action is taken within the next couple of days.
3 participants
Problem:
The
TlsCertificateTest.test.tssuite was experiencing timeouts during the cluster creation phase in thebeforeAllhook. The original timeout was set to 50 seconds (TIMEOUT = 50000).slower CI environments or under load, the process of starting up the standalone and cluster-mode Valkey instances with TLS certificates could exceed this limit, leading to
Exceeded timeout of 50000 ms for a hookerrors and test failures.Solution:
The timeout constant
TIMEOUTinnode/tests/TlsCertificateTest.test.tshas been increased from 50,000ms (50 seconds) to 120,000ms (2 minutes). This provides ample time for the clusterinitialization process to complete, significantly reducing the likelihood of timeouts and improving the test suite's stability.
Verification:
This fix was verified by running a reproduction script that looped the test execution. With the increased timeout, the test suite consistently passed over hundreds of iterations, whereas it
would previously fail sporadically due to timeouts.
Issue link
This Pull Request is linked to issue (URL): #4989