test(host-runtime): write test keystores to the temp dir, not the real one - #1850
Virgile-pct wants to merge 2 commits into
Conversation
…l one Six tests wrote their owner keystore fixture to `default_keystore_path()`, which resolves through `dirs::home_dir()` in `mesh-llm-identity` and has no test override. On Windows that is the developer's real `~/.mesh-llm/owner-keystore.json`, so running the suite replaced it with a fresh `owner_id` and two fresh key pairs, six of its fifteen fields. #1847 stopped the same suite from deleting the node key, and measured this one as still outstanding. The `#[cfg(test)]` hook that protects the node key lives in host-runtime, so it cannot reach a path resolved inside `mesh-llm-identity`. Each of the six call sites already had a `tempfile::tempdir()` on the line above and already handed the path to `state.set_owner_key_path()`, so the default location was only ever a place to write the fixture. They now write into that temp directory instead. Measured on Windows 11 against `3f4f1c35a`, comparing `~/.mesh-llm` before and after a full `cargo test -p mesh-llm-host-runtime`: the keystore hash is now unchanged, as is the node key. 3457 passed and 17 failed, the same 17 as the parent commit, compared by diffing both complete sorted lists. One write into the real home remains: the skippy hash cache still lands in `~/.mesh-llm/cache/hashes` because tests leave `MESH_LLM_HASH_CACHE_DIR` unset. That is a cache rather than identity material, and it is not addressed here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughAPI tests now save owner keystores under temporary directories instead of using the default keystore path. The test module removes the unused ChangesTemporary keystore test updates
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change prevents these tests from modifying the developer’s real owner keystore without altering their intended behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
i386
left a comment
There was a problem hiding this comment.
Reviewed the synchronized exact head. Windows keystore tests use isolated temporary paths without changing production behavior; the full host-runtime package suite passes locally.
Follow-up to #1847, which stopped this suite from deleting the node key and measured the owner keystore as still outstanding. This closes that one.
What happens today
Six tests write their keystore fixture to
default_keystore_path():default_keystore_path()resolves throughdirs::home_dir()inmesh-llm-identityand has no test override. The#[cfg(test)]hook that protects the node key lives in host-runtime, so it cannot reach a path resolved inside another crate, andHomeEnvGuardhas no effect on it.Measured on Windows 11 against
3f4f1c35a, one fullcargo test -p mesh-llm-host-runtime, comparing~/.mesh-llm/owner-keystore.jsonbefore and after: same size, same fifteen fields, six of them different.owner_id,created_at, and both the signing and encryption key pairs. The developer's owner identity is replaced by running the suite.What this changes
Each of the six call sites already had a
tempfile::tempdir()on the line above, and each already handed the path tostate.set_owner_key_path(). The default location was only ever somewhere to put the fixture; the code under test reads whatever path it is given. They now write into that temp directory.Six lines, plus one import that became unused.
Measured effect
Same box, same command, comparing
~/.mesh-llmbefore and after:keyowner-keystore.jsonThe 17 failures are identical to the parent commit, compared by diffing both complete sorted lists rather than by eye. No new failures, and none of the six tests changed state: they passed before and still pass, they were simply passing while overwriting the developer's identity.
What still remains
The skippy hash cache still writes into
~/.mesh-llm/cache/hashes, six files per run, because tests leaveMESH_LLM_HASH_CACHE_DIRunset andhash_cache.rsfalls back to the home. That is a cache rather than identity material, so I left it out of this branch rather than mixing a third concern in. Happy to take it next if you want the suite to touch nothing at all.Gates
cargo test -p mesh-llm-host-runtime: 3457 passed, 17 failed, as above.cargo fmt --all --check: clean.cargo clippy -p mesh-llm-host-runtime --all-targets: 8 lints, and the complete sorted set is identical tomain.cargo run -p xtask -- repo-consistency no-console-print: passed.scripts/check-env-mutation-contract.pyandpython -m unittest scripts.tests.test_env_mutation_contract: passed, census unchanged at 234 sites.Summary by CodeRabbit