Add property-based tests (proptest) exhaustively verifying config-precedence invariant across all 8 selector combinations

[coderabbitai]

Testing (Property / Proof)

Context

PR #285 (#285) implements a four-level config precedence order:

--config  >  NETSUKE_CONFIG  >  NETSUKE_CONFIG_PATH  >  automatic discovery

The three binary selectors (cli.config present/absent, NETSUKE_CONFIG set/unset, NETSUKE_CONFIG_PATH set/unset) produce 8 combinations. The current test suite covers 5 of these states with named integration tests. The remaining 3 are untested, and no framework enforces the precedence invariant exhaustively across all combinations.

Required work

1. Add proptest to Cargo.toml (or use an equivalent property-based testing crate already present).

2. Implement a property-based test that generates arbitrary combinations of the three selectors and verifies the invariant:

   When selector S₁ is present, the resolved config is always the one selected by S₁, regardless of the values of lower-priority selectors S₂ and S₃.

3. Alternatively, enumerate all 8 states in a regression-prevention matrix (e.g. rstest parametric cases) with a documented proof assumption explaining why exhaustive property testing is not required.

Acceptance criteria

• All 8 selector combinations are covered, either by generated or enumerated cases.
• The precedence invariant is expressed as a testable predicate, not just as prose.

Raised by

Requested by @leynos as a follow-up to #285.

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.\n\nOnce a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch issue `#291` details
gh issue view 291 --repo leynos/netsuke --json title,body,labels,state,comments

echo "---LABELS---"
gh label list --repo leynos/netsuke

Length of output: 3596

---

🏁 Script executed:

#!/bin/bash
# Check Cargo.toml for proptest/rstest dependencies
fd -e toml -i 'cargo' --exec cat {}

echo "=== CONFIG-RELATED SOURCE FILES ==="
fd -e rs | xargs grep -l -i 'config' 2>/dev/null

echo "=== TEST FILES ==="
fd -e rs --search-path . | xargs grep -l '#\[test\]\|#\[cfg(test)\]\|proptest\|rstest' 2>/dev/null

Length of output: 14908

---

🏁 Script executed:

#!/bin/bash
echo "=== tests/cli_tests/config_selection.rs ==="
cat tests/cli_tests/config_selection.rs

echo ""
echo "=== tests/cli_tests/config_discovery.rs ==="
cat tests/cli_tests/config_discovery.rs

echo ""
echo "=== Check proptest usage across test files ==="
rg -n 'proptest' --type rust

Length of output: 36927

---

🏁 Script executed:

#!/bin/bash
# Apply the 'low' label to issue `#291`
gh issue edit 291 --repo leynos/netsuke --add-label "low"

Length of output: 197

---

---

The low label has been applied to #291.

To summarise the assessment:

Status: still open — code changes required

Two of the eight (--config, NETSUKE_CONFIG, NETSUKE_CONFIG_PATH) selector combinations remain untested:

• (Y, Y, Y) — all three selectors active simultaneously
• (Y, N, Y) — --config and NETSUKE_CONFIG_PATH active, NETSUKE_CONFIG absent

proptest has been added to [dev-dependencies] but is not yet wired up to the config-precedence invariant. The rstest matrix approach is also acceptable per the issue's acceptance criteria, provided all 8 states are enumerated.

Criticality: low

The implementation is almost certainly correct — both missing combinations are straightforward extensions of already-verified cases. The risk is indirect: reduced regression protection rather than a known user-facing defect. The blast radius is confined to developer and CI confidence in the config-selection pipeline.

Impact of the required changes:

• Add two #[case] entries to config_selection_precedence_cases in tests/cli_tests/config_selection.rs for the two missing combinations, or implement a proptest! block that generates all three-selector booleans and asserts the highest-priority present selector always wins.
• Low effort; no changes to production code are needed.

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

