Skip to content

Build: share one test RNG across randomized tests (#1868) - #1882

Open
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
agent/fix-ci-test-rng-dedup
Open

Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
agent/fix-ci-test-rng-dedup

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1868

Root cause

crate::test_rng says it is the one deterministic RNG the randomized tests share, but socket-patch-core still had five more copies of the same xorshift64* generator:

  • crawlers/oracle_support.rs: pub(crate) struct Rng, used by the crawler *_matches_the_async_oracle tests and vex::verify.
  • crawlers/npm_crawler/oracle.rs: inlined into the Gen fixture builder.
  • patch/copy_tree.rs: a private struct Rng(u64).
  • vendor/go_sum_edit.rs and vendor/go_mod_edit.rs: inline closures. The issue didn't list these two.

pnpm_equivalence_tests.rs also wrote out Rng::new's seed mixing by hand.

Change

  • I deleted the oracle_support copy. The composer, go, ruby, python, nuget, cargo and maven crawler tests and vex::verify now import crate::test_rng::Rng directly.
  • npm_crawler::oracle::Gen holds a test_rng::Rng and calls self.rng.below/chance/pick directly. chance now takes a u64, as it does in test_rng.
  • copy_tree uses test_rng::Rng with the same raw seed.
  • The go.mod and go.sum property tests draw from test_rng::Rng::new(seed).
  • The pnpm equivalence test calls Rng::new(seed).

The seeds, the seed mixing (seed * 0x9E37_79B9_7F4A_7C15 | 1) and chance(p) == next() % 100 < p are all unchanged, so every generated case and case count stays the same. Everything touched is #[cfg(test)]; there is no production change.

After this PR, grep -rn 0x2545_F491_4F6C_DD1D crates/socket-patch-core/src finds the generator in test_rng.rs and one other hit in cargo_lock_equivalence_tests.rs, where the constant is a seed value, not a generator.

Review findings I didn't fix (from /code-review high)

  • CLI test copy (socket-patch-cli/src/commands/scan/hosted.rs test): left as is. The CLI crate can't see core's #[cfg(test)] test_rng, and sharing it would mean exporting a test helper from core's public API or adding a feature. That deserves its own decision.
  • go.mod / go.sum generators: they still take a FnMut(usize) -> usize closure, which now wraps Rng::below. Switching them to &mut Rng would rewrite about 40 rng(n) calls for no change in behaviour. The issue's goal, no second generator, is met either way.

CI cost

No workflow change. A lean PR run is unchanged: ~28 Linux jobs, about 8 min.

Test evidence

  • CI on f352ebf: all green (ci-ok, clippy, coverage (1)/(2), every lean e2e leg). Bugbot found no issues on f352ebf.
  • cargo fmt --all -- --check: clean.
  • cargo clippy --workspace --all-features -- -D warnings: clean. With --all-targets, the remaining lints are all in lines this PR doesn't touch (for example python_crawler.rs:4187, crawler_ruby_e2e.rs, CLI tests/prebuilt_common).
  • cargo test -p socket-patch-core --all-features --lib: 6994 passed, 4 failed. The 4 also fail on unmodified origin/main in this sandbox, which runs as uid 0, so root ignores the read-only dirs they depend on. Running the same test binary as uid 65534 (setpriv --reuid=65534), all 4 pass: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files.
  • This is a refactor with an identical random stream, so there is no red→green test. The randomized tests replaying the same seeds and still passing is the evidence.

https://claude.ai/code/session_011f9aeSP5GEeiLsq6p3Rmrk


Generated by Claude Code

The crawler oracle tests, copy_tree's random-tree test, the npm oracle
fixture builder and the go.mod / go.sum editor property tests each
carried their own copy of the xorshift64* generator that
crate::test_rng says is the only one. They now all draw from
test_rng::Rng. Seeds and the seed mixing are unchanged, so every test
replays exactly the same cases.

Test-only; no production code changes.

Refs #1868

Assisted-by: Claude Code:claude-opus-5-5
Review follow-up. The crawler and vex oracle tests now import
crate::test_rng::Rng instead of a re-export through oracle_support, the
npm oracle's Gen calls its Rng directly instead of through forwarding
methods, and the pnpm equivalence test seeds with Rng::new rather than
spelling the seed mixing out by hand. Same seeds, same cases.

Refs #1868

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 30, 2026 04:22
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f352ebf. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 30, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent).

  • Head: f352ebf, mergeable with no conflicts against main 5ca5623
  • CI: 32/32 checks green (50 skipped by the lean PR matrix), no failures
  • Bugbot: reviewed f352ebf, no findings; no open review threads
  • What to check: test-only refactor. Every randomized core test now uses crate::test_rng::Rng with the same seeds, so the generated cases don't change. The CLI test copy in scan/hosted.rs was left on purpose (see the description).
  • Slack: not announced. The Slack send action wasn't available in this run.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at f352ebf.

  • CI: 82/82 checks green on this head; no main-wide failures.
  • Bugbot: reviewed f352ebf, no findings; no open review threads.
  • Mergeable: yes, no conflicts with main.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief (f352ebf)

What it does: Five copies of the same xorshift64* test RNG used to live in core tests:

  • oracle_support
  • the npm oracle's Gen
  • copy_tree
  • the closures in go_mod_edit and go_sum_edit

They now all use the existing crate::test_rng::Rng. This PR changes test code only.

Risk: low. Every line it touches is #[cfg(test)]. I compared the random streams by hand and they are bit-identical to main: same seed mixing, same shifts, same multiplier, same below/chance, and copy_tree keeps its raw seed. Seeds, iteration counts and ranges are unchanged.

Look here:

Verified:

  • Targeted core tests (equivalence, oracle, copy_tree, go_mod_edit, go_sum_edit, pnpm): 613 passed, 1 failed. The one failure is relax_loop_must_not_traverse_symlinked_root. It is a 0o555 permission test that can only fail when run as root, it doesn't use the RNG, and the PR description already lists it.
  • Clippy is clean on the lines this PR changes.
  • CI is 82/82 green (32 success, 50 skipped). Bugbot is clean on this head.
  • CHANGELOG.md is untouched, Cargo.toml has no changes, and I found no debug leftovers.

Open questions (non-blocking): A sixth copy of the RNG remains in socket-patch-cli/src/commands/scan/hosted.rs. That's deliberate: core's cfg(test) helper isn't visible to the CLI crate.

Auto-merge is armed, so approving sends this PR straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use crate::test_rng::Rng in every randomized test: three more xorshift64* copies sit beside the module that claims to be the only one

2 participants