From 92ae08ec3639848f0ec67a0f9de3730534824f99 Mon Sep 17 00:00:00 2001 From: t0kubetsu Date: Wed, 12 Aug 2026 16:05:26 +0200 Subject: [PATCH 1/2] test: close the coverage gap and enforce the documented 100% target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 16 +++++++++++++++ README.md | 4 ++-- pyproject.toml | 2 +- tests/test_checker.py | 47 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c947e8..b874857 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ Version numbers follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html ## [Unreleased] +### Added + +- Two `checker.py` tests covering the last uncovered paths: a malformed + `root-anchors.xml` body (distinct from the network failure already tested — + it reaches the parse `try`, not the fetch one), and an insecure delegation + leaving the target zone with no validated keys, where `check()` must finalise + and return `None` rather than validate the final RRset against `None`. + +### Changed + +- `pyproject.toml`: `--cov-fail-under=100` now enforces the coverage target + CLAUDE.md documents. Coverage was reported but never gated, so the suite sat + at 99% against a documented 100% with nothing to catch it. +- `README.md`: test count corrected to 285 (the badge and prose both still + claimed 274, stale by 11 before this change). + --- ## [0.1.6] — 2026-07-08 diff --git a/README.md b/README.md index 0a2f264..bf0fa9b 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ $ chainvalidator check example.com ``` ![Python](https://img.shields.io/badge/python-%3E%3D3.11-blue) -![Tests](https://img.shields.io/badge/tests-274%20passing-brightgreen) +![Tests](https://img.shields.io/badge/tests-285%20passing-brightgreen) ![Coverage](https://img.shields.io/badge/coverage-100%25-brightgreen) ![License](https://img.shields.io/badge/license-GPLv3-lightgrey) @@ -216,7 +216,7 @@ pytest tests/test_checker.py pytest tests/test_checker.py::TestValidateNsec3Nxdomain -v ``` -The test suite has **274 tests** and achieves **100% coverage** of all +The test suite has **285 tests** and achieves **100% coverage** of all testable code. The one `# pragma: no cover` annotation marks a defensive guard inside the `validate_nsec3_rrset` closure in `_validate_nsec3_nxdomain` — it is structurally unreachable because the closure is only ever called with diff --git a/pyproject.toml b/pyproject.toml index ebe9e7f..1f773d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,4 +85,4 @@ select = ["E4", "E7", "E9", "F"] [tool.pytest.ini_options] pythonpath = ["."] testpaths = ["tests"] -addopts = "--cov=chainvalidator --cov-report=term-missing" +addopts = "--cov=chainvalidator --cov-report=term-missing --cov-fail-under=100" diff --git a/tests/test_checker.py b/tests/test_checker.py index 5267bb5..9c855c5 100644 --- a/tests/test_checker.py +++ b/tests/test_checker.py @@ -312,6 +312,18 @@ def test_network_failure_records_error_and_returns_empty(self): assert ds_list == [] assert len(c.errors) == 1 + def test_malformed_xml_records_parse_error_and_returns_empty(self): + # The fetch succeeds but the body is not parseable XML, which is a + # distinct failure from the network error above: it reaches the parse + # try/except rather than the fetch one. + c = _make_checker() + mock_resp = MagicMock() + mock_resp.content = b"truncated" + with patch("requests.get", return_value=mock_resp): + ds_list = c._load_trust_anchor() + assert ds_list == [] + assert any("Failed to parse root-anchors.xml" in e for e in c.errors) + def test_expired_key_digest_skipped(self): c = _make_checker() mock_resp = MagicMock() @@ -1260,6 +1272,41 @@ def mock_check_final(zone, keys, **kwargs): patch.object(c, "_check_final_rrset", side_effect=mock_check_final), ) + def test_unsigned_target_zone_returns_none(self): + # An insecure delegation leaves the target zone with no validated keys. + # check() must finalise and report insecure rather than attempting the + # final RRset validation with a None key set. + c = _make_checker() + dnskey_rr = make_dnskey_rrset(".") + + def mock_build(fqdn): + c._zone_ns_map = { + ".": [("a.root", "1.1.1.1")], + "example.com.": [("ns1.example.com.", "2.2.2.2")], + } + return [".", "example.com."] + + def mock_check_root(ta, validated): + validated["."] = dnskey_rr + return True + + def mock_check_zone(**kwargs): + # Delegation proven unsigned: succeeds without recording keys for + # the child, exactly as _handle_insecure_delegation leaves it. + return True + + with ( + patch.object(c, "_build_zone_list", side_effect=mock_build), + patch.object(c, "_load_trust_anchor", return_value=[MagicMock()]), + patch.object(c, "_check_root", side_effect=mock_check_root), + patch.object(c, "_check_zone", side_effect=mock_check_zone), + patch.object(c, "_check_final_rrset") as final_rrset, + ): + result = c.check() + + assert result is None + final_rrset.assert_not_called() + def test_trust_anchor_failure_returns_false(self): c = _make_checker() From 6bb8f227720ceafbc2fdf97f0a20373513028280 Mon Sep 17 00:00:00 2001 From: t0kubetsu Date: Wed, 12 Aug 2026 16:13:21 +0200 Subject: [PATCH 2/2] fix: move the coverage gate to CI; assert the insecure path finalises 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. --- .github/workflows/ci.yml | 7 ++++++- CHANGELOG.md | 9 ++++++--- pyproject.toml | 2 +- tests/test_checker.py | 4 ++++ 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd61a7b..9bd8fc9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,4 +52,9 @@ jobs: - name: Test # Network I/O is isolated in the *_utils modules and mocked there, so # the suite needs neither a live network nor any external binary. - run: pytest --tb=short -q + # + # --cov-fail-under lives here rather than in pyproject addopts: in + # addopts it also fires on targeted runs like + # `pytest tests/test_checker.py::TestX -v`, which measure the whole + # package while running one class and would always fail the gate. + run: pytest --tb=short -q --cov-fail-under=100 diff --git a/CHANGELOG.md b/CHANGELOG.md index b874857..eb4aa96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,12 @@ Version numbers follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html ### Changed -- `pyproject.toml`: `--cov-fail-under=100` now enforces the coverage target - CLAUDE.md documents. Coverage was reported but never gated, so the suite sat - at 99% against a documented 100% with nothing to catch it. +- CI now enforces the 100% coverage target CLAUDE.md documents. Coverage was + reported but never gated, so the suite sat at 99% against a documented 100% + with nothing to catch it. The gate lives in the workflow's pytest step rather + than in `addopts`, because in `addopts` it also fires on targeted runs such as + `pytest tests/test_checker.py::TestX -v`, which measure the whole package + while running one class and would fail the gate every time. - `README.md`: test count corrected to 285 (the badge and prose both still claimed 274, stale by 11 before this change). diff --git a/pyproject.toml b/pyproject.toml index 1f773d0..ebe9e7f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,4 +85,4 @@ select = ["E4", "E7", "E9", "F"] [tool.pytest.ini_options] pythonpath = ["."] testpaths = ["tests"] -addopts = "--cov=chainvalidator --cov-report=term-missing --cov-fail-under=100" +addopts = "--cov=chainvalidator --cov-report=term-missing" diff --git a/tests/test_checker.py b/tests/test_checker.py index 9c855c5..039f971 100644 --- a/tests/test_checker.py +++ b/tests/test_checker.py @@ -1301,11 +1301,15 @@ def mock_check_zone(**kwargs): patch.object(c, "_check_root", side_effect=mock_check_root), patch.object(c, "_check_zone", side_effect=mock_check_zone), patch.object(c, "_check_final_rrset") as final_rrset, + patch.object(c, "_finalise", wraps=c._finalise) as finalise, ): result = c.check() assert result is None final_rrset.assert_not_called() + # Without this the test would also pass if check() returned None + # without finalising, leaving the report status unset. + finalise.assert_called_once() def test_trust_anchor_failure_returns_false(self): c = _make_checker()