test(ticket): fix flaky closeWorkflow mock.module race (unblock 3.1.41 deploy)#5
Conversation
|
Warning Review limit reached
More reviews will be available in 11 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughTest file refactoring that centralizes Bun mock registrations into an ChangesMock Setup Centralization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/utils/ticket/closeWorkflow.test.ts`:
- Around line 122-147: The test installs seam module mocks via
installSeamMocks() using mock.module for paths like
src/utils/discord/verifiedDelete, src/utils/fetchAllMessages,
src/utils/forumTagManager, and src/utils/ticket/builtinTypes but never restores
them; update the test teardown (afterAll) to restore those mocked modules (e.g.,
call mock.restoreModule for each of the same module paths or re-register the
original exports captured before mocking) so exports like
verifiedThreadDelete/verifiedMessageDelete and resolveTicketType are returned to
the module registry and do not leak into other tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ea742230-857a-4592-a7be-cd03a92ee730
📒 Files selected for processing (1)
tests/unit/utils/ticket/closeWorkflow.test.ts
NindroidA
force-pushed
the
fix/closeworkflow-test-flake
branch
5 times, most recently
from
54e3064 to
59f0267
Compare
…low module
Root cause of the flaky CI (11 archiveAndCloseTicket tests returning
archived:false on the push to main, green on macOS + on the PR check):
ticketHandlers.test.ts did `mock.module("ticket/closeWorkflow", () => ({
archiveAndCloseTicket: fake }))`. bun's mock.module is process-shared and is
NOT undone by mock.restore (verified on 1.2.17). On the Linux runner's file
order, that fake was registered before closeWorkflow.test.ts imported its SUT,
so closeWorkflow's entire suite exercised ticketHandlers' fake — whose return
value a sibling test sets to {archived:false} — instead of the real workflow.
macOS happened to evaluate closeWorkflow's real module first, hiding it.
Fix — remove the shared-module mock; inject instead:
- ticketHandlers.ts: registerTicketHandlers takes an optional injectable
`archiveAndCloseTicket` (default = real). router.ts caller is unchanged.
- ticketHandlers.test.ts: drop mock.module(closeWorkflow); pass the fake as the
3rd arg to registerTicketHandlers. Nothing mock.module's ticket/closeWorkflow
anymore, so closeWorkflow.test always binds the real SUT.
Hardening (same spirit, makes closeWorkflow.test fully injection-based with zero
shared global state — no mock.module, no AppDataSource.getRepository patch):
- closeWorkflow.ts: archiveAndCloseTicket gains an optional `deps`
(CloseWorkflowDeps) bundling fetchMessagesAsTranscript / ensureForumTag /
applyForumTags / verifiedChannelDelete / resolveTicketType / builtinTypeInfo /
archivedTicketRepo, defaulting to the real implementations. Production callers
(events/ticket/close.ts, api/handlers/ticketHandlers.ts) are unchanged.
- closeWorkflow.test.ts: pass all fakes via `deps`; static-import the SUT; drop
mock.module and the getRepository patch entirely.
Test-only behavior change; production code paths are identical (real defaults).
Verified: tsc clean, biome ./src clean, bun test 1171/0 (full + isolated +
[ticketHandlers, closeWorkflow] poison order).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NindroidA
force-pushed
the
fix/closeworkflow-test-flake
branch
from
59f0267 to
372ee22
Compare
The 3.1.41 hotfix (PR #3) merged to main but the CI run on main failed on 11
archiveAndCloseTickettests — a bunmock.moduleprocess-global race where the SUT bound the real deps at import time (returnedarchived:false). Passed locally + on the PR check; only the main-push file order hit it. This re-asserts the seam mocks immediately before the dynamic SUT import. Test-only; unblocks the gated deploy.Verified locally: tsc clean, biome ./src clean,
bun test1171/0 ×3 + synthetic poison-order scenario green.Summary by CodeRabbit