CodeRabbit Generated Unit Tests: Add unit tests for PR changes - #16
coderabbitai[bot] wants to merge 1 commit into
Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
canstralian
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I’m going to close this PR rather than merge it in its current form.
There are a few issues that make the generated test suite too brittle and noisy for the repository right now:
- The PR includes an unintended file (=8.0) containing pip warning output.
- Several tests hardcode implementation details (exact vector counts, IDs, states, contract names, etc.) instead of validating durable invariants.
- Some tests appear to assume base64-encoded YAML while other parts of the suite parse the same files directly as YAML, which suggests the generator may have misunderstood the repository structure.
- The signal-to-noise ratio is too low relative to the maintenance burden these tests would introduce.
There are still useful ideas here worth preserving:
- validating set -euo pipefail
- ensuring dry-run behavior is safe
- checking registry structure and required fields
- verifying subtree commands are not executed during dry-run flows
I’ll likely replace this with a smaller hand-written invariant-focused test set that aligns more closely with the repo’s actual control-plane and drift assumptions.
Appreciate the effort and automation experimentation regardless.
Cherry-pick the worthwhile bits from CodeRabbit's auto-generated #16: - tests/test_import_vectors.py: regression coverage for the dry-run safety contract — including the two specific bugs fixed in this branch (the unquoted "EXECUTE" guard and the wrong `${entry#:*}` URL extraction). Skips the trivial filesystem-stat tests #16 included. - tests/test_scope_mapper.py: replace the single is_authorized check with a parametrized matrix covering empty / numeric / special-char / IPv4 / wildcard / unicode / oversized inputs, and also assert the return is a real bool. Dropping #16's test_contracts.py rewrite (asserts a hallucinated diff that base64-decodes the YAML files and checks for vector deletions that never happened in this PR) and the `=8.0` artifact (shell redirection accident from `pip install pytest >= 8.0`).
Unit test generation was requested by @canstralian.
The following files were modified:
=8.0tests/test_contracts.pytests/test_import_vectors.pytests/test_scope_mapper.py