• Extend the existing resolve_config_path_precedence rstest in src/cli/config_merge_tests.rs with 4 additional cases to cover all 8 selector combinations
• Add a proptest using arbitrary path generation to verify the precedence invariant holds across varied inputs
• Document the invariant formally as a testable predicate within the test module

Design Choices

Design Choice 1: Testing layer selection

Options Considered:

1. Proptest only — generates arbitrary combinations and path values, verifying the invariant holds for all generated inputs
2. Rstest exhaustive enumeration only — adds the 4 missing cases to the existing parametric test, documenting the proof assumption
3. Both approaches combined — completes rstest enumeration for regression clarity and adds proptest for input-space exploration

Chosen Option: 3

Rationale: Both approaches provide complementary value — rstest gives explicit, documented coverage of all 8 states whilst proptest validates the invariant with varied path values and edge cases.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Exhaustive Selector Coverage and Property-Based Verification

This phase extends the existing unit test suite in src/cli/config_merge_tests.rs to achieve complete coverage of all 8 selector combinations and adds a proptest that formally encodes the precedence invariant as a testable predicate.

Task 1: Complete rstest Enumeration for All 8 Combinations

Add the 4 missing cases to the existing resolve_config_path_precedence rstest parametric test to form a complete truth table.

• Add case all_absent_returns_none — cli absent, env absent, legacy absent → expects None
• Add case cli_only_uses_cli — cli present, env absent, legacy absent → expects cli path
• Add case cli_wins_over_legacy_alone — cli present, env absent, legacy present → expects cli path
• Add case env_only_uses_env — cli absent, env present, legacy absent → expects env path
• Add a doc-comment above the test function expressing the precedence invariant formally and noting that the 8 cases constitute exhaustive enumeration of the 2³ selector states

Task 2: Implement Proptest for Precedence Invariant

Add a new proptest-based test alongside the existing rstest cases that generates arbitrary selector combinations with arbitrary path values and verifies the invariant holds.

• Define a proptest strategy that generates Option<PathBuf> for each of the three selectors (cli, env, legacy), using proptest::option::of with a non-empty string strategy mapped to PathBuf
• Implement the invariant predicate as a pure function: given (cli, env, legacy), return the expected winner according to precedence rules
• Within the proptest! block, invoke resolve_config_path with a mock Cli and a closure-based fake var_os that returns the generated env values
• Use prop_assert_eq! to compare the actual result against the independently computed expected value
• Follow the established pattern from tests/release_help/script_functions.rs for error handling (TestCaseError::fail) and assertion style

Task 3: Document the Invariant as a Testable Predicate

Add a module-level doc-comment or inline documentation that expresses the precedence invariant in formal terms.

• State the invariant: "For selectors S₁ (cli.config), S₂ (NETSUKE_CONFIG), S₃ (NETSUKE_CONFIG_PATH), the resolved path is S₁ if present, else S₂ if present, else S₃ if present, else None"
• Reference both the rstest exhaustive enumeration and the proptest as implementations of this invariant
• Place this documentation in src/cli/config_merge_tests.rs near the relevant tests

🤖 Prompt for AI agents

Extend `src/cli/config_merge_tests.rs` to achieve full coverage of all 8
selector combinations and add a proptest that formally encodes the precedence
invariant as a testable predicate.

**Task 1: Complete rstest Enumeration**
- Locate the existing `resolve_config_path_precedence` rstest parametric test in
`src/cli/config_merge_tests.rs`
- Add the following 4 missing `#[case]` entries to form a complete 2³ truth
table:
- `all_absent_returns_none` — cli absent, env absent, legacy absent → expects
`None`
- `cli_only_uses_cli` — cli present, env absent, legacy absent → expects cli
path
- `cli_wins_over_legacy_alone` — cli present, env absent, legacy present →
expects cli path
- `env_only_uses_env` — cli absent, env present, legacy absent → expects env
path
- Add a doc-comment directly above the test function that states the precedence
invariant formally and notes that the 8 cases constitute exhaustive enumeration
of the 2³ selector states

