test(config): cover the _ZO_* env readers with shimforge - #1
Open
tanglearncode wants to merge 4 commits into
Open
tanglearncode wants to merge 4 commits into
tanglearncode wants to merge 4 commits into
Conversation
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
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.
Problem
src/config.rshas no tests. Its seven readers parse and validate real input —_ZO_MAXAGEmust be a number,_ZO_DATA_DIRmust be absolute,_ZO_EXCLUDE_DIRSmust be valid globs — and every error path is uncovered.The reason is the environment itself. Testing these needs a chosen value for one variable, and
env::set_varisunsafein the 2024 edition and writes to the whole process, so parallel tests read each other's values. The usual ways out are an env lock, serialized tests, or changing the functions to take parameters.Changes
config.rs: the_ZO_MAXAGEdefault, a parsed value, a non-numeric value, a relative_ZO_DATA_DIR, and the_ZO_RESOLVE_SYMLINKSopt-in.env::var_osper thread with shimforge instead of writing the environment. Nounsafe, no lock, no serialization, and the tests run in parallel with the rest of the suite.config.rsitself is unchanged — that is the point of the approach.How did you test this code?
Verified on the fork: run 35046566443, green at zoxide's own MSRV, 1.88.0.
--test-threads=8: 5 passed every time. Nothing is serialized and no environment lock is involved.--locked, so the committed lockfile is what cargo resolves, not a hand-patched approximation.cigate passes with the change:cargo fmt --check,clippy --all-features --all-targets -- -Dwarnings,cargo msrv verify,cargo udepsandnextest, all in the repo's nix shell.udepspassing also shows the new dependency is used rather than merely declared.Unchecked: platforms other than x86-64 Linux.
Note on the dependency
shimforge is source-available under PolyForm, not an OSI license. It is free for noncommercial use, which covers a hobby project, and free for companies under 100 people and 1M USD revenue; anything above that needs a commercial license. That is a real consideration for a widely packaged project, since it reaches contributors and packagers who run the suite. This PR is a demonstration of the technique on a fork, not a request to take the dependency.