Skip to content

fix(tests): resolve announcement test flakiness under --pool=threads#3098

Merged
bokelley merged 2 commits intomainfrom
claude/issue-3092-announcement-test-module-isolation
Apr 25, 2026
Merged

fix(tests): resolve announcement test flakiness under --pool=threads#3098
bokelley merged 2 commits intomainfrom
claude/issue-3092-announcement-test-module-isolation

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #3092

Summary

Under --pool=threads, vitest workers share the module registry. The 7 tests/announcement/ files that use dynamic await import() inside test bodies were susceptible to stale module state bleeding across concurrent workers — manifesting as TypeError: Cannot read properties of undefined and 5 s timeouts when the machine is under Conductor multi-workspace load.

Changes:

  • Adds vi.resetModules() as the first call in beforeEach in all 7 affected test files (announcement-backlog, announcement-backlog-route, announcement-admin-route, announcement-handlers, announcement-backfill, announcement-channel-resolver, announcement-reminder). This clears the module registry before each test so the subsequent await import() always resolves a fresh module with the vi.mock factory applied cleanly.
  • Adds mockLoadAnnouncementBacklog.mockReset() to announcement-backlog-route.test.ts beforeEachclearAllMocks() alone does not drain unconsumed mockResolvedValueOnce queues.
  • Adds testTimeout: 10000 to vitest.config.ts so a single stalled test fails with its name rather than silently consuming the entire 60 s precommit budget.

Non-breaking justification: test-only changes; no production code modified; --pool=threads config preserved (required to avoid non-TTY stdin hang in pre-commit hook — see comment in vitest.config.ts).

Pre-PR review:

  • code-reviewer: approved — no blockers; nits noted (lock flag reset is a pre-existing issue, not introduced here)
  • dx-expert: approved — sign-off granted; two nits logged as follow-up material

Session: https://claude.ai/code/session_01RRJENfLEsQ47mMs4NZKUaa


Generated by Claude Code

claude added 2 commits April 25, 2026 09:55
Adds vi.resetModules() to beforeEach in all 7 announcement test files
that use dynamic await import() calls, preventing stale module cache
state from bleeding across concurrent vitest workers. Also adds
testTimeout:10000 to vitest.config.ts so a stalled test fails with
its name rather than silently exhausting the 60s precommit budget.

Closes #3092

https://claude.ai/code/session_01RRJENfLEsQ47mMs4NZKUaa
The original comment said "cached module from another thread bleeds"
which is wrong: vitest 4.x pool:'threads' isolates each file in its
own worker. The actual bug is within-file — repeated await import()
returns the same cached instance, and clearAllMocks() doesn't clear
the module registry. Fix comment in all 7 patched files.

https://claude.ai/code/session_01RRJENfLEsQ47mMs4NZKUaa
@bokelley bokelley marked this pull request as ready for review April 25, 2026 10:00
@bokelley bokelley merged commit fa9f227 into main Apr 25, 2026
12 checks passed
@bokelley bokelley deleted the claude/issue-3092-announcement-test-module-isolation branch April 25, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky precommit: tests/announcement/* fail under thread-pool when system has concurrent Node load

2 participants