test: fix expired GovObject proposal fixtures#316
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo test suites—GovObject and Proposal—now set up and tear down Sinon fake timers at a fixed UTC timestamp ( ChangesSinon fake timer setup in test suites
🎯 1 (Trivial) | ⏱️ ~3 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 review |
✅ Actions performedReview triggered.
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Test-only PR freezes the wall clock at 2024-01-01 inside the two GovObject/Proposal describes whose fixtures embed an end_epoch of 2025-10-10, so Proposal#getSerializationError's end_epoch < now check no longer rejects them. Sinon is already required in both files, the freeze date sits inside the [2015-10-10, 2025-10-10] window, and the explicit expired-date negative tests use pre-2024 dates so they still trigger expiry. Both reviewing agents reported no defects; verified against the diff.
Out-of-scope follow-up suggestions (2)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Proposal validation hard-codes wall-clock check —
lib/govobject/types/proposal.jsvalidatesend_epoch >= nowagainst real wall time. That is the underlying reason fixtures must be patched in tests as time marches on, and downstream consumers can see previously-valid GovObjects become invalid solely due to clock drift. Not introduced or affected by this PR.- Follow-up: Open a separate issue to discuss decoupling Proposal serialization validation from wall-clock time (e.g., optional clock parameter, or separating parse-vs-submit checks).
- Other GovObject test files may share the same time-bomb risk — This PR patches two known suites. Any future test that constructs Proposals with hard-coded end_epoch dates will hit the same expiry issue. A shared mocha root-hook helper would prevent recurrence without touching production code.
- Follow-up: Author-requested follow-up PR to introduce a shared
freezeProposalClock()helper (or a mocha root before/after) for any test that serializes Proposals.
- Follow-up: Author-requested follow-up PR to introduce a shared
Fix expired GovObject proposal fixtures
Summary
2024-01-01T00:00:00Z.2025-10-10proposal fixture valid without changingserialized fixture hex.
existing fixture data.
Validation
npm run test:node -- --grep 'GovObject|Proposal'-> 50 passingnpm run test:node-> 3509 passing, 17 pendingnpm testwas also attempted locally, but the browser/Karma phase cannotlaunch Firefox in this environment:
No binary for Firefox browser on your platform. The node test phase iscovered above.
Review
Summary by CodeRabbit