test(config): cover the four-layer config precedence chain - #76
Merged
Merged
Conversation
src/config/mod.rs had zero tests. Adds 12 covering DaemonConfig::new, which
layers CLI/env args > process environment > {data_dir}/config.toml >
built-in defaults. The mutants that matter here reorder an .or() chain, swap
a default constant, or drop an .is_empty() filter, so every field is
exercised at more than one layer — a test that only checks the default
cannot tell a correct chain from one that ignores its overrides.
Environment safety: these mutate process-global state and Rust runs tests in
parallel threads. This follows the idiom already established in
session::cursor::tests (a module lock held across the whole sequence,
tolerating a poisoned mutex) and adds a Drop-based guard that CLEARS all
eight CLAWD_* variables on entry and restores them on drop, including on
panic. Without the clear, 'the default applies' tests would fail on any
machine that happens to export CLAWD_API_URL. Verified by running the full
706-test lib suite twice with no failures.
Verified by hand-applying six mutants and confirming each fails a test:
DEFAULT_PORT 4300 -> 8080, default_bind_address -> 0.0.0.0, port precedence
flipped to toml.port.or(port), bind precedence putting env before the CLI
arg, dropping the is_empty filter on CLAWD_REGISTRY_URL, and the log default
info -> warn. All six were caught.
One finding recorded rather than changed: api_base_url and relay_url are the
only two env reads WITHOUT a .filter(|s| !s.is_empty()), so CLAWD_API_URL=''
beats the TOML value and blanks the URL, where CLAWD_REGISTRY_URL='' falls
through to the next layer. A test pins the asymmetry so it cannot drift
silently; adding the filter would change startup behaviour and is a decision
rather than a coverage change.
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.
src/config/mod.rshad zero tests. Adds 12 coveringDaemonConfig::new, which layers CLI/env args > process environment >{data_dir}/config.toml> built-in defaults.The mutants that matter here reorder an
.or()chain, swap a default constant, or drop an.is_empty()filter — so every field is exercised at more than one layer. A test that only checks the default cannot tell a correct chain from one that ignores its overrides.Environment safety
These mutate process-global state, and Rust runs tests in parallel threads. This follows the idiom already established in
session::cursor::tests(a module lock held across the whole sequence, tolerating a poisoned mutex) and adds aDrop-based guard that clears all eightCLAWD_*variables on entry and restores them on drop, including on panic.Without the clear, a "the default applies" test would fail on any machine that happens to export
CLAWD_API_URL— an environment-dependent test is its own kind of hollow gate. Verified by running the full 706-test lib suite twice with zero failures.Verification
Hand-applied six mutants; each fails a test:
DEFAULT_PORT4300 → 8080default_bind_address→0.0.0.0(LAN exposure)toml.port.or(port)is_emptyfilter onCLAWD_REGISTRY_URLinfo→warnclippy --all-targets --all-features -- -D warningsandcargo fmt --checkboth exit 0.One finding recorded, not changed
api_base_urlandrelay_urlare the only two env reads without a.filter(|s| !s.is_empty()):""CLAWD_API_URLCLAWD_RELAY_URLCLAWD_REGISTRY_URL,CLAWD_BIND,CLAWD_UPDATE_POLICY,CLAWD_LOG_FORMAT,CLAWD_LICENSE_TOKEN,CLAWD_API_TOKENAn unset shell variable expands to
"", soCLAWD_API_URL=$SOME_UNSET_VARsilently points the daemon at an empty API URL. A test pins the current asymmetry so it cannot drift silently; I did not add the filter, since that changes startup behaviour for anyone relying on it. Worth a decision.