Make MockEventQueue run jobs inline; drop test-suite leak workarounds

[jacksonloper]

Fixes #1481.

What was wrong

MockEventQueue drained its jobs on a self-rearming 1-second timer that nothing awaited:

private async triggerJobs(){
    ...
    await new Promise(r => setTimeout(r, this._delay));  // 1000ms
    await this.processNextJob();   // → triggerJobs() again
}

Jest isolates module registries but not the event loop. Each file got a fresh queue, fresh mock DBs and a fresh app, but the previous file's chain kept running — against the previous file's objects — for roughly one second per queued job. That's the leak --forceExit was papering over, and the reason the whole suite ran in one process: --detectOpenHandles forces runInBand (@jest/core/build/testSchedulerHelper.js:29).

The leaked jobs also never did their work. Once a file has torn down, console.info throws, and the "MEQ: Processing job" log sits inside processNextJob's try. So roughly 4 jobs per run aborted before their handler ran and were silently discarded, and the catch's own log threw again. The timer wasn't modelling async delivery — it was randomly dropping about a third of the jobs.

What changed

Jobs run inline. publish() resolves once its handlers have finished. Tests that need to observe the gap between "enqueued" and "handled" use pause() / resume(), which is deterministic and needs no timer. This is no less faithful to production than before: pg-boss polls at newJobCheckInterval: 500 with teamSize: 5 (PGBossEventQueue.ts:39), so the mock's strictly-serial 1000 ms modelled neither its latency nor its concurrency.

Removed the waits this made unnecessary — the six waitUntilJobsFinished() call sites and finalizeElection's 4-second sleep.

Every one of them was already a no-op, which I verified rather than assumed: with the old 1 s delay left intact and waitUntilJobsFinished neutered to a no-op, all 47 tests in the four affected files still passed. The reason is structural — ballots are persisted synchronously at castVoteController.ts:256, before the response; the queue only mails receipts.

finalizeElection's sleep was worse than decorative. Its comment said "Wait a few seconds for job queue to process emails", but finalize publishes nothing at all — invites go out through POST /Election/:id/sendInvites. It burned 4 s of a 5 s timeout waiting for jobs that were never created, and it's the one test the issue records failing on a genuine timeout.

Added ballotReceiptEmail.test.ts. Nothing in the suite had ever asserted on the queue's output: the mock EmailService records into sentEmails and TestHelper:47 wires it up, but no test read it, and both emailService.clear() calls were commented out. So handleCastVoteEvent — the only consumer the suite exercises at all — had zero coverage. The receipt is worth pinning down because castVoteController deliberately scrubs ballot_id from the HTTP response, making the email the only channel that carries it back to the voter.

Both tests are mutation-checked, so they aren't vacuous:

mutant	test 1	test 2
drop the sendEmails call in handleCastVoteEvent	✗ fails	✗ fails
send the receipt inline instead of publishing	✓ passes	✗ fails

Dropped both jest flags. --detectOpenHandles was reporting nothing while serialising 22 files into one process; --forceExit is no longer needed because nothing is left scheduled.

Switched emailRoll to th.expressApp instead of a second makeApp(), which was re-registering all three handlers and printing three already have handler errors every run. (This was cited in the issue as evidence of cross-file collision — it isn't; it's one file calling makeApp() twice.)

Results

Measured locally, 50 consecutive runs:

	before	after
wall time	15.3 s	2.8 s
failures	4 / 50 (per issue)	0 / 50
"Cannot log after tests are done"	9 per run	0
already have handler errors	3 per run	0
exits without --forceExit	no	yes

151 tests, 23 suites, tsc clean.

One honest caveat on the flake number: 0/50 is consistent with the fix working, but if the true rate were still 8% you'd see a clean 50 about 1.5% of the time. The mechanism that selected 404 vs 400 vs ECONNRESET was never pinned down in #1481 and isn't pinned down here either — what this PR proves is that the leak is gone and the suite exits on its own. If a flake resurfaces, the remaining suspect is supertest's one-ephemeral-listener-per-request pattern (request(app) calls http.createServer on every call), not the queue.

Follow-ups not in this PR

