test(cli): give the Rust test suite its own package-manager caches - #143
test(cli): give the Rust test suite its own package-manager caches#143John-David Dalton (jdalton) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cb1c007. Configure here.
| ("ASDF_DIR", ".asdf"), | ||
| ("ASDF_DATA_DIR", ".asdf"), | ||
| ("SDKMAN_DIR", ".sdkman"), | ||
| ("MISE_DATA_DIR", ".local/share/mise"), |
There was a problem hiding this comment.
Mise root misses custom XDG
Medium Severity
isolate() always pins XDG_DATA_HOME to the sandbox, but mise root preservation only checks $HOME/.local/share/mise. Mise’s real default is ${XDG_DATA_HOME:-$HOME/.local/share}/mise, so when MISE_DATA_DIR is unset and mise lives under a custom parent XDG_DATA_HOME, shims lose their installs and fixture setup fails into silent SKIPs — the failure mode TOOLCHAIN_ROOTS is meant to prevent.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit cb1c007. Configure here.
cb1c007 to
b38fe97
Compare
Several suites under crates/socket-patch-cli/tests/ do real installs as
part of their fixture setup — npm, corepack yarn/pnpm, bun, go build,
pip, gem, bundler. None of them are #[ignore]d, so a plain `cargo test`
runs them all, and with no environment of their own each one writes into
the home directory of whoever ran the suite. Measured on macOS, one full
run of the CLI integration tests left 3,601 files there: corepack's
downloaded package managers, Go's build cache, yarn classic's global
cache, bundler's compact index, the RubyGems spec cache.
It also makes results depend on what happened to be lying around: a
fixture install can succeed against something an unrelated run already
cached, then fail on a clean CI runner.
tests/common/cache_env.rs adds `isolate()`, which points one child
process at a sandbox under the OS temp dir. It pins HOME plus every
variable that outranks HOME — GOCACHE (separate from GOMODCACHE and
GOPATH), COREPACK_HOME, PNPM_HOME, npm_config_cache, YARN_*, BUN_*,
CARGO_HOME, PIP_CACHE_DIR, UV_CACHE_DIR, GEM_SPEC_CACHE,
BUNDLE_USER_HOME, COMPOSER_*, NUGET_* — and carries over the version
manager roots (RUSTUP_HOME, RBENV_ROOT, MISE_DATA_DIR, ~/.tool-versions,
…) so a redirected home cannot make the toolchain itself unresolvable.
The sandbox is a stable directory, not a fresh one per run: these
fixtures reinstall the same handful of packages every time. Tests that
need a genuinely cold cache keep passing their own empty directory,
which still wins because `isolate()` runs before the test's own env.
Two things this turned up:
* e2e_vendor_yarn_classic_dev_flow.rs ran its ambient-env scrub AFTER
seeding the caller's YARN_CACHE_FOLDER. The scrub ends with
env_remove("YARN_CACHE_FOLDER"), so the private cache was wiped and
the fixture install silently used the developer's global one (165
files). e2e_vendor_yarn_classic_build.rs documents having fixed the
same bug; this file kept it.
* The `has_command` / `has_corepack_pm` availability probes leaked
more than the installs did. Where pnpm or yarn is a corepack shim,
`pnpm --version` downloads the package manager (~900 files). The
probes are now isolated too, which also makes them answer for the
environment the install will actually run in.
Same measurement after the change: 0 files. No test changed status —
136 test binaries, identical pass/fail before and after.
b38fe97 to
34a64e2
Compare
|
Heads up on the red check: Bundler falls back from the compact index to the dependency API against the gem patch-registry and gets a body it cannot unmarshal ( I A/B'd it on this branch:
The job last passed on |


Running the Rust test suite installs packages into your home directory. Several suites under
crates/socket-patch-cli/tests/do real installs as part of their setup —npm install,corepack yarn install,pnpm install,bun install,go build,pip install,gem install,bundle install— and none of them are#[ignore]d, so a plaincargo testruns every one. With no environment of their own, each writes to whatever cache it finds in$HOME.I measured it by giving the suite a stand-in home directory and counting. One run of these suites left 3,601 files in it: corepack's downloaded package managers, Go's build cache, yarn classic's global cache, bundler's compact index, the RubyGems spec cache. After this change the same run leaves 0.
The second problem is trust. A fixture install can succeed because the package it needs was already sitting in your cache from something unrelated — the kind of pass that disappears on a colleague's laptop or a fresh CI runner.
This is the follow-up #142 promised for the Rust side. That PR does the same job for the shell-driven setup matrix and touches only
tests/setup_matrix/run-case.sh; this one touches onlycrates/, so the two cannot conflict.The helper — one function, called on each package-manager child process
New file
crates/socket-patch-cli/tests/common/cache_env.rs. The whole interface is:It points that one child at a sandbox under the OS temp directory.
common/mod.rsre-exports it, and the self-contained suites (this repo's convention is that e2e files carry their own helpers) pull in just this module with#[path = "common/cache_env.rs"] mod cache_env;.The good pattern already in the repo is the one
e2e_vendor_npm_build.rsande2e_vendor_bun_build.rsuse: hand the fixture install a private cache directory. Both of those measure clean today. What the helper does is generalise it, so a suite gets the whole set by default instead of having to remember one variable at a time.Setting HOME is not enough — every one of these is read in preference to it
A redirected
HOMEalone still leaves the real cache in play whenever one of these is exported — and CI actions export them too (pnpm/action-setupsetsPNPM_HOME). So each is pinned explicitly:npm_config_cachePNPM_HOMEYARN_CACHE_FOLDER,YARN_GLOBAL_FOLDERCOREPACK_HOMEBUN_INSTALL,BUN_INSTALL_CACHE_DIRGOPATH,GOMODCACHEGOCACHECARGO_HOMEPIP_CACHE_DIR,UV_CACHE_DIRGEM_SPEC_CACHE,BUNDLE_USER_HOMECOMPOSER_HOME,COMPOSER_CACHE_DIRNUGET_PACKAGES,NUGET_HTTP_CACHE_PATHXDG_CACHE_HOME,XDG_DATA_HOME,XDG_STATE_HOMEXDG_CONFIG_HOMEis deliberately left alone. It is not a cache, and when someone has set it explicitly it usually points at real configuration — a registry mirror, a corporate CA bundle — that the installs still need.Redirecting HOME can break the toolchain itself — so the version-manager roots are carried over
This is the part that would have bitten people.
rbenv,pyenv,nvm,fnm,volta,asdf,mise,sdkmanandrustupall live under the home directory by default. MoveHOMEwithout carrying them and the shim cannot find its own root, sorubyornodeorcargofails to launch at all.That failure would have been quiet rather than loud: most of these suites respond to a failed fixture install by printing
SKIPand returning, so the coverage would have vanished without anything going red.isolate()therefore seeds each root variable from the real home first — but only when the variable is not already exported and the default directory actually exists, which makes it a no-op on machines and CI runners that do not use that version manager.asdfandmisealso read a global~/.tool-versionsand neither accepts an absolute path to it from the environment, so that one file is copied into the sandbox home.Five self-tests in the module pin this: that every leak-prone variable is still in the list, that no override resolves inside the real home, that
isolate()really applies them to aCommand, and that the root-preservation pass never invents a path. They are pure path arithmetic, so they cost nothing in the binaries that pick the module up. They follow the same "ungated#[test]in a#[path]module" pattern the existingoracle_selftestsincommon/mod.rsuses, and for the same reason: integration-test crates never getcfg(test), so gating them would silently exclude them.Where the call goes — after the ambient scrub, before the test's own env
Command's environment operations are keyed by variable name and the last call for a name wins, so ordering decides who gets the final say:SOCKET_*/npm_config_*/YARN_*prefix scrubs run first — they iterate the parent environment and would otherwise strip the sandbox values right back out;isolate();Step 3 is why the cold-cache tests keep working. Several suites deliberately pass an empty directory to prove a fresh checkout installs the patched bytes —
e2e_vendor_golang_build.rseven asserts its emptyGOMODCACHEstays empty afterwards. Those values are applied last, so they still win.Per-file changes — mostly one line each, in the helper the file already has
common/cache_env.rsisolate(), and the self-tests.common/mod.rsnpm_run/pnpm_run/cargo_runand thehas_commandprobe use it.e2e_golang_build.rs,e2e_vendor_golang_build.rsGOMODCACHEand stop, sogo buildstill filled the real home.e2e_vendor_pypi_build.rse2e_vendor_{npm,bun,pnpm,yarn_berry,yarn_classic,yarn_classic_dev_flow,gem,composer}_build.rse2e_redirect_{npm,bun,rush_sim,yarn_berry,yarn_classic}_build.rse2e_hosted_production.rsin_process_alternate_installers.rs#[ignore]d.in_process_gem_apply.rsgem installran with an empty environment.global_packages_e2e.rse2e_{npm,scan,gem,pypi}.rs#[ignore]d, so not part of a plaincargo test, but the dedicatede2eCI job runs them with--ignored.Two bugs this turned up — a wiped cache override and a probe that leaked more than the installs
e2e_vendor_yarn_classic_dev_flow.rsran its scrub in the wrong order. The scrub ends withenv_remove("YARN_CACHE_FOLDER"), and the helper called it after seeding the caller's private cache — so the override was wiped and every fixture install quietly used the developer's global yarn cache. Measured: 165 files.e2e_vendor_yarn_classic_build.rscarries a comment describing having fixed exactly this; the dev-flow file, added later, kept the bug. The scrub now runs first.The availability probes leaked more than the installs did.
has_command("pnpm")looks harmless, but wherepnpmoryarnis a corepack shim,pnpm --versionmakes corepack download the package manager — around 900 files from a single probe.has_commandandhas_corepack_pmare now isolated too. That has a second benefit: the probe now answers for the same environment the fixture install will run in, instead of a different one.What I checked — before-and-after file counts per ecosystem, on a real run
Method: build the test binaries normally, then run each one with
HOMEpointed at an empty stand-in directory and count the files in it afterwards. That proves the leak and the fix without writing to anyone's actual home. The same binaries were built from the base commit for the "before" column.Ran — every suite this PR touches, before and after:
e2e_golang_builde2e_vendor_golang_builde2e_vendor_pnpm_buildin_process_alternate_installerse2e_vendor_yarn_classic_dev_flowe2e_vendor_yarn_classic_buildin_process_gem_applye2e_vendor_yarn_berry_buildglobal_packages_e2eAnd the sandbox really is where it all went: the Go build cache and corepack directories under it are populated with exactly the content that used to land in the home directory.
Ran — the whole CLI integration suite on both sides, 136 test binaries each. Identical pass/fail, no status changes in either direction. One binary fails on my machine in both arms:
in_process_alternate_installers's bundler leg looks for the gem undervendor/bundle/ruby/<version>/, which is the Bundler 2 layout; Bundler 1.17 puts it atvendor/bundle/. I confirmed it fails identically on the base commit with an untouched home, so it is pre-existing and unrelated.Ran —
cargo clippy --workspace --all-features --all-targetson the pinned toolchain. No new warnings from any file here. (Three pre-existing clippy findings insocket-patch-corereproduce identically on the base commit.)cargo fmt --checkreports no diff in any file this PR touches.Did not run locally —
e2e_hosted_production.rs. It contacts production Socket endpoints and real registries for every ecosystem, so I did not drive production traffic from my machine to verify a cache path. CI runs it, and 13 of its 14 legs pass with the change.The
hosted-e2echeck on this PR is red, and it is not this change. Itsgem_bundler_hosted_redirect_and_known_install_defectleg fails because bundler now falls back from the compact index to the dependency API against the gem patch-registry and gets a body it cannot unmarshal (ArgumentError: marshal data too short). The test is written to tolerate one specific known server defect (APIResponseMismatchError) and this is a different one, so it reports it as a new regression. I A/B'd it: I pushed a revision of this branch withe2e_hosted_production.rsrestored byte-identical tomain, and the same leg failed the same way. The job last passed onmainon 31 July, so something changed on the server side in between. Happy to drop the one-line change to that file if you would rather this PR not touch it at all.Did not run — the Docker e2e suites and the setup matrix. Neither is affected: both already run inside containers whose home is thrown away, and the setup matrix is #142's subject.
Trade-off — the sandbox is a stable directory under the OS temp dir, not a fresh one per run. A throwaway directory would re-download the same handful of fixture packages every single time and buy no extra safety, since the sandbox is outside the home directory either way. The cost is that it persists between runs and grows to a couple of hundred megabytes, mostly the Go build cache and corepack. Note if you ever want to delete it by hand: Go writes its module cache read-only, so a plain
rm -rffails partway through with permission errors — rungo clean -modcacheagainst it first, orchmod -R u+wthe tree. The module docs say so too.Trade-off — the sandbox home means a fixture install no longer reads your
~/.npmrc. That is the intended direction (it is what makes the run reproducible), but if you install through a private registry configured only in your home npmrc, the fixture installs will now go to the public one.Left for a follow-up — a different leak, with a different cause
Two suites still write to the home directory after this change, and the cause is not the tests:
in_process_cargo_applyandin_process_remote_ecosystems_apply(~900 files each, cold). They callapplyin process, and the crawler's global-prefix resolution shells out tonpm/yarn/pnpmitself. Those children inherit the test process's environment, so the only fix is to set the variables on the test process rather than on aCommand.That is a real change of mechanism — this repo has already had to fix races caused by tests mutating the process environment (#135, #136) — so it does not belong in the same PR as a per-
Commandhelper.global_packages_e2e.rsis the one place I did handle it here, and only partly: it spawns the binary as a child, so it can takeCOREPACK_HOMEandnpm_config_cachedirectly. It deliberately does not get the fullisolate(), because resolving the real global prefixes is the entire point of that file and those come out of$HOMEandPNPM_HOME. The two it does take cannot change the answer, and they take it from 907 files to 0.