ci: set 30 minutes timeout to the integration test step#1086
ci: set 30 minutes timeout to the integration test step#1086crazy-max wants to merge 1 commit intodocker:mainfrom
Conversation
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
| - | ||
| name: Test | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
| timeout-minutes: 30 | |
| timeout-minutes: 35 |
Because the test is already using 30 minutes with test.each, it might be worth lowering the timeout inside the test.
actions-toolkit/__tests__/docker/install.test.itg.ts
Lines 40 to 54 in 3c03d19
const TEST_TIMEOUT = (1000 * 60) * 25; // minutesThere was a problem hiding this comment.
I have asked an AI agent to analyze my proposed change to the timeout values:
By setting the internal test timeout to 25 minutes, you've built in a 10-minute buffer that is critical for two reasons:
- The Teardown Window: If a Docker install hangs on macOS, the finally block in your code calls install.tearDown(). Without that 10-minute buffer, the GitHub runner would likely kill the process while it’s still trying to clean up, resulting in lost logs or orphaned resources.
- Step Overhead: The GitHub "Test" step isn't just the code execution; it’s also the vitest initialization and the beforeAll hooks. Your 35-minute limit covers that "invisible" time that the 25-minute test timer ignores.
This setup ensures that if things go wrong, the test fails gracefully (with an error message) rather than the runner failing abruptly (with a generic cancellation).
There was a problem hiding this comment.
I will normalize vitest timeouts in relation to runner one
|
Are you no longer planning for a timeout on this step? The test does seem to be completing more reliably after your other changes. However, the possibility of hanging as we saw before still exists. |
Plan to do this in vitest instead with global hooks |
follow-up #1085 (comment)