From 0c3d2dcc85582b1bd2fc3e3976b9af78fbea53ad Mon Sep 17 00:00:00 2001 From: "cmeans-claude-dev[bot]" <272174644+cmeans-claude-dev[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 22:51:49 -0500 Subject: [PATCH] test: 100% coverage on src/ via real tests for 5 defensive lines + fail_under gate (closes #37) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the priority-low coverage gap surfaced during the independent code-review pass (issue #37). Five new tests, no `# pragma: no cover`, no `coverage_exclude_lines` — every previously-uncovered defensive line is now exercised by a real test. Per-line coverage delta on src/ (post-PR #36 baseline → here): __init__.py 60% → 100% (lines 7-8) __main__.py 97% → 100% (line 52) badge.py 100% (unchanged) collector.py 99% → 100% (line 190) config.py 96% → 100% (lines 42, 86) TOTAL 99% → 100% New tests (5 total): 1. tests/test_init.py (new file) :: test_init_falls_back_to_dev_version_when_package_not_installed — monkeypatches importlib.metadata.version to raise PackageNotFoundError, importlib.reload(pypi_winnow_downloads), asserts __version__ == "0.0.0+dev". Restores the real version() and reloads in finally to keep other tests' view of __version__ correct. 2. tests/test_main.py :: test_main_module_invokes_main_when_run_as_script — uses runpy.run_module("pypi_winnow_downloads", run_name="__main__") to trigger the guard at __main__.py:52 in-process, with collector.collect stubbed to return an empty CollectorResult and sys.argv set to a valid --config path so main() falls through cleanly. Patches collector.collect BEFORE runpy fires so __main__.py's `from .collector import collect` reads the stub. 3. tests/test_collector.py :: test_run_pypinfo_raises_on_non_integer_download_count — feeds run_pypinfo a row with download_count: "not-an-int" (string), asserts CollectorError("non-integer download_count") raised. Locks in collector.py:190. 4. tests/test_config.py :: test_load_config_rejects_non_mapping_service_value — `service: just-a-string` YAML, asserts ConfigError matching "'service' must be a mapping, got str". Locks in config.py:42 (the _require_field isinstance check). 5. tests/test_config.py :: test_load_config_rejects_non_list_packages_value — `packages: 42` YAML (non-list, non-null), asserts ConfigError matching "'packages' must be a list, got int". Distinct from the existing null-packages test which exercises a different branch. pyproject.toml gains: [tool.coverage.run] source = ["src/pypi_winnow_downloads"] [tool.coverage.report] fail_under = 100 show_missing = true Now `uv run pytest --cov` exits non-zero if any line in src/ is uncovered. CI's `test` job will fail fast on a coverage regression rather than silently letting it land — same shape as the strict-markers / strict-config pytest gates already in place. Per the user's standing rule, no exclusion mechanism is used to game the metric; if a future line is "hard to test", the answer is to write the test (as done here for the __main__ guard via runpy) or to delete the line as dead code. Local sweep: 71/71 pytest pass (was 66, 5 new), ruff/format/ mypy clean, coverage gate green at 100.00%. --- CHANGELOG.md | 1 + pyproject.toml | 14 ++++++++++++++ tests/test_collector.py | 26 ++++++++++++++++++++++++++ tests/test_config.py | 34 ++++++++++++++++++++++++++++++++++ tests/test_init.py | 40 ++++++++++++++++++++++++++++++++++++++++ tests/test_main.py | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 147 insertions(+) create mode 100644 tests/test_init.py diff --git a/CHANGELOG.md b/CHANGELOG.md index db75662..c878780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Coverage on `src/` reaches 100%; `fail_under = 100` gate added in `pyproject.toml`.** Five previously-uncovered defensive lines now have real tests, no `# pragma: no cover` annotations, no `coverage_exclude_lines` patterns. Specifically: `__init__.py:7-8` (`PackageNotFoundError` fallback) covered by a `monkeypatch + importlib.reload` test; `__main__.py:52` (the `if __name__ == "__main__": main()` guard) covered by a `runpy.run_module(..., run_name="__main__")` test that stubs `collector.collect` to a no-op so the line runs without shelling out to pypinfo; `collector.py:190` (non-integer `download_count` defensive raise) covered by feeding the runner a row with a string download_count; `config.py:42` (non-mapping `service:` value) covered by `service: just-a-string` YAML; `config.py:86` (non-list `packages:` value) covered by `packages: 42` YAML. Adds `[tool.coverage.run] source = ["src/pypi_winnow_downloads"]` and `[tool.coverage.report] fail_under = 100, show_missing = true` so future regressions trip CI immediately. Total tests: 71 (was 66). Closes #37. - **collector: `service.stale_threshold_days` is now actually consulted.** `config.example.yaml` documented "warn (in the log) if the last successful collector run is older than this many days" but no caller read the field — it was loaded, validated, and silently ignored. `collect()` now calls a new `_check_staleness(output_dir, threshold_days, now)` helper at the start of each run that reads the previous `_health.json`, parses its `finished` timestamp, and emits `logger.warning("collector: previous successful run is %.1f days old (threshold: %d days); previous finished: %s", ...)` when the gap exceeds the threshold. The check is log-only (does NOT mutate badge JSON, per the documented v1 contract) and degrades silently when the previous health file is absent (first run / fresh deploy), unreadable, malformed, missing the `finished` field, or shows a future timestamp (clock skew). Seven regression tests cover: warn-when-stale, silent-when-fresh, silent-on-no-previous-health, silent-on-malformed-json, silent-on-future-timestamp, silent-on-unreadable-previous-health (OSError other than `FileNotFoundError` — exercised by creating `_health.json` as a directory so `Path.read_text` raises `IsADirectoryError`), and silent-on-missing-`finished`-key (independent branch coverage of the `KeyError` arm of the JSON-parse `except` union). Both new silent-* tests assert on the documented DEBUG-log records, locking in the operator-visible signal that distinguishes the failure modes. Closes #33. - **collector: `_write_health` `OSError` no longer escapes per-package isolation.** A failure inside the health-file write step (disk full, output dir not writable, atomic-replace cross-device, etc.) used to propagate as an unhandled `OSError` traceback through `__main__.main()`, bypassing the structured `winnow-collect: N package(s) failed: …` exit message and producing a raw exit. The fix wraps `_write_health(...)` inside `collect()` with `try/except OSError`, logs the failure, and surfaces it via a new `CollectorResult.health_write_error: str | None` field (default `None`, backward-compat). `__main__.main()` now combines per-package failures and health-write failures into one structured exit message (`winnow-collect: 2 package(s) failed: foo, bar; health file write failed: [Errno 28] No space left on device`). Adds `test_collect_health_write_oserror_recorded_not_raised` (monkeypatches `os.replace` to raise `ENOSPC` only on the health file) and `test_main_combines_package_and_health_failure_messages` for regression coverage. Closes #32. - `README.md` line 11: canonicalized the [shields.io](https://shields.io/badges/endpoint-badge) doc link to match the form already used at line 96. Both references now point at the same canonical URL instead of relying on `/endpoint` redirecting to `/badges/endpoint-badge`. Closes #16. diff --git a/pyproject.toml b/pyproject.toml index 95ee7ac..69aaf84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,6 +57,20 @@ minversion = "8.0" testpaths = ["tests"] addopts = "-ra --strict-markers --strict-config" +[tool.coverage.run] +# Measure only the package source, not the tests themselves. The "Missing" +# column on tests stayed at 0 historically because they don't have defensive +# branches, so excluding them keeps the report focused on production code. +source = ["src/pypi_winnow_downloads"] + +[tool.coverage.report] +# Fail CI if statement coverage drops below 100%. No `# pragma: no cover` +# annotations or `exclude_lines` patterns — every line either has a real +# test or is deleted as dead code (see issue #37 / PR for the five +# previously-uncovered defensive lines that motivated this gate). +fail_under = 100 +show_missing = true + [tool.ruff] line-length = 100 target-version = "py311" diff --git a/tests/test_collector.py b/tests/test_collector.py index b5be98e..ff25a45 100644 --- a/tests/test_collector.py +++ b/tests/test_collector.py @@ -439,6 +439,32 @@ def fake_runner(argv: list[str], env: dict[str, str]) -> subprocess.CompletedPro run_pypinfo("mypkg", 30, credential_file=creds, runner=fake_runner) +def test_run_pypinfo_raises_on_non_integer_download_count(tmp_path: Path) -> None: + """pypinfo's BigQuery output normally has int `download_count` values. + A row with a non-int (e.g., a string from a malformed/changed query + response) must raise CollectorError rather than silently coerce or + pass a bad type into downstream sums and badge JSON. Locks in the + defensive raise at collector.py:190. + """ + stdout = json.dumps( + { + "rows": [ + {"ci": "False", "download_count": "not-an-int", "installer_name": "pip"}, + ], + "query": {}, + } + ) + + def fake_runner(argv: list[str], env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return _ok_result(argv, stdout=stdout) + + creds = tmp_path / "creds.json" + creds.write_text("{}") + + with pytest.raises(CollectorError, match=r"non-integer download_count"): + run_pypinfo("mypkg", 30, credential_file=creds, runner=fake_runner) + + def test_run_pypinfo_raises_on_subprocess_timeout(tmp_path: Path) -> None: def slow_runner(argv: list[str], env: dict[str, str]) -> subprocess.CompletedProcess[str]: raise subprocess.TimeoutExpired(cmd=list(argv), timeout=180) diff --git a/tests/test_config.py b/tests/test_config.py index 4c32d13..ea1da2b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -211,3 +211,37 @@ def test_load_config_raises_configerror_on_null_package_name(tmp_path: Path) -> with pytest.raises(ConfigError, match=r"packages\[0\]\.name"): load_config(config_path) + + +def test_load_config_rejects_non_mapping_service_value(tmp_path: Path) -> None: + """`service: just-a-string` (or any non-mapping at the service key) must + raise ConfigError via _require_field's isinstance check. Locks in the + defensive raise at config.py:42 — fires when YAML supplies a scalar + where a mapping is required. + """ + config_path = tmp_path / "config.yaml" + config_path.write_text( + "service: just-a-string\npackages:\n - name: foo\n window_days: 30\n" + ) + + with pytest.raises(ConfigError, match=r"'service' must be a mapping, got str"): + load_config(config_path) + + +def test_load_config_rejects_non_list_packages_value(tmp_path: Path) -> None: + """`packages: 42` (a non-list, non-null scalar) must raise ConfigError + distinct from the null-packages branch immediately above. Locks in the + isinstance check at config.py:86, which the existing null-packages + test does NOT exercise (separate branches). + """ + config_path = tmp_path / "config.yaml" + config_path.write_text( + "service:\n" + f" output_dir: {tmp_path / 'out'}\n" + f" credential_file: {tmp_path / 'creds.json'}\n" + " stale_threshold_days: 3\n" + "packages: 42\n" + ) + + with pytest.raises(ConfigError, match=r"'packages' must be a list, got int"): + load_config(config_path) diff --git a/tests/test_init.py b/tests/test_init.py new file mode 100644 index 0000000..71bb605 --- /dev/null +++ b/tests/test_init.py @@ -0,0 +1,40 @@ +"""Tests for `pypi_winnow_downloads/__init__.py` — primarily the +`PackageNotFoundError` defensive fallback for `__version__`. +""" + +import importlib +import importlib.metadata + +import pytest + +import pypi_winnow_downloads + + +def test_init_falls_back_to_dev_version_when_package_not_installed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`__init__.py:7-8` declares a defensive fallback: if `importlib.metadata` + can't find an installed `pypi-winnow-downloads` package (i.e., we're + running from source without an editable install, or the dist-info is + missing), `__version__` is set to `"0.0.0+dev"` instead of crashing. + + Patch `importlib.metadata.version` to raise `PackageNotFoundError`, + `importlib.reload` the package, and assert the fallback fires. After + the test, reload again with the patch undone so subsequent tests see + the real installed version on `pypi_winnow_downloads.__version__`. + """ + real_version = importlib.metadata.version + + def raising_version(name: str) -> str: + raise importlib.metadata.PackageNotFoundError(name) + + monkeypatch.setattr(importlib.metadata, "version", raising_version) + + try: + importlib.reload(pypi_winnow_downloads) + assert pypi_winnow_downloads.__version__ == "0.0.0+dev" + finally: + # Restore the real version() and reload so other tests aren't poisoned + # with the dev-version fallback string. + monkeypatch.setattr(importlib.metadata, "version", real_version) + importlib.reload(pypi_winnow_downloads) diff --git a/tests/test_main.py b/tests/test_main.py index 25801f2..b060bd1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,8 +1,10 @@ +import runpy from datetime import UTC, datetime from pathlib import Path import pytest +from pypi_winnow_downloads import collector as collector_module from pypi_winnow_downloads.__main__ import main from pypi_winnow_downloads.collector import CollectorResult, PackageOutcome @@ -127,3 +129,33 @@ def both_failed_collect(_cfg): SystemExit, match=r"broken-pkg.*health file write failed.*Permission denied" ): main([f"--config={cfg}"], collector_fn=both_failed_collect) + + +def test_main_module_invokes_main_when_run_as_script( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`if __name__ == "__main__": main()` at __main__.py:52 fires when the + package is invoked as `python -m pypi_winnow_downloads`. Use + `runpy.run_module` with `run_name="__main__"` to exercise the same code + path in-process so the line is observed by coverage. + + Stub `collector.collect` to a no-op CollectorResult so we don't shell out + to pypinfo; main() will see no failures and return without SystemExit. + """ + cfg = _valid_config(tmp_path) + now = datetime(2026, 4, 24, 21, 0, 0, tzinfo=UTC) + + def stub_collect(_config: object) -> CollectorResult: + return CollectorResult(started=now, finished=now, outcomes=()) + + # __main__.py does `from .collector import collect`, which reads the + # `collect` attribute on the collector module at import time. Patching + # collector_module.collect BEFORE runpy.run_module ensures the freshly + # executed __main__.py picks up the stub. + monkeypatch.setattr(collector_module, "collect", stub_collect) + monkeypatch.setattr("sys.argv", ["winnow-collect", "--config", str(cfg)]) + + # run_name="__main__" makes the guard at line 52 evaluate True. With + # zero failures returned by the stub, main() falls through cleanly + # (no SystemExit). + runpy.run_module("pypi_winnow_downloads", run_name="__main__")