test(rivetkit-core): cover sleep/destroy guards and prevent_sleep no-op#4750
test(rivetkit-core): cover sleep/destroy guards and prevent_sleep no-op#4750NathanFlurry wants to merge 1 commit intosleep-cleanup/08-sleep-docsfrom
Conversation
d844ecd to
e68bdc2
Compare
89647d5 to
ed93ef7
Compare
391fcaa to
e4395f8
Compare
e4395f8 to
d1c2bbf
Compare
ed93ef7 to
8d93174
Compare
|
Code Review - PR 4750 - test(rivetkit-core): cover sleep/destroy guards and prevent_sleep no-op. Overview: adds 6 tests covering sleep/destroy pre-start rejection, double-call rejection, deprecated no-op contract, and deadline token cancellation. Issues: (1) Missing cross-guard coverage - sleep() and destroy() use separate atomics so destroy-then-sleep succeeds on both; unclear if intentional. (2) CLAUDE.md says tests go under tests/ not inline in src/; pub(crate) helpers justify the exception but a comment calling it out would help. Suggestions: (1) Three tests are sync-only and don't need start_paused=true. (2) The stub wording in the no-op assertion message could be updated to match the NAPI bridge compatibility rationale in context.rs. (3) Adding assert_eq!(ctx.can_sleep().await, CanSleep::NotReady) to a before-started test would cover both guards on the started flag. Positives: structured error assertions are correct, allow(deprecated) is explicit, unique actor IDs avoid global state risk. |
Preview packages published to npmInstall with: npm install rivetkit@pr-4750All packages published as Engine binary is shipped via Docker images: docker pull rivetdev/engine:slim-9b1b005
docker pull rivetdev/engine:full-9b1b005Individual packagesnpm install rivetkit@pr-4750
npm install @rivetkit/react@pr-4750
npm install @rivetkit/rivetkit-napi@pr-4750
npm install @rivetkit/workflow-engine@pr-4750 |

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: