Skip to content

Verify config precedence invariant (#291)#327

Draft
lodyai[bot] wants to merge 1 commit into
mainfrom
issue-291-property-tests-verifying-config-precedence-invariant
Draft

Verify config precedence invariant (#291)#327
lodyai[bot] wants to merge 1 commit into
mainfrom
issue-291-property-tests-verifying-config-precedence-invariant

Conversation

@lodyai
Copy link
Copy Markdown
Contributor

@lodyai lodyai Bot commented May 31, 2026

Summary

Closes #291

  • Move explicit config path precedence tests into a focused test module.
  • Enumerate all 8 --config / NETSUKE_CONFIG / NETSUKE_CONFIG_PATH selector states.
  • Add a proptest predicate that verifies the highest-priority present selector wins for generated path values.
  • Add the two missing integration precedence cases for --config with NETSUKE_CONFIG_PATH and with both config environment variables.

Validation

  • make check-fmt
  • make lint
  • make test
  • coderabbit review --agent exited successfully after setup/status output with no concerns reported.

Summary by Sourcery

Verify and enforce the precedence of CLI and environment-based config selectors.

Tests:

  • Add focused unit test module for explicit config path selector precedence, including exhaustive cases for all CLI/env selector combinations.
  • Introduce a proptest-based property test to ensure resolve_config_path always respects the defined precedence invariant across generated paths.
  • Extend CLI integration tests to cover --config precedence over NETSUKE_CONFIG and NETSUKE_CONFIG_PATH combinations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 31, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8e052274-04c5-4de7-852f-d18a45e266c8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-291-property-tests-verifying-config-precedence-invariant

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 31, 2026

Reviewer's Guide

Refactors and strengthens configuration path precedence testing by moving explicit precedence tests into a dedicated module, exhaustively enumerating selector combinations (CLI, NETSUKE_CONFIG, NETSUKE_CONFIG_PATH), and adding both example-based and property-based tests to ensure the invariant that higher-priority selectors always win, while plugging two missing integration cases.

Flow diagram for config selector precedence invariant

flowchart TD
    A[Start config selection] --> B{--config present?}
    B -->|yes| C[Use CLI --config path]
    B -->|no| D{NETSUKE_CONFIG present?}
    D -->|yes| E[Use NETSUKE_CONFIG path]
    D -->|no| F{NETSUKE_CONFIG_PATH present?}
    F -->|yes| G[Use NETSUKE_CONFIG_PATH path]
    F -->|no| H[Use default config path]
    C --> I[Pass selected path to merge_with_config]
    E --> I
    G --> I
    H --> I
    I --> J[End config selection]
Loading

File-Level Changes

Change Details Files
Extract config path precedence tests into a focused unit test module and wire it into the CLI config merge code.
  • Remove resolve_config_path precedence tests from the existing config_merge_tests module.
  • Introduce a new config_path_precedence_tests module containing dedicated helpers and precedence tests for resolve_config_path.
  • Register the new config_path_precedence_tests module under cfg(test) in config_merge.rs so tests compile and run.
src/cli/config_merge_tests.rs
src/cli/config_merge.rs
src/cli/config_path_precedence_tests.rs
Exhaustively test precedence behavior for CLI, primary env, and legacy env config selectors using table-driven rstest cases.
  • Implement TestEnv helper to simulate environment variables without touching the real process environment.
  • Add resolve_config_path_with_selectors helper to construct Cli and env inputs from selector presence and call resolve_config_path.
  • Add rstest cases that cover all 2^3 combinations of presence/absence for CLI, NETSUKE_CONFIG, and NETSUKE_CONFIG_PATH, asserting the correct winning path or None.
src/cli/config_path_precedence_tests.rs
Add a property-based test to validate the precedence invariant across arbitrary path values.
  • Implement precedence_winner helper that encodes the intended precedence rule (CLI over env over legacy).
  • Define a path_selector proptest strategy that generates valid path-like strings and wraps them as optional PathBufs.
  • Add a proptest that generates random combinations of selectors and asserts resolve_config_path matches precedence_winner for all cases.
src/cli/config_path_precedence_tests.rs
Augment integration-style config selection tests to cover missing cases where the CLI --config flag competes with env-based selectors.
  • Add a case ensuring --config overrides NETSUKE_CONFIG_PATH when both are provided.
  • Add a case ensuring --config overrides both NETSUKE_CONFIG and NETSUKE_CONFIG_PATH when all selectors are present.
  • Reuse existing ConfigSelectionCase helpers to assert the resulting ThemePreference and description strings.
tests/cli_tests/config_selection.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#291 Add a property-based testing framework (proptest or equivalent) to the project and wire it into the test suite.
#291 Implement a property-based test that generates arbitrary combinations of the three selectors (cli.config, NETSUKE_CONFIG, NETSUKE_CONFIG_PATH) and verifies that the highest-priority present selector always determines the resolved config path.
#291 Ensure all 8 selector presence combinations are exhaustively tested (either via property-based tests or explicit enumeration), and express the precedence invariant as a testable predicate rather than only prose.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-delta-analysis[bot]

This comment was marked as outdated.

Add an exhaustive selector matrix for `--config`, `NETSUKE_CONFIG`,
and `NETSUKE_CONFIG_PATH`, and express the expected winner as a pure
predicate checked by `proptest`.

Move the focused precedence tests into their own module so the existing
config merge test file stays under the repository line-count limit.
@lodyai lodyai Bot force-pushed the issue-291-property-tests-verifying-config-precedence-invariant branch from 7f861a2 to c645462 Compare June 1, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant