Cover held ordering locks in cfworkers - #1019
Conversation
src/mod.test.ts covered the ordering lock being taken and released, but never the case where processMessage() finds one already held and returns shouldProcess: false. test/mq.test.ts covers that branch by stubbing get() to return a lock unconditionally; this test takes the lock with a real processMessage() call and sends a second message for the same key without releasing it, so it also proves the key that processMessage() writes is the one it later reads back. Closes fedify-dev#879 Changelog: none Assisted-by: Claude Code:claude-opus-5
✅ Deploy Preview for fedify-json-schema canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughAdded test coverage for ChangesQueue ordering lock behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This test-only change adds coverage for held ordering-key locks without changing production behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Closes #879.
WorkersMessageQueue.processMessage()returnsshouldProcess: falsewhen an ordering key lock is already held, which is what tells a queue handler to retry the message instead of running it out of order.src/mod.test.tscovered the lock being taken and released, but never the already-held case.test/mq.test.tsalready covers that branch, but it stubsget()to return a lock unconditionally, so the lock key itself is never exercised. This test takes the lock with a realprocessMessage()call and then sends a second message for the same ordering key without releasing it, which is the state a queue handler is in while the first message is still in flight. That also proves the keyprocessMessage()writes is the one it later reads back.Checks
mise run check-each cfworkers— passes.vitest run— 44 tests pass across 3 files.processMessage()withif (false)makes it fail withexpected true to be false, and it passes again once reverted.Rebased onto
mainafter #1016 touched the same file; both tests coexist.No changelog fragment, since this is test-only work; the commit carries
Changelog: none.AI disclosure
Per AI_POLICY.md: I wrote the test myself. Claude Code (claude-opus-5) helped diagnose why my first attempt asserted against the lock-acquire path instead of the lock-exists path, and ran the checks listed above. The commit carries the
Assisted-by: Claude Code:claude-opus-5trailer.