• handleSendInviteEvent and handleSendEmailEvent are subscribed and never invoked by any test — no test hits /sendInvites, /sendEmails, or /sendInvite/:voter_id. Worth covering with the same drain pattern.
• When you do, note that sendInvitesController.ts:136 reads emailResponse.length and emailResponse[0][0].statusCode from EmailService.sendEmails, but the mock returns undefined. That path will throw on the mock the moment a test drives it — the fake's return contract never matched what the production code expects, because nothing ever exercised it.
• MockEventQueue.ts still lives beside the real implementation in src/Services/EventQueue/ rather than a __mocks__/ directory, so it compiles into build/. Left alone here to keep the diff focused.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ECGsUoD9JCefu95Aw7HieW

[netlify]

✅ Deploy Preview for bettervoting ready!

Name	Link
🔨 Latest commit	e4e6aa0
🔍 Latest deploy log	https://app.netlify.com/projects/bettervoting/deploys/6a76468c81f2f90008b71eb7
😎 Deploy Preview	https://deploy-preview-1491--bettervoting.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved background event processing for more reliable and predictable task handling.
  • Added clearer error reporting when queued tasks fail.
  • Preserved asynchronous email delivery after ballot submission.

• Tests

  • Added coverage for ballot receipt emails, including recipient, subject, and ballot identifier validation.
  • Simplified asynchronous test flows by removing fixed delays and manual queue-wait steps.
  • Improved test reliability and execution consistency.

Walkthrough

The mock event queue now processes jobs through an inline, re-entrant-safe drain loop. It exposes pending job counts and asynchronously drains jobs on resume. Handler failures include job and queue details. Tests now validate asynchronous ballot receipt delivery and remove obsolete queue waits and fixed delays. The backend test command no longer uses Jest force-exit or open-handle detection options.

Poem

I’m a rabbit watching jobs hop through the queue,
Draining them safely, one by one, on cue.
Receipts wait quietly, then emails take flight,
Tests check each ballot and metadata right.
No forced exits, no four-second delay—
Clean little hops make the suite run this way.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes to MockEventQueue and the removal of test-suite leak workarounds.
Description check	✅ Passed	The description provides detailed problem, solution, testing, issue, and follow-up information; screenshots are not required for this backend change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1481-mock-event-queue-inline

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@packages/backend/src/Services/EventQueue/MockEventQueue.ts`:
- Around line 64-65: Update the interaction between publish() and drain() in
MockEventQueue so a publish invoked by a currently running handler does not
resolve until its queued handler has executed. Replace or refine the _draining
early return in drain(), and define consistent nested-publish behavior while
preserving completion semantics for both top-level and nested jobs.

🪄 Autofix

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: bb58472f-c0f1-40bd-808b-95b4f0790c0b

📥 Commits

Reviewing files that changed from the base of the PR and between 7bc75a8 and e4e6aa0.

📒 Files selected for processing (8)

• packages/backend/package.json
• packages/backend/src/Services/EventQueue/MockEventQueue.ts
• packages/backend/src/test/anonymizedBallots.test.ts
• packages/backend/src/test/ballotReceiptEmail.test.ts
• packages/backend/src/test/emailRoll.test.ts
• packages/backend/src/test/finalizeElection.test.ts
• packages/backend/src/test/idRoll.test.ts
• packages/backend/src/test/writeIns.test.ts

💤 Files with no reviewable changes (4)

• packages/backend/src/test/finalizeElection.test.ts
• packages/backend/src/test/anonymizedBallots.test.ts
• packages/backend/src/test/writeIns.test.ts
• packages/backend/src/test/idRoll.test.ts

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Preserve publish() completion semantics for nested jobs.

When a handler awaits publish(), drain() returns at Line 64 because _draining is true. The nested publish() then resolves before its handler runs. The outer drain cannot process that job until the current handler returns. This violates the stated publish() completion contract.

Define nested publish semantics explicitly, or change the drain design so nested publish() calls do not report completion before their jobs finish.

🤖 Prompt for 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.

In `@packages/backend/src/Services/EventQueue/MockEventQueue.ts` around lines 64 -
65, Update the interaction between publish() and drain() in MockEventQueue so a
publish invoked by a currently running handler does not resolve until its queued
handler has executed. Replace or refine the _draining early return in drain(),
and define consistent nested-publish behavior while preserving completion
semantics for both top-level and nested jobs.