fix(encrypt-upload-client): ensure playwright tests exit cleanly in CI#634
fix(encrypt-upload-client): ensure playwright tests exit cleanly in CI#634gitsofaryan wants to merge 30 commits intostoracha:mainfrom
Conversation
travis
left a comment
travis
left a comment
There was a problem hiding this comment.
this looks fine, but fwiw it does not seem to solve the core issue here - I tried re-enabling the browser tests in CI (see #635 and https://github.com/storacha/upload-service/actions/runs/20974056083 which demonstrated the same "hanging forever" behavior
that said, it seems reasonable to exit the process here, so I've approved
|
Thanks for catching that, @travis . You're rightI realized test.afterAll only runs in the worker process, so process.exit(0) there wasn't killing the main process where the secure server lives. should we move with the explicit exit to |
|
Update: Moved the process.exit(0) call from the test worker (afterAll hook) to globalTeardown. Why: The previous attempt only exited the worker process, leaving the main Playwright orchestration process (and the secure server) alive, which caused the CI job to hang. Verification: Verified locally with |
|
Acknowledged, @travis. |
hey thanks! really appreciate you looking into this! let me know when it's ready and I will take another look. |
|
Hey @travis , I've successfully fixed the CI hang and verified it in my forked repo. (Note: I'm not 100% sure about the 'billing plan' tests passing since the credentials aren't set up in my fork, but the hangs are definitely gone.) check my run -> https://github.com/gitsofaryan/upload-service/actions/runs/21056512559 I also fixed some lint errors and validated the lockfiles. Please take a look at this run, and if it looks good, we can run it on the main repo. Thanks a lot! |
travis
left a comment
travis
left a comment
There was a problem hiding this comment.
this looks fine, but fwiw it does not seem to solve the core issue here - I tried re-enabling the browser tests in CI (see #635 and https://github.com/storacha/upload-service/actions/runs/20974056083 which demonstrated the same "hanging forever" behavior
that said, it seems reasonable to exit the process here, so I've approved
There was a problem hiding this comment.
this test is still disabled in CI - this should fix that and prove this is working
| "test:browser": "npm run test:setup-certs && npx playwright test" |
- Simplified all test wrapper scripts to minimal timeout-based pattern - Removed unnecessary output parsing and complex exit logic - Added global timeout (5min) to Playwright config for CI - Improved browser launch options for CI stability - Enhanced server cleanup in global teardown - Removed fix/** branch trigger from CI workflow This approach relies on: 1. Global teardown for proper server cleanup 2. Simple timeout protection in wrappers 3. Playwright's globalTimeout as safety net 4. Clean process.exit() to prevent hanging Signed-off-by: Aryan Jain <aryan.jain.csbs22@ggits.net>
…ent CI hangs - Add 5s timeout per fetch in revocation check to prevent hanging on slow endpoints - Use AbortSignal.any() to compose global abort and per-fetch timeouts - Add queue.onIdle() in finally block to ensure all queue tasks complete - Simplify test commands to direct Playwright/test runner invocation - Remove redundant test wrapper scripts from CLI, encrypt-upload-client, and UI packages - Keep Playwright global teardown with process.exit(0) in CI to force clean exit - Revert unnecessary lint ignore patterns and focus on actual hang fix
- Add test:browser to CI workflow - Remove playwright from tsconfig exclude
2 participants
Context
Browser tests for encrypt-upload-client were hanging in CI even after all tests passed, causing jobs to be cancelled after several hours (see #629). CI browser tests were temporarily disabled in #630.
What this does
Adds an explicit CI-only teardown to ensure Playwright tests exit cleanly and do not keep the Node event loop alive.
Why
This addresses the underlying hang while keeping the fix minimal and safe. It should allow browser tests to be safely re-enabled in CI in a follow-up PR.
Scope