🐛 fix: revert vi.restoreAllMocks() breaking 265 tests#20435
Conversation
vi.restoreAllMocks() in afterEach restores spy/mock implementations to originals, breaking tests that set up mocks in beforeAll or rely on module-level vi.mock() declarations. Also reverts threads pool change since forks provides better test isolation. Fixes #20434 Signed-off-by: GitHub Actions <actions@github.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
🐝 Hi @clubanderson! I'm Trusted users — org members and contributors with write access — can mention Automation may take a moment to start, and follow-up happens through workflow activity rather than chat replies. |
kubestellar-prow
Bot
added
the
size/XS
There was a problem hiding this comment.
Pull request overview
This PR fixes widespread Vitest failures introduced by PR #20425 by undoing aggressive global mock restoration in the test setup, and it reverts the CI test worker pool away from threads to avoid shared-memory mock contamination.
Changes:
- Removed
vi.restoreAllMocks()from the globalafterEachtest cleanup (keepsvi.clearAllMocks()). - Reverted the Vitest
pool: 'threads'CI configuration back to the default pool behavior (forks).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/vite.config.ts | Stops forcing the CI threads pool (reverts to default pool) to improve isolation between test files. |
| web/src/test/setup.ts | Removes global vi.restoreAllMocks() from afterEach to prevent breaking tests that rely on persistent spies/mocks. |
| // CI: use threads pool instead of forks to prevent OOM from subprocess spawn storm (#20007) | ||
| // Threads share memory, no fork overhead. Local dev uses default (forks) for stronger isolation. | ||
| pool: process.env.CI ? 'threads' : undefined, | ||
| // pool: process.env.CI ? 'threads' : undefined, // Reverted: threads share memory, can cause mock cross-contamination |
❌ Playwright Tests Failed📊 View Full ReportDownload the To view the report locally: # Download and extract playwright-report.zip
npx playwright show-report path/to/playwright-report |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
✅ Post-Merge Verification: passedCommit: |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
…low (#20440) PR #20435 reverted pool:'threads' in vite.config.ts but workflow still hardcoded --pool=threads on line 69, forcing threads mode despite config. Threads share memory → mock cross-contamination → 265 test failures. Fixes #20439 Signed-off-by: sec-check <sec-check@hive.kubestellar.io> Co-authored-by: sec-check <sec-check@hive.kubestellar.io>
Fixes #20434
Root Cause
PR #20425 added
vi.restoreAllMocks()to the globalafterEachinweb/src/test/setup.ts. This restores all spy/mock implementations to their original values after every test, which breaks any test that:beforeAllexpecting them to persistvi.mock()factory return values that get clearedAlso reverts the
pool: 'threads'change since threads share memory and can cause mock cross-contamination between test files.Fix
vi.restoreAllMocks()from afterEach (keepvi.clearAllMocks()which is safe)