Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 26 additions & 0 deletions tests/test_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 34 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
40 changes: 40 additions & 0 deletions tests/test_init.py
Original file line number Diff line number Diff line change
@@ -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)
32 changes: 32 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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__")
Loading