**Task 2: Implement Proptest**
- Add a new proptest-based test function in `src/cli/config_merge_tests.rs`
alongside the existing rstest cases
- Define a proptest strategy generating `Option<PathBuf>` for each of the three
selectors (cli, env, legacy) using `proptest::option::of` with a non-empty
string strategy mapped to `PathBuf`
- Implement the invariant predicate as a pure function: given `(cli, env,
legacy)`, return the expected winner according to the precedence rules
- Inside the `proptest!` block, invoke `resolve_config_path` with a mock `Cli`
and a closure-based fake `var_os` returning the generated env values
- Use `prop_assert_eq!` to compare the actual result against the independently
computed expected value
- Mirror the error handling (`TestCaseError::fail`) and assertion style
established in `tests/release_help/script_functions.rs`

**Task 3: Document the Invariant**
- Add a module-level doc-comment or inline comment in
`src/cli/config_merge_tests.rs` near the relevant tests
- State the invariant: "For selectors S₁ (cli.config), S₂ (NETSUKE_CONFIG), S₃
(NETSUKE_CONFIG_PATH), the resolved path is S₁ if present, else S₂ if present,
else S₃ if present, else None"
- Reference both the rstest exhaustive enumeration and the proptest as
implementations of this invariant

Research

The config selection logic resides in src/cli/config_merge.rs, with the private resolve_config_path function implementing the precedence chain. Unit tests in src/cli/config_merge_tests.rs access this function via use super::* and use rstest parametric cases. The existing resolve_config_path_precedence test covers 4 of the 8 possible combinations. Proptest is already declared in Cargo.toml under dev-dependencies and has an established usage pattern in tests/release_help/script_functions.rs.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Extend `src/cli/config_merge_tests.rs` to achieve full coverage of all 8
selector combinations and add a proptest that formally encodes the precedence
invariant as a testable predicate.

**Task 1: Complete rstest Enumeration**
- Locate the existing `resolve_config_path_precedence` rstest parametric test in
`src/cli/config_merge_tests.rs`
- Add the following 4 missing `#[case]` entries to form a complete 2³ truth
table:
- `all_absent_returns_none` — cli absent, env absent, legacy absent → expects
`None`
- `cli_only_uses_cli` — cli present, env absent, legacy absent → expects cli
path
- `cli_wins_over_legacy_alone` — cli present, env absent, legacy present →
expects cli path
- `env_only_uses_env` — cli absent, env present, legacy absent → expects env
path
- Add a doc-comment directly above the test function that states the precedence
invariant formally and notes that the 8 cases constitute exhaustive enumeration
of the 2³ selector states

**Task 2: Implement Proptest**
- Add a new proptest-based test function in `src/cli/config_merge_tests.rs`
alongside the existing rstest cases
- Define a proptest strategy generating `Option<PathBuf>` for each of the three
selectors (cli, env, legacy) using `proptest::option::of` with a non-empty
string strategy mapped to `PathBuf`
- Implement the invariant predicate as a pure function: given `(cli, env,
legacy)`, return the expected winner according to the precedence rules
- Inside the `proptest!` block, invoke `resolve_config_path` with a mock `Cli`
and a closure-based fake `var_os` returning the generated env values
- Use `prop_assert_eq!` to compare the actual result against the independently
computed expected value
- Mirror the error handling (`TestCaseError::fail`) and assertion style
established in `tests/release_help/script_functions.rs`

**Task 3: Document the Invariant**
- Add a module-level doc-comment or inline comment in
`src/cli/config_merge_tests.rs` near the relevant tests
- State the invariant: "For selectors S₁ (cli.config), S₂ (NETSUKE_CONFIG), S₃
(NETSUKE_CONFIG_PATH), the resolved path is S₁ if present, else S₂ if present,
else S₃ if present, else None"
- Reference both the rstest exhaustive enumeration and the proptest as
implementations of this invariant

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!