test: close the coverage gap and enforce the documented 100% target - #7
Conversation
CLAUDE.md documents 100% coverage; the suite actually sat at 99%, with five uncovered statements in checker.py and no --cov-fail-under to catch the drift. CI would have stayed green all the way down. Adds tests for both uncovered paths, which are real error branches rather than unreachable code: - a malformed root-anchors.xml body. Distinct from the network failure already covered: that one raises out of the fetch try/except, this one reaches the parse handler and reports a different message. - an insecure delegation that leaves the target zone with no validated keys, where check() must finalise and return None instead of calling _check_final_rrset with a None key set. Then enforces it with --cov-fail-under=100, so the documented bar is a gate rather than a claim. Also corrects the README test count, which said 274 against an actual 285 — stale by 11 before this change.
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request adds two ChangesChecker Coverage
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds tests for the remaining uncovered
Confidence Score: 4/5The coverage enforcement should be scoped to full-suite runs, or the documented targeted commands should explicitly disable the threshold, before merging. Global pytest addopts now require every partial test selection to cover the entire package, so documented single-file and single-test workflows exit unsuccessfully even when all selected tests pass. Files Needing Attention: pyproject.toml, README.md
|
| Filename | Overview |
|---|---|
| pyproject.toml | Adds a global 100% coverage threshold that also breaks documented partial-test invocations. |
| tests/test_checker.py | Adds focused tests for malformed trust-anchor XML and the top-level no-target-keys return path. |
| README.md | Updates the test count, although its documented targeted commands now conflict with the global coverage threshold. |
| CHANGELOG.md | Documents the new tests, coverage enforcement, and corrected test count. |
Reviews (1): Last reviewed commit: "test: close the coverage gap and enforce..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Line 88: Update the pytest configuration so --cov-fail-under=100 is not
applied through the global addopts used by focused test commands. Keep coverage
collection/reporting as appropriate, and apply the 100% threshold only in the
full-suite CI invocation or document a focused-test override that disables it.
In `@tests/test_checker.py`:
- Around line 1293-1308: Update the test around check() to spy on c._finalise,
include that patch in the existing mock context, and assert it is called after
check() completes. Keep the current result and _check_final_rrset assertions
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8691070-ccd9-4205-8cc4-00b53dd214e5
📒 Files selected for processing (4)
CHANGELOG.mdREADME.mdpyproject.tomltests/test_checker.py
Review caught that putting --cov-fail-under=100 in pyproject addopts
applies it to every pytest invocation, not just full runs. A targeted
command from CLAUDE.md such as
pytest tests/test_checker.py::TestCheckerCheck -v
measures the whole package while executing one class, so it would exit
non-zero even with every selected test passing. That is a regression in
the documented developer workflow, introduced by this branch.
The gate now lives in the workflow's pytest step: CI still enforces 100%,
targeted runs work again. Verified both -- targeted run passes, full run
reports "Required test coverage of 100% reached".
Second finding, also valid: the new insecure-delegation test asserted
only that check() returned None and did not call _check_final_rrset. It
would therefore have passed had check() returned None without finalising,
leaving report status unset. Adds a _finalise spy and asserts it is
called once.
There was a problem hiding this comment.
t0kubetsu has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
1 participant
What
Adds two
checker.pytests, enforces--cov-fail-under=100, and corrects a stale README count.Why
CLAUDE.mddocuments 100% coverage. The suite actually sat at 99% — five uncovered statements inchecker.py— andaddoptsonly reported coverage, so nothing enforced the documented bar. CI would have stayed green while coverage drifted arbitrarily far down.Both gaps are real error branches, not unreachable code:
checker.py:405-407root-anchors.xml. Distinct from the network failure already tested — that raises out of the fetchtry, this reaches the parse handler and reports a different message.checker.py:205-206check()must finalise and returnNonerather than call_check_final_rrsetwith aNonekey set.Also
The README badge and prose both claimed 274 tests against an actual 283 before this change — stale by 9, and now corrected to 285.
Verified
285 passed,Required test coverage of 100% reached. Total coverage: 100.00%, ruff clean.Summary by CodeRabbit
Bug Fixes
Tests
Documentation