refactor(stella-core): take the two shared sleeper doubles from stella-time - #6491
Closed
macanderson wants to merge 4 commits into
Closed
refactor(stella-core): take the two shared sleeper doubles from stella-time#6491macanderson wants to merge 4 commits into
macanderson wants to merge 4 commits into
Conversation
…verity advisories
… license check, as arenabench's once were
…Sleeper and Clock ports #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, and a sleeper trait of stella-fleet's own with a third. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature for the test sweep that follows. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Refs #6484
…a-time About thirty test files across stella-core, stella-engine and their tests/ directories each wrote the same Sleeper double, a no-op one under three names and one on tokio's paused clock, and each copy grew a now() body in #6486. They are deleted. Integration tests and stella-engine take PausedSleeper and NoopSleeper from stella_time::test_util through a dev-dependency; stella-core's own unit tests keep one copy in src/tests.rs, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. stella-time's one_home witness now scans test files too. Closes #6484
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 3 days and 4 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
Reviewer's GuideThis PR removes duplicated sleeper test doubles across Flow diagram for the sleeper double one-home witnessflowchart TD
Sources[Scan Rust files under\ncrates/*/src and crates/*/tests]
Find[Find Sleeper double definitions]
Allow[Allow shared test_util doubles\nand documented exceptions]
Fail[Fail witness if an unexpected\nduplicate appears]
Pass[Pass with one home enforced]
Sources --> Find --> Allow
Allow -->|unexpected definition| Fail
Allow -->|kept exception or shared home| Pass
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Sep 11, 2026
Merged
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
…rp from the docs site (#6498) ## What & why Two follow-ups to #6488, each its own commit. - **`make core-no-io` told test code from shipping code by file name alone** (a `tests/` directory or a `tests.rs` file). A module declared under `#[cfg(test)]` by any other name was read as shipping code, which is why #6488's shared sleeper doubles had to be named `src/tests.rs`. The guard now reads every `#[cfg(test)] mod name;` line and skips the file or directory it names, at any depth. `scripts/test-core-no-io.sh` gains the case and its control (a plain `mod` line must still count), 29 cases in all. - **`sharp` is removed from the docs site instead of floored.** The site imports no `next/image`, so sharp was never loaded: inert weight from `next`'s optional dependencies that carried the advisory #6488 floored and fourteen LGPL-3.0 libvips tuples that `dependency-review` had to be told about by name on every `next` bump (the recurrence issue #2532 describes). The pnpm override drops it from the tree, its build-script approval goes with it, and the per-tuple exemption #6488 added to `dependency-review.yml` is deleted, since nothing is left to exempt. The `js-yaml` floor stays. Refs #6484. Closes nothing by design (`closes-nothing`). ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `scripts/test-core-no-io.sh`'s new case `a module named under #[cfg(test)] is not shipping code, by any name and at any depth` fails against `main`'s guard (the file it names reads `std::fs`) and passes here; its control `a module named by a plain mod line is shipping code` pins that the rule did not widen. For the sharp change the evidence is the lockfile: `sharp` appears once in `website/pnpm-lock.yaml`, in the override line, and `docs.yml`'s `typecheck + build` job is what proves `next build` does not need it. ## The gate - [x] `cargo fmt --check` (no Rust touched) - [x] clippy — no Rust touched - [x] tests — the guard harness ran locally, 29 passed; `make prose`, `make license-allowlist-parity` and `make action-pins` green - [x] Docs updated where behavior changed (the guard's own docstring and AGENTS.md's no-io paragraph name the new rule) - [x] CLA signed ## Fix over file - [x] Extra fixes in this PR: none beyond the two named - [x] Nothing was deferred ## Ground-rule check - [x] No I/O added to `stella-core` (not touched) - [x] No new outbound network calls - [x] No new cross-boundary serde types ## Anything reviewers should know? - If the docs site ever renders a `next/image`, restore a `sharp` floor in `website/pnpm-workspace.yaml` and put the tuples back in the workflow; the comment there says so. - The stacked doubles PR (#6491) was closed when #6488's branch was deleted on merge; it is re-opened against `main` separately and does not depend on this one. ## Summary by Sourcery Correct test-code classification in the core no-I/O check and remove unused Sharp dependencies from the documentation site. Bug Fixes: - Update the core no-I/O guard to exclude modules declared under `#[cfg(test)]` regardless of their file name or nesting depth, while preserving shipping-code detection for plain modules. Enhancements: - Remove the unused Sharp dependency from the documentation site and retain the existing js-yaml security floor. CI: - Remove Sharp and its platform-specific license exemptions from dependency-review configuration and its build-script approval. Documentation: - Document the expanded test-module exclusion rule in the core no-I/O guard guidance. Tests: - Add regression and control cases covering cfg-gated modules at multiple depths and plain modules.
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
…a-time (#6499) ## What & why The test-double half of #6484, on top of the merged `stella-time` PR (#6488). Re-opened: the first copy (#6491) was closed by GitHub when #6488's branch was deleted on merge. About thirty test files across `stella-core`, `stella-engine` and their `tests/` directories each wrote the same `Sleeper` double — a no-op one under three names, and one on tokio's paused clock — and each copy grew a `now()` body in #6486. Every one of them is deleted here. - Integration tests and `stella-engine` take `PausedSleeper` and `NoopSleeper` from `stella_time::test_util` through a dev-dependency on `stella-time` with `test-util` on. For `stella-core` that is a dependency cycle, which cargo allows; it is the tokio / tokio-test shape. - `stella-core`'s own unit tests keep one copy, `src/tests.rs`, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. The compiler forces that copy (the first attempt at taking the crate's doubles from `stella-time` failed on every unit test with "multiple different versions of crate `stella_core`"). It is named `tests.rs` so `make core-no-io` reads it as test code, which is how that guard tells test code from shipping code. - The `one_home` witness in `stella-time` now scans test files too. Its kept list shrinks to the four doubles with a shape a shared one cannot have — the retry tests' recording sleeper, the monitor's advancing sleeper, the hanging sleeper, and `step/tests.rs`'s unpaused-time sleeper — plus the forced copy. Closes #6484 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `crates/stella-time/tests/one_home.rs` reads every `.rs` under `crates/*/src` and `crates/*/tests`. On the base branch it fails on twenty-three files that each define a sleeper double; here it passes, and a second test fails if a kept one disappears. Locally: `stella-core`'s 893 lib tests and every integration test, `stella-engine`'s tests and the time crate's 6 + 3 tests pass. ## The gate - [x] `cargo fmt --check` - [ ] clippy over the workspace at `-D warnings` — CI; ran locally on `stella-core`, `stella-engine`, `stella-time` - [ ] the workspace test suite — CI - [x] Docs updated where behavior/flags changed (README, `--help`, doc comments) - [x] CLA signed - [x] `Closes #N` appears **both** above and as a commit trailer ## Fix over file - [x] Extra fixes in this PR: none beyond the sweep - [x] Nothing was deferred ## Ground-rule check - [x] No I/O added to `stella-core`; `src/tests.rs` is `#[cfg(test)]` and `make core-no-io` is green - [x] No new outbound network calls - [x] No new cross-boundary serde types ## Anything reviewers should know? - The rename `TokioSleeper` → `PausedSleeper` in the test tree is deliberate: the paused-clock double must not share a name with the real `stella_time::TokioSleeper`. - Once #6498 lands (the no-io guard reads `#[cfg(test)] mod` lines), `src/tests.rs` could take a more descriptive name; it is named for the guard's current file-name rule. ## Summary by Sourcery Consolidate test sleeper doubles in `stella-time` and enforce a single ownership location across the workspace. Enhancements: - Centralize the shared no-op and paused-clock sleeper doubles in `stella-time` and update `stella-core` and `stella-engine` tests to use them. - Retain only the specialized sleeper doubles that require unique behavior, including the compiler-required `stella-core` unit-test copy. - Extend the sleeper ownership witness to scan test sources and document the shared test utility arrangement. Build: - Add `stella-time` as a test-only dependency with the `test-util` feature where shared sleeper doubles are used. Documentation: - Update architecture and crate documentation to describe the shared sleeper test utilities and the `stella-core` unit-test exception. Tests: - Update the affected unit and integration tests to use the centralized sleeper doubles.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What & why
The test-double half of #6484, stacked on the
stella-timePR. About thirty test files acrossstella-core,stella-engineand theirtests/directories each wrote the sameSleeperdouble — a no-op one under three names, and one on tokio's paused clock — and each copy grew anow()body in #6486. Every one of them is deleted here.stella-enginetakePausedSleeperandNoopSleeperfromstella_time::test_utilthrough a dev-dependency onstella-timewithtest-utilon. Forstella-corethat is a dependency cycle, which cargo allows; it is the tokio / tokio-test shape.stella-core's own unit tests keep one copy,src/tests.rs, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. The compiler forces that copy (the first attempt at taking the crate's doubles fromstella-timefailed on every unit test with "multiple different versions of cratestella_core"). It is namedtests.rssomake core-no-ioreads it as test code, which is how that guard tells test code from shipping code.one_homewitness instella-timenow scans test files too. Its kept list shrinks to the four doubles with a shape a shared one cannot have — the retry tests' recording sleeper, the monitor's advancing sleeper, the hanging sleeper, andstep/tests.rs's unpaused-time sleeper — plus the forced copy.Closes #6484
The witness
main, passes here)crates/stella-time/tests/one_home.rsreads every.rsundercrates/*/srcandcrates/*/tests. On the base branch it fails on twenty-three files that each define a sleeper double; here it passes, and a second test fails if a kept one disappears. Locally:stella-core's 893 lib tests and every integration test,stella-engine's tests and the time crate's 6 + 3 tests pass.The gate
cargo fmt --check-D warnings— CI; ran locally onstella-core,stella-engine,stella-time--help, doc comments)Closes #Nappears both above and as a commit trailerFix over file
Ground-rule check
stella-core;src/tests.rsis#[cfg(test)]andmake core-no-iois greenAnything reviewers should know?
TokioSleeper→PausedSleeperin the test tree is deliberate: the paused-clock double must not share a name with the realstella_time::TokioSleeper.stella-timePR; retarget tomainonce that merges.Summary by Sourcery
Centralize shared sleeper test doubles in
stella-timeand remove their duplicated definitions across core and engine tests.Enhancements:
NoopSleeperand paused-clock sleeper test doubles aroundstella-time's shared test utilities acrossstella-core,stella-engine, and their tests.stella-coreunit-test copy.Build:
stella-timewith thetest-utilfeature as a development dependency forstella-coreandstella-engine.Documentation:
stella-coreunit-test exception.Tests:
NoopSleeperandPausedSleeperimplementations.