You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(usage): pin checkpoint fixture clock after scan yields - #8571
The checkpoint-digest test can fail with content_changed on a loaded worker after #8455. Its fourth cancellation check deliberately rewrites the ledger after verification, but an 8 ms row yield adds an earlier check and triggers the rewrite before verification. Pin the monotonic clock only in this fixture and restore it in finally; keep the scanner's elapsed-time and cancellation tests on their existing controlled clocks.
Addresses the post-merge CodeRabbit finding on #8455 and the Codex finding at #8455 (comment). Runtime scheduling, cancellation checks, the 750 ms health probe and lock-acquisition budget are unchanged. No user documentation change is needed for this test-only fix.
Verification
Current-head validation at c3992e31412f59d2bb1a5ca8784dc32f638c539f after merging deva159cdde84032324f03cbce7dbf467037d0c5412:
bun test tests/usage/usage-ledger-scanner.test.ts: 23 pass, 0 fail in the scanner file, including elapsed-time yields and post-yield cancellation.
bun run typecheck: no errors in 2,435 files. bun run privacy:scan, bun run structure:check and git diff --check --cached origin/dev (before the merge commit): pass.
bun test tests/ci-workflows/file-size-ratchet.test.ts: 9 pass, 0 fail.
The personal patch has the same stable patch ID before and after the base refresh. Historical red/green at 114698a7: advancing the original checkpoint fixture clock by 9 ms makes it fail with content_changed; the fixed fixture pins only that clock. That reproduction was not repeated as part of this base refresh.
Resource exception: focused validation covers this test-only patch on the new base. The shared Mac runs the live proxy; previous broad runs were costly. The full suite and cross-platform CI were not run here; required CI remains for upstream. No manual full-CI workflow was dispatched.
Checklist
Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed (no user behavior change).
Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (test clock only).
Review readiness checklist
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Ready for review based on the verified local checks above. This does not attest completion of a fresh automated review or required CI.
Summary by CodeRabbit
Tests
Improved the reliability of a ledger-scanning test by isolating rewrite timing and ensuring the timing spy is restored, while retaining checks for detected content changes.
No actionable comments were generated in the recent review.
Walkthrough
The checkpoint-digest test now freezes performance.now() during the ledger rewrite scenario and restores the spy in finally. It still checks that the scan returns the original checkpoint and that a later incremental scan detects changed content.
Changes: Checkpoint digest test
Layer / File(s)
Summary
Control timing in the rewrite scenario tests/usage/usage-ledger-scanner.test.ts
The test freezes performance.now() around the rewrite-after-verification scenario and restores the spy in finally. It retains the assertions for the original checkpoint and the content_changed result.
Reviewing files that changed from the base of the PR and between de0914a and 114698a.
Files selected for processing: 1
tests/usage/usage-ledger-scanner.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
all PR quality gates passed; the review readiness checklist is complete.
Review readiness checklist
✅ Required local validation passed; commands, results, and any full-suite exception are documented.
✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
✅ I resolved all correct Codex and CodeRabbit findings.
✅ My PR is ready for review.
✅ 4/4 boxes ticked.
This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun@Ingwannu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
choreMaintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).review-ready
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The checkpoint-digest test can fail with
content_changedon a loaded worker after #8455. Its fourth cancellation check deliberately rewrites the ledger after verification, but an 8 ms row yield adds an earlier check and triggers the rewrite before verification. Pin the monotonic clock only in this fixture and restore it infinally; keep the scanner's elapsed-time and cancellation tests on their existing controlled clocks.Addresses the post-merge CodeRabbit finding on #8455 and the Codex finding at #8455 (comment). Runtime scheduling, cancellation checks, the 750 ms health probe and lock-acquisition budget are unchanged. No user documentation change is needed for this test-only fix.
Verification
Current-head validation at
c3992e31412f59d2bb1a5ca8784dc32f638c539fafter mergingdeva159cdde84032324f03cbce7dbf467037d0c5412:bun test tests/usage/usage-ledger-scanner.test.ts: 23 pass, 0 fail in the scanner file, including elapsed-time yields and post-yield cancellation.bun run typecheck: no errors in 2,435 files.bun run privacy:scan,bun run structure:checkandgit diff --check --cached origin/dev(before the merge commit): pass.bun test tests/ci-workflows/file-size-ratchet.test.ts: 9 pass, 0 fail.114698a7: advancing the original checkpoint fixture clock by 9 ms makes it fail withcontent_changed; the fixed fixture pins only that clock. That reproduction was not repeated as part of this base refresh.Checklist
Review readiness checklist
Ready for review based on the verified local checks above. This does not attest completion of a fresh automated review or required CI.
Summary by CodeRabbit