diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2760c84..6c1818a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -214,8 +214,11 @@ jobs: if: ${{ !cancelled() }} run: make npm-audit - # Secret scan over the PR/push diff and full history (SEC-17/18). Installs - # the gitleaks CLI directly (checksum-verified) rather than the third-party + # Secret scan over the full history and, separately, the checked-out tree + # (SEC-17/18). `make secrets` runs both: a commit scan cannot see a file that + # was never committed, and the tree scan cannot see a secret that was + # committed and later deleted. See the Makefile recipe. Installs the gitleaks + # CLI directly (checksum-verified) rather than the third-party # `gitleaks/gitleaks-action`, which is license-gated for organizations. secrets: runs-on: ubuntu-latest diff --git a/.gitleaks.toml b/.gitleaks.toml new file mode 100644 index 0000000..e979cd8 --- /dev/null +++ b/.gitleaks.toml @@ -0,0 +1,40 @@ +# gitleaks configuration for the `make secrets` gate (SEC-17/18). +# +# `make secrets` runs two scans, because they answer different questions and +# neither answers the other's: +# +# gitleaks detect --source . -> every commit reachable from HEAD +# gitleaks detect --no-git --source . -> the files on disk right now +# +# The history scan is blind to a secret that has been written but not yet +# committed, which is exactly the state a working tree is in when a gate runs +# against it. Measured on this repository: a file containing an AWS key pair, a +# GitHub PAT and a Slack bot token, saved at the repository root and never +# added to the index, produced "no leaks found" and exit 0 from the history +# scan and "leaks found: 1" and exit 1 from the --no-git scan. +# +# This file exists so the second scan has a stable scope. Without it the +# working-tree scan also walks .venv/ and node_modules/, which the release +# verification workflow populates before it runs `make verify`: roughly 29 MB +# of third-party code whose test fixtures are a standing source of findings +# this project cannot fix. Paths below are build output and installed +# dependencies, never project source. + +[extend] +useDefault = true + +[allowlist] +description = "Build output, installed dependencies and tool caches" +paths = [ + '''(^|/)\.git/''', + '''(^|/)\.venv/''', + '''(^|/)node_modules/''', + '''(^|/)\.mypy_cache/''', + '''(^|/)\.ruff_cache/''', + '''(^|/)\.pytest_cache/''', + '''(^|/)\.hypothesis/''', + '''(^|/)\.worktrees/''', + '''(^|/)dist/''', + '''(^|/)htmlcov/''', + '''(^|/)__pycache__/''', +] diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5fbad..50b019e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,45 @@ Fixed: the honest answer to "the page this repository publishes", and which goes green again on the deploy that resolves any real drift. +- The secret-scan gate could not see the working tree. `make secrets` ran + `gitleaks detect --source .`, which walks commits, under a comment claiming + it covered "working tree + history". Measured: a file at the repository root + holding an AWS key pair, a GitHub PAT and a Slack bot token, saved and never + added to the index, gave "283 commits scanned / no leaks found" and exit 0; + the same tree with `--no-git` gave "leaks found: 1" and exit 1. The gate now + runs both scans, each reporting its own result, and neither can + short-circuit the other. `.gitleaks.toml` scopes the working-tree scan away + from `.venv/` and `node_modules/`, which `verify.yml` populates before it + runs `make verify`. +- The performance budget could only fail for one reason, and doing less work + made it greener. `scripts/check_perf_budget.py` divided an assumed row count + (`trips * 2`) by CPU time and discarded the timed run's result entirely, so + a validator that had stopped reading the feed would have burned almost no + CPU, reported an enormous rate, and passed further inside the budget than a + correct one. Every repetition now counts the rows the loader actually + parsed, refuses to report a rate below the floor the generator writes, and + refuses when two repetitions disagree about the count. + `maxRegressionFactor` is also bounded: it was read unbounded from the same + file as the baseline, where a large enough value retires the gate rather + than loosening it. +- `mypy` did not check the scripts that are the gates. `ruff` covered + `src tests scripts`; `mypy` had `files = ["src"]`, leaving + `check_public_contract.py`, `check_npm_audit.py`, `generate_rules_doc.py` + and `spec_watch.py` unchecked. Now `["src", "scripts"]`, 44 files. One real + error surfaced and is fixed: `spec_watch.py`'s spec fetch returned `Any` + from a function declared `-> str`. +- `make verify` could be green on a tree CI rejects, without saying so. Two + `pull_request` jobs had no `make` equivalent and no mention in the Makefile + header that enumerates CI-only work: `perf`, and the VS Code extension + package job (path-filtered to `editor/vscode/**`, which is why it went + unnoticed). Both are named now, and `tests/test_ci_gate_parity.py` compares + the header to the workflows so the next one cannot go unnamed. +- `docs/read-api.md` was outside the currency gate. It documents ten of the + nineteen names the v1 contract freezes and carried no `Last verified` stamp, + so `make docs-check` had nothing to fail on when it drifted. Added to + `STAMPED`, and `tests/test_doc_currency.py` now derives its parametrize from + that list instead of restating it. + Changed: - `loader.py` now pools cell values per file: equal cells in one file share one @@ -268,6 +307,42 @@ Added: a pin PyPI did not serve, so `micropip.install` rejected it for every visitor while every gate stayed green). Runs in `pages.yml` after each deploy and weekly in `playground-deployment.yml` (#146). +- `src/tods_validate/py.typed`. The package declared no type information, so + every downstream type checker treated an installed `tods-validate` as + untyped and refused to look inside it: a five-line consumer importing + `validate_feed` got `Skipping analyzing "tods_validate": ... missing library + stubs or py.typed marker` and exit 1 from `mypy --strict`, and gets + "Success" now. `mypy --strict` has run over `src/` on every pull request + since 0.1.0 without any of that reaching a caller. +- Tests for the two public exports nothing exercised. + `tods_validate.read.to_dataframe` and `tods_validate.__version__` are both in + `docs/v1-contract-candidate.json` and were named in none of the 52 test + modules, which a 90% line-coverage floor cannot see. Both are covered, and + `tests/test_contract_surface.py` adds the floor that finds the next one. +- `tests/test_readme_claims.py`: every `--flag` the README names must exist in + the CLI, or be attributed to another program, or be recorded as + documented-absent with a link to the gap that tracks it. + +Documentation: + +- The Observability section claimed an opt-in `--log-format json` flag. No + such flag exists, and no module under `src/` imports `logging`, so there are + no log records for one to format. The section says that now, the Standards + Conformance table points at the new + `docs/CONFORMANCE-GAPS.md#observability` row, and the row sets out both ways + to close it without picking one. +- `docs/api.md` listed seven `Finding` fields and two helpers. The dataclass + has ten fields and three helpers, and `docs/report.schema.json` already + required the three it omitted (`data`, `caused_by`, `severity_original`) and + `fingerprint()`, which is the identity `--baseline` matches on. +- `docs/read-api.md` now documents `FeedFile.readable` and `LoadProblem`; + `problems` had been documented without its element type. +- Two smaller README corrections: "16 reference checks" is now "the 16 checks + that read GTFS files" (six of the sixteen are field, semantic or coverage + rules), and the `ingest-ready` paragraph records that it currently resolves + to the same settings as `strict`. +- `docs/plans/v1.0.0-readiness.md`, an item-by-item readiness assessment with + evidence per item, and `docs/plans/improvement-plan.md`, the log behind it. ## [0.10.0] - 2026-08-21 diff --git a/Makefile b/Makefile index 4d6b1c3..03690e2 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,24 @@ # make verify runs every merge-blocking gate that can run on a laptop, with the # same command CI runs (CICD-27). Run it before opening a PR; the release # workflows re-run it at the tagged commit before anything publishes -# (REL-14/15). CI additionally runs what needs GitHub itself -- the composite -# action's self-test, CodeQL, Semgrep and zizmor -- so a green `make verify` is -# a necessary condition for merge, not a sufficient one. +# (REL-14/15). +# +# CI additionally runs five things this file does not, so a green `make verify` +# is a necessary condition for merge and not a sufficient one: +# +# - the composite action's self-test, CodeQL, Semgrep and zizmor, which need +# GitHub itself; +# - the `perf` job, which runs `make perf-check` and `make memory-check` +# against baselines recorded on the runner's machine class (see those +# targets below); +# - the VS Code extension package job, which type-checks, audits and builds +# a VSIX out of editor/vscode. It is path-filtered to that directory, so it +# is absent from most pull requests, which is how it stayed off this list +# for as long as it did. +# +# This paragraph is checked against the workflows by +# tests/test_ci_gate_parity.py, so a job added later cannot reject a tree that +# `make verify` has just called green without saying so here. .PHONY: verify lockfile lint format typecheck test docs-check contract-check i18n-check audit npm-audit secrets a11y citation-cff perf-check memory-check # Every gate `make verify` runs, in reporting order. Each one is independent: @@ -144,8 +159,36 @@ audit: # Secret scan over the working tree + history (SEC-17/18). Requires the # gitleaks binary (see https://github.com/gitleaks/gitleaks#installing); the # CI job installs it explicitly rather than via the license-gated Action. +# +# Two scans, because one of them cannot see what the other is for. `gitleaks +# detect --source .` walks commits: it answers "was a secret ever committed", +# and it is blind to a file that exists on disk and has not been committed +# yet. That is the state every working tree is in while a gate runs against +# it. Measured on this repository at v0.10.0: a file at the repository root +# holding an AWS key pair, a GitHub PAT and a Slack bot token, saved and never +# added to the index, gave "283 commits scanned / no leaks found" and exit 0 +# from the history scan, and "leaks found: 1" and exit 1 from --no-git. The +# comment above this recipe had said "working tree + history" since the gate +# was written; only the history half existed. +# +# Both run, whatever the other one did, and each reports its own result -- the +# same reason `verify` does not stop at its first failure. `.gitleaks.toml` +# scopes the working-tree scan away from installed dependencies; see that file. secrets: - gitleaks detect --source . --redact --exit-code 1 + @status=0; \ + printf '%s\n' '' 'gitleaks scan 1 of 2: committed history'; \ + if gitleaks detect --source . --redact --exit-code 1; then \ + printf '%s\n' 'gitleaks history: PASS'; \ + else \ + status=1; printf '%s\n' 'gitleaks history: FAIL'; \ + fi; \ + printf '%s\n' '' 'gitleaks scan 2 of 2: working tree, uncommitted files included'; \ + if gitleaks detect --no-git --source . --redact --exit-code 1; then \ + printf '%s\n' 'gitleaks working tree: PASS'; \ + else \ + status=1; printf '%s\n' 'gitleaks working tree: FAIL'; \ + fi; \ + exit $$status # Node dependency vulnerability audit (SEC-11). This used to be the first line # of the `a11y` recipe, which meant a HIGH advisory anywhere in the npm diff --git a/README.md b/README.md index 3e55fdb..c47845f 100644 --- a/README.md +++ b/README.md @@ -207,7 +207,10 @@ be passed with `--config path/to/file.toml`. A config may also `extends = A third preset, `ingest-ready`, is for a downstream CAD/AVL system deciding whether to import a feed at all: it is at least as strict as `strict` (fails on warnings, enables `coverage` and `advisory`) and adds no ignores, so it -doubles as a go/no-go gate rather than an authoring-time policy. +doubles as a go/no-go gate rather than an authoring-time policy. Today it +resolves to exactly the same settings as `strict`; it is a separate name +because the two answer different questions, and a later change to one should +not silently move the other. Some checks are off by default because they surface judgement calls rather than spec violations. Turn them on with `--enable coverage` (which GTFS trips have no @@ -388,8 +391,8 @@ jobs: The action runs `--format github`, so the annotations it leaves on the pull request include the checks that did not run and why (see [Rule-set coverage](#rule-set-coverage)). Leaving `gtfs` -out is the case worth knowing about: 16 reference checks cannot run, 9 of them -ERROR-severity, and the job still passes. Add +out is the case worth knowing about: the 16 checks that read GTFS files cannot +run, 9 of them ERROR-severity, and the job still passes. Add `require-complete-run: "true"` to fail it instead. The action installs `tods-validate` from a hash-verified @@ -473,8 +476,18 @@ bug — please report it. ## Observability -Observability: Tier C — OTel tracing out-of-scope (no network surface). Opt-in ---log-format json only. +Observability: Tier C. OpenTelemetry tracing is out of scope, because there is +no network surface to trace. + +The tier also asks for an opt-in `--log-format json` flag, and that flag does +not exist. It is not an oversight that a release would quietly carry: the +package emits no log records at all (nothing under `src/` imports `logging`), +so a flag to choose their format would be a claim rather than a capability. +What is machine-readable here is the report, through `--format json`, `--format +sarif`, and the schema at [docs/report.schema.json](docs/report.schema.json). +That is a different thing from a log stream, and this section previously +conflated them. Tracked in +[docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#observability). ## Standards Conformance @@ -488,7 +501,7 @@ Applicability and current state: | CI-CD | Applies | Applies — gap tracked, see [docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#ci-cd) | | RELEASE-AND-VERSIONING | Applies (PyPI + GHCR + GitHub Releases + Action) | Applies — gap tracked, see [docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#release-and-versioning) | | ACCESSIBILITY | Applies (scoped to the `--format html` report and the `web/` playground) | Applies — gap tracked, see [docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#accessibility) | -| OBSERVABILITY | Applies at Tier C (see `## Observability` above) | Applies — Tier C; N/A — tracing has no network surface, as declared above | +| OBSERVABILITY | Applies at Tier C (see `## Observability` above) | Applies — Tier C; tracing N/A (no network surface); the tier's `--log-format json` is a gap, see [docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#observability) | | INTERNATIONALIZATION | N/A — no user-facing strings requiring translation | N/A — see [docs/I18N.md](docs/I18N.md) | | AI Development Measurement | Applies | Applies — gap tracked, see [docs/CONFORMANCE-GAPS.md](docs/CONFORMANCE-GAPS.md#ai-development-measurement) | | AI Evaluation | N/A — no LLM/AI runtime | N/A — no LLM SDK or generative/agentic component anywhere in `src/` or `scripts/`; deterministic rule engine only | diff --git a/docs/CONFORMANCE-GAPS.md b/docs/CONFORMANCE-GAPS.md index d68a55c..c480379 100644 --- a/docs/CONFORMANCE-GAPS.md +++ b/docs/CONFORMANCE-GAPS.md @@ -34,6 +34,42 @@ validator itself has no model runtime. v2.0.0 tiers and add a mechanically checked data-card/source inventory without claiming ownership of users' input feeds. +## observability + +**Current boundary:** Tier C, per `OBSERVABILITY-STANDARD.md` §0. OTel tracing +is out of scope and the README's `## Observability` section declares it: there +is no network surface to trace, and the tool is offline by design. + +**Still open (found 2026-08-28):** Tier C also asks for "an opt-in +`--log-format json` flag backed by `structlog`" (`OBSERVABILITY-STANDARD.md` +§3, and `QUALITY-AND-METRICS-STANDARD.md` line 190 restates it as a must). The +flag does not exist anywhere in `src/`. Until today the README reproduced the +standard's own declaration sentence verbatim, ending "Opt-in `--log-format +json` only", which reads as a statement that the flag is there; nothing in +this ledger recorded otherwise, and no gate compared the sentence to the CLI. +`tests/test_readme_claims.py` now does, so the claim cannot return without the +flag returning with it. + +Two ways to close it, and the choice is a product decision rather than a +remediation: + +1. **Restate the tier.** Nothing under `src/` imports `logging`; the package + emits no log records at all, so there is no stream for a format flag to + select. The machine-readable surface here is the *report* (`--format json`, + `--format sarif`, `docs/report.schema.json`), which is a different artifact + from a log. If the standard's intent is "a machine can consume this tool's + output", that is already met, and the row should say so in those words + rather than by naming a flag. +2. **Implement it.** `structlog` would be a second runtime dependency for a + tool that deliberately has one (`click`), added to satisfy a sentence + rather than a user. Weaker unless an operator asks for parseable progress + logs on large feeds. + +Not on the v1.0.0 critical path either way: `--log-format` does not appear in +`docs/v1-contract-candidate.json`, so adding it later is an additive minor +release. What was on the critical path was shipping v1.0.0 with the README +claiming it. + ## incident-response **Still open:** security reporting and release recovery exist, but the v2.0.0 diff --git a/docs/api.md b/docs/api.md index 7f829ba..482fee3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -45,10 +45,24 @@ all. ## `Finding` -A frozen dataclass: `rule_id`, `severity`, `message`, `file`, `row`, `field`, -`suggestion`. Helpers: `location()` (human string) and `pointer()` (a stable -`file.txt#L4/field` identifier). `to_dict()` matches -[docs/report.schema.json](report.schema.json). +A frozen dataclass. Fields: `rule_id`, `severity`, `message`, `file`, `row`, +`field`, `suggestion`, `data` (the rule's own machine context, such as the +offending value or the ID a reference failed to resolve), `caused_by` (set +when this finding is a downstream echo of another, carrying that root's +`pointer()`), and `severity_original` (set when a config `[severity]` remap +moved the level, so the change is disclosed rather than silent). + +Helpers: `location()` (human string), `pointer()` (a stable +`file.txt#L4/field` identifier), and `fingerprint()` (a content hash over rule +ID, file, field and `data`, deliberately not over row or message, so inserting +an unrelated row does not change every later finding's identity; this is what +`--baseline` matches on). `to_dict()` matches +[docs/report.schema.json](report.schema.json), which requires every field +above. + +The last three fields and `fingerprint()` were missing from this list while +the report schema already required them, so a caller reading only this page +did not know what they were being handed. ## `suggest_fixes(path, gtfs=None, *, enable=(), encoding=None, spec_version=SPEC_VERSION)` @@ -121,12 +135,16 @@ stable. The lower-level `tods_validate.runner.run` is available too, but --- -Last verified: 2026-08-14, against tods-validate 0.8.0. Every signature, +Last verified: 2026-08-28, against tods-validate 0.10.0. Every signature, `ValidationResult` and `Finding` member, `Suggestion` field, and test helper on this page was called and checked against the implementation, including the documented `PackageNotFoundError` and the `SUPPORTED_SPEC_VERSIONS` values. +The `Finding` list was checked field by field against `findings.py` and +`report.schema.json` this time, which is how the four missing entries were +found; the previous stamp said 0.8.0 while the tree shipped 0.10.0, and this +gate compares content rather than versions, so it had no way to say so. Recheck cadence: every release, and whenever this page changes — `make docs-check` fails if the page is edited without a fresh verification. - + diff --git a/docs/plans/improvement-plan.md b/docs/plans/improvement-plan.md new file mode 100644 index 0000000..c538383 --- /dev/null +++ b/docs/plans/improvement-plan.md @@ -0,0 +1,365 @@ +# Improvement plan and running log + +Opened 2026-08-28. Working notes for an audit pass whose brief was: read the +open issues and pull requests, produce a v1.0.0 readiness assessment with +evidence, check the validator's own falsifiability rule by rule, fix the real +defects, and name what is blocked. + +The readiness assessment is the headline and lives in +[`v1.0.0-readiness.md`](v1.0.0-readiness.md). This file is the plan, the log, +and the provenance. + +**Nothing in this pass is committed.** Every change is left unstaged in the +working tree by instruction. This file is the durable record; the working tree +is not. + +## Working-tree provenance (read this first) + +The checkout this pass ran against is **behind the published `main`**: + +| Ref | Commit | Contains | +| --- | --- | --- | +| local `HEAD` / `main` | `a019bbe` | up to #141, `v0.10.0` | +| local `origin/main` | `fed1cf1` | + #142, #150, #151, #152 | +| GitHub `main` (read through the API) | `7a25056` | + #153 | + +`git fetch`, `pull`, `checkout` and every other HEAD-moving command were out of +scope for this pass, so the tree could not be advanced. Consequences, and how +they were handled: + +- Anything landed in #142/#150/#151/#152/#153 is **absent from this tree** and + would look like an open defect if audited naively. Every candidate finding + was therefore re-checked against `git show origin/main:` and, for + #153, against the GitHub contents API before being called a defect. Two + fail-opens found during the audit (`spec_watch.py` treating an unrecognised + document as "in sync"; `check_npm_audit.py` disarming its own cross-check on + an unparseable report) turned out to be **already fixed on `main` by #151**, + and are not re-fixed here. +- Fixes are written against `a019bbe`. Files also changed upstream are noted + per change below, so the merge is not a surprise. +- Three open pull requests (#154 and #155, stacked, and draft #79) were read + and deliberately not touched. #154/#155 already report `CONFLICTING` against + `main`; nothing here edits a file they add. + +## Issue triage + +| Issue | Classification | Note | +| --- | --- | --- | +| #146 playground boots on 0.10.0 | Real, open, human-gated | Needs a browser, an OS and a date recorded against the live page. #150 added `scripts/check-playground-boots.cjs`, which answers most of it mechanically; the dated human record is the remainder. | +| #145 PEP 735 dependency groups | **Already fixed** | Landed in #152, absent from this checkout. Do not re-do; close it against that PR. | +| #144 second advisory rule | Aspiration, correctly parked | `good first issue`, needs a spec citation chosen and defended. Does not block v1.0.0. | +| #143 structure warning for a recognized-but-unexpected file | Aspiration, correctly parked | Same shape as #144. | +| #76 production-feed feedback | Real, open, gated on people | Named in the multiyear plan as standing work with no owner and no date, which is the honest state. | +| #74 VoiceOver walkthrough | Real, open, gated on people | Was blocked by the stale playground pin; #142 removed that blocker, so it is workable now but still needs a human with assistive technology. | + +Two issue texts did not survive checking. #145's body describes work that is +done. #146's premise ("this has never been confirmed end to end") predates +#150, which now boots the live page in a real browser and asserts a finding +renders; what remains is narrower than the issue says. + +## The plan, ranked by value + +Ranked by "what would be worst to ship at v1.0.0", not by effort. + +| Phase | Item | State | +| --- | --- | --- | +| 1 | Falsifiability census: enumerate every rule by AST, prove each fires, prove none fires on a clean feed, prove each finding is caused by its own check | **Done** | +| 2 | The secret-scan gate cannot see the working tree | **Done** | +| 3 | The published package ships no type information | **Done** | +| 4 | The perf gate's failure is unreachable from below | **Done** | +| 5 | `mypy` does not cover the scripts that are the gates | **Done** | +| 6 | The README claims a flag that does not exist, under a conformance assertion | **Done** | +| 7 | `make verify` green on a tree CI would reject | **Done** | +| 8 | Two v1-contract exports exercised by no test | **Done** | +| 9 | API docs incomplete, and half of them outside the currency gate | **Done** | +| 9b | A rule threshold no test exercised at its boundary | **Done** | +| 10 | The v1.0.0 readiness assessment itself | **Done** | +| 11 | Branch ruleset, PyPI environment, upstream PR #156, a conformance-only release | **Blocked, named** | + +Phase 11 is the whole of what is left, and none of it is code in this +repository. + +## Change log, file by file + +Every fix below was broken and restored, and both directions were observed. +Logs are under `/private/tmp/tods-audit/`. + +### `Makefile` + +Two changes. + +**`secrets` recipe.** Was one line, `gitleaks detect --source . --redact +--exit-code 1`, under a comment claiming "working tree + history". `gitleaks +detect --source .` walks commits and is blind to an uncommitted file. Measured +on this repository with a root-level file holding an AWS key pair, a GitHub +PAT and a Slack bot token, never added to the index: + +| Command | Result | +| --- | --- | +| `gitleaks detect --source .` (the gate as it was) | `283 commits scanned` / `no leaks found`, **exit 0** | +| `gitleaks detect --no-git --source .` | `leaks found: 1`, **exit 1** | +| `--no-git` on the clean tree | `no leaks found`, exit 0 | + +It now runs both, each reporting its own PASS/FAIL, neither able to +short-circuit the other, for the same reason `verify` does not stop at its +first failing gate. `make secrets` on a tree with the planted file: **exit 2, +"gitleaks working tree: FAIL"**. On the clean tree: **exit 0, both PASS**. + +**Header.** It enumerated CI-only work as "the composite action's self-test, +CodeQL, Semgrep and zizmor" and omitted two `pull_request` jobs with no `make` +equivalent: `perf`, and the VS Code extension `package` job (path-filtered to +`editor/vscode/**`, which is why it went unnoticed). Both are named now, and +the paragraph is checked against the workflows. + +Upstream note: identical at `origin/main`, so both hunks apply cleanly. + +### `.gitleaks.toml` (new) + +Scopes the working-tree scan away from `.venv/`, `node_modules/` and build +caches. Not cosmetic: `.github/workflows/verify.yml` runs `uv sync` and +`npm ci` before `make verify`, so without this the release gate scans about +29 MB of third-party code whose test fixtures are a standing source of +findings this project cannot fix. Measured: 28.74 MB in 2.34s without it, +2.12 MB in 0.30s with it, same verdict on the planted secret. + +### `tests/test_secret_scan_gate.py` (new) + +Five tests. Two read the recipe and always run, so deleting the working-tree +half goes red in every job that runs pytest. Two drive the real binary against +a planted secret and against a clean control (the control matters: a scanner +that failed on everything would satisfy the first without it). One checks that +the `.gitleaks.toml` allowlist covers only dependencies and build output, so it +cannot become a way to stop scanning `src/`. + +Break: against the pre-fix one-line recipe, **2 failed, 3 passed**. Restore: +**5 passed**. + +The planted values are assembled from string fragments rather than written out +whole. Written as literals, the file was itself a finding: `make secrets` +reported `leaks found: 1` against `tests/test_secret_scan_gate.py:39`. The +honest answer to that is an inert file, not an allowlist entry exempting the +file that exists to defend the scan. + +### `src/tods_validate/py.typed` (new) and `tests/test_typing_marker.py` (new) + +The package had no PEP 561 marker, here or on `main`. `mypy --strict` runs over +`src/` on every pull request and the v1 contract reserves stability guarantees +for the public exports, and neither reached a caller: a five-line consumer +importing `validate_feed` got + +``` +error: Skipping analyzing "tods_validate": module is installed, but missing +library stubs or py.typed marker [import-untyped] +``` + +exit 1, and with the marker "Success: no issues found", exit 0. Confirmed the +file ships: `uv build --wheel` produces a wheel containing +`tods_validate/py.typed`. + +Break: with the marker removed, **all three tests fail**. Restore: **3 passed**. + +### `pyproject.toml` and `scripts/spec_watch.py` + +`[tool.mypy] files = ["src"]` became `["src", "scripts"]`. `ruff check src +tests scripts` already covered the scripts; `mypy` did not, so the +merge-blocking gates (`check_public_contract.py`, `check_npm_audit.py`, +`generate_rules_doc.py`, `spec_watch.py`) were the least verified code in the +repository. 34 files checked became 44. + +This only became cheap once `py.typed` existed: 16 of the 17 errors were +`import-untyped` against the package's own modules. The seventeenth was real +and is fixed in `spec_watch.py`, where `resp.read().decode("utf-8")` returns +`Any`, so the function's `-> str` was a promise mypy had never checked. + +Break: with the annotation reverted, `make typecheck` **exit 2**, one error. +Restore: **exit 0, 44 files**. + +Upstream note: the same construct is at `origin/main:scripts/spec_watch.py` +line 159, in a file #151 changed heavily; expect to re-apply this one-line hunk +by hand. + +### `scripts/check_perf_budget.py` and `tests/test_perf_budget.py` + +The gate measured `rows / cpu` where `rows = trips * 2`, an **assumed** +constant, and discarded the result of the timed `run(feed)` entirely. A +throughput budget can only fire on slowness, so doing less work makes it +greener: a validator that had quietly stopped reading the feed would burn +almost no CPU, report an enormous rate, and pass further inside the budget than +a correct one. Its failure was unreachable from below. Nothing noticed, because +every existing test stubs `measure` out. + +Now each repetition counts the rows the loader actually parsed, refuses to +report a rate below the floor the generator writes, and refuses when two +repetitions of the same feed disagree about the count. The rate's *denominator* +is deliberately left as the fixed unit of work: the real count is about 10% +higher (110,101 rows for 50,000 trips, not 100,000), and switching would raise +every published number by that margin and make the committed baseline look like +a speedup nobody made. + +Also bounded `maxRegressionFactor`, which was read unbounded from the same data +file as the baseline. A large enough value there does not loosen the gate, it +retires it, and retiring a gate belongs in a reviewed change rather than a data +edit. + +Break: with the three guards removed, **5 tests fail**. Restore: **17 passed**. +End to end on a real 300-trip feed: `761 rows parsed`, within budget, exit 0. + +### `README.md`, `docs/CONFORMANCE-GAPS.md`, `tests/test_readme_claims.py` (new) + +The README declared `Observability: Tier C ... Opt-in --log-format json only` +and, two paragraphs later, asserted conformance with that tier in the Standards +Conformance table. `--log-format` exists nowhere in `src/`, and no row in the +gaps ledger recorded that. `OBSERVABILITY-STANDARD.md` section 3 defines Tier C +as "an opt-in `--log-format json` flag backed by `structlog`". + +Nothing under `src/` imports `logging` at all, so there is no stream for such a +flag to format. The section now says that, links the new +`CONFORMANCE-GAPS.md#observability` row, and the row sets out both ways to +close it (restate the tier, or add `structlog` as a second runtime dependency +to a tool that deliberately has one) without picking. + +The durable half is the test: every `--flag` the README names must exist in the +CLI, unless it is another program's flag (three entries, each attributed) or is +recorded as documented-absent (one entry) **and** linked to the gap that tracks +it. Both allowlists are themselves checked for dead entries. + +Break, against the README and ledger exactly as at `HEAD`: **2 failed**. +Restore: **5 passed**. + +Two smaller README corrections in the same pass: "16 reference checks" became +"the 16 checks that read GTFS files" (6 of the 16 are field, semantic or +coverage rules), and the `ingest-ready` paragraph now says it currently +resolves to the same settings as `strict`, which it does, byte for byte. + +### `tests/test_ci_gate_parity.py` (new) + +Pins the Makefile header's promise against the workflows: every job in a +`pull_request` workflow either runs a `make verify` gate or is named in the +header as something CI does on its own. + +Two parser bugs were found and fixed while writing it, both of the kind the +test exists to catch. Slicing job bodies with `str.index` over the whole file +resolved a job id to an earlier occurrence, so one job's body ran on into the +next. And a job's slice ends where the *next* job's leading comment block +begins, so the `perf` job looked as though it ran `make a11y`, because the +paragraph introducing the accessibility job says so. A check that matched prose +instead of commands would have passed while measuring nothing. + +Break, against the header as it was: **`vscode-extension.yml:package` and +`ci.yml:perf` reported**. Restore: **3 passed**. + +### `tests/test_contract_surface.py` (new) + +`tods_validate.read.to_dataframe` and `tods_validate.__version__` are in +`docs/v1-contract-candidate.json` and were named in **none** of the 52 existing +test modules. A 90% line-coverage floor cannot see an export nothing imports. + +Both are covered now. `to_dataframe` gets its documented no-pandas +`ImportError` (pandas is deliberately not a dev dependency, so that was the +path most callers would hit first and the one nothing checked), plus a +happy-path test through a stub module. `__version__` is pinned to +`pyproject.toml` and asserted not to be the `0.0.0+unknown` fallback. + +Underneath both, a floor: every contract export must be named somewhere in the +suite. Pointed at the 52 pre-existing modules it returns exactly +`tods_validate.__version__` and `tods_validate.read.to_dataframe`. + +### `docs/api.md`, `docs/read-api.md`, `scripts/check_doc_currency.py`, `tests/test_doc_currency.py` + +`docs/api.md` described `Finding` as 7 fields and 2 helpers. It has 10 fields +and 3 helpers, and `report.schema.json` **requires** the three it omitted +(`data`, `caused_by`, `severity_original`) and `fingerprint()`, which is the +identity `--baseline` matches on. Re-verified field by field and re-stamped; +the old stamp said "against tods-validate 0.8.0" on a 0.10.0 tree, which the +gate could not have caught because it hashes content rather than comparing +versions. + +`docs/read-api.md` documents 10 of the 19 contract names and carried no +currency stamp, so `make docs-check` had nothing to fail on when it drifted. +Added to `STAMPED`, with `FeedFile.readable` and `LoadProblem` documented (both +public, both previously absent; `problems` was documented without its element +type, which left the field unusable from the page alone). + +`tests/test_doc_currency.py` parametrized over a hand-written list of two +paths. It now derives from the checker's own `STAMPED`, plus an assertion that +the list is not empty, because a parametrize over an empty list reports success +without running. + +Break: appending one line to `read-api.md` gives `check_doc_currency.py` +**exit 1**, naming the file and the new hash. Restore: **exit 0, three files +current**. + +### `tests/test_coverage_advisory.py` + +`TODS-I601` flags a run spanning more than `_LONG_SPAN_SECONDS = 6 * 3600` +with no break event. Its fixture spans eight hours, which proves the rule +fires and says nothing about where the bound is. Demonstrated: moving the +constant from six hours to seven left the conformance test for `TODS-I601` +**passing, exit 0**. Two runs one second apart now sit either side of the +boundary; against the same seven-hour bound they **both fail**. Restored: +10 passed. + +The pair also asserts the constant's current value, so moving the bound has to +move both sides deliberately rather than turning one of them red by accident. + +### `.github/workflows/ci.yml` + +Comment only. The `secrets` job described itself as scanning "the PR/push diff +and full history"; it now describes both scans and why neither substitutes for +the other. + +## Verification + +``` +make verify < /dev/null; echo "EXIT=$?" +``` + +`EXIT=0`. 13 of 13 gates PASS. 701 tests (from 664), coverage 91.80% against a +90% floor. Full log: `/private/tmp/tods-audit/20260828T1710-verify-final.log`. + +The AST rule census and the falsifiability harness were re-run after every +change: still 43 registered, 43 emitted, 0 non-literal emission sites, and 43 +of 43 rules firing on their own fixture, silent on the valid feed, and silent +when their check is neutered. + +`.venv/bin` must be on `PATH`, which is what CI does (`ci.yml` writes it to +`$GITHUB_PATH`). Without it six gates fail for environment reasons on a laptop. +That is not a repository defect, but it is a sharp edge for a new contributor, +because `CONTRIBUTING.md`'s local instructions and CI's `PATH` step describe +two different setups. + +## Left undone, and why + +- **`--log-format json`** (Observability Tier C). Not implemented. Adding + `structlog` as a second runtime dependency to a tool that deliberately has + one, in order to format log records that do not exist, is a product decision + and not a remediation. Recorded as a gap with both options written out. +- **The branch ruleset and the PyPI environment.** Live settings, inspected + read-only. See `v1.0.0-readiness.md` section C7 for what is actually there + and why the committed payload should be diffed against the live export + before anyone applies it. +- **Deleting the `v0` tag.** It is a lightweight tag pointing at the v0.5.0 + release commit, so `ChelseaKR/tods-validate@v0` resolves to v0.5.0 today. + Deleting a published ref is a live action; the gaps ledger already records + it and the two commands. +- **`tests/` under `mypy`.** Out of scope for this pass; `scripts/` was the + half that gates merges. +- **`docs/rulesets/main.json` corrections.** The file does not exist in this + checkout (it arrived with #152), so editing it here would have created a + conflicting duplicate. The divergence is written up in + `v1.0.0-readiness.md` section C7 instead. +- **Any new or changed TODS rule.** None was touched, so no spec text needed + citing. That is deliberate: #143 and #144 both need a citation chosen and + defended, and inventing one to close an issue would be the exact failure + this validator exists to refuse. + +## Status log + +- 2026-08-28: pass opened; issues, pull requests, roadmap and multiyear plan + read. +- 2026-08-28: baseline `make verify` recorded (green with `.venv` on `PATH`; + 664 tests, 91.71%). +- 2026-08-28: AST rule census and falsifiability census run; 43 of 43 clean. +- 2026-08-28: phases 2 to 9 executed, each broken and restored. +- 2026-08-28: `make verify` green, 701 tests, 91.80%. Readiness assessment + written. Phase 11 remains blocked on people. diff --git a/docs/plans/v1.0.0-readiness.md b/docs/plans/v1.0.0-readiness.md new file mode 100644 index 0000000..1d50f53 --- /dev/null +++ b/docs/plans/v1.0.0-readiness.md @@ -0,0 +1,159 @@ +# v1.0.0 readiness: item by item, with evidence + +Assessed 2026-08-28. This does not decide the release. It exists so the +decision can be made from evidence rather than from recall, and so nothing on +the list is "probably fine". + +## Where the checklist comes from + +There was no single v1.0.0 checklist, but there were four overlapping ones, +and they do not agree about the world. This assessment merges them and marks +where a document is wrong about its own repository: + +- `docs/roadmap.md`, "v1.0.0 -- Stability commitments" +- `docs/v1-contract-audit.md`, "Before this becomes the contract" (on `main`; + see the provenance note in `improvement-plan.md`) +- `docs/MULTIYEAR-PLAN.md`, Phase 2 "Done when" (on `main`) +- `DEFINITION_OF_DONE.md`, RELEASE-GATE and "Branch protection" + +## Verdict in one line + +**Not ready, and the blockers are not engineering.** Two of the three +remaining items are gated on other people (an upstream spec PR, and live +GitHub/PyPI settings only the maintainer can change); the third is that no +qualifying release has been cut. Everything this repository can finish on its +own is now finished, including six things that were not, and were not on +anyone's list. + +--- + +## A. The contract + +| # | Item | State | Evidence | +|---|---|---|---| +| A1 | The v1 snapshot matches the implementation | **Met** | `make contract-check` PASS in `/private/tmp/tods-audit/20260828T1710-verify-final.log`; `tests/test_public_contract.py` runs the same comparison in the suite. Whole-dict equality, so a snapshot with missing keys cannot pass. | +| A2 | One conformance-only release shipped, snapshot unchanged across it | **Not met** | `v0.10.0` disqualified itself in its own release note: it added `TODS-E207` and changed coverage-manifest behavior in three commands. `## Unreleased` changes behavior again. Earliest qualifying release is therefore two away, and only the maintainer can cut one. | +| A3 | Every name in the contract is exercised by a test | **Was not met; now met** | `tods_validate.read.to_dataframe` and `tods_validate.__version__` were named in **none** of the 52 test modules. Measured: `unreferenced_contract_exports()` against the pre-existing suite returns exactly those two. `tests/test_contract_surface.py` now covers both and adds the floor that finds the next one. | +| A4 | `employee_run_dates.txt` primary key settled upstream | **Not met, blocked** | MobilityData PR #156 read live 2026-08-28: `state=open`, `merged=false`, last updated 2026-07-17. Freezing `TODS-E204` vs `TODS-W408` today freezes this repository's reading of an issue thread. | +| A5 | The gate that verifies the contract cannot itself fail open | **Met on `main`** | `scripts/check_public_contract.py` compared `pythonExports` against each module's `__all__` rather than the resolved names; fixed in #152 (`_exports()` raises on an unresolvable name). Not in the audited checkout; verified by reading `git show origin/main:scripts/check_public_contract.py`. | + +## B. The rules, and whether any of them is decorative + +The sharpest failure available to a validator is a rule that is listed, +emitted, documented, and never actually fires. Enumerated by AST, not regex, +because a `[A-Z_]+` pattern misses `TODS-E207`. + +| # | Item | State | Evidence | +|---|---|---|---| +| B1 | Every registered rule is emitted, and every emitted rule is registered | **Met** | AST census of `src/`: 43 `@rule(...)` registrations, 43 distinct literal `Finding(rule_id=...)` values, sets identical. Zero non-literal `rule_id` construction sites, so no rule ID can be assembled at runtime and escape the census. `/private/tmp/tods-audit/20260828-census.txt`. | +| B2 | Every rule fires on input that should trip it | **Met** | All 43 rules run against their own `tests/fixtures/invalid//` fixture: 43 fire, 0 silent. `/private/tmp/tods-audit/20260828-falsifiability-census.log`. | +| B3 | No rule fires on a clean feed | **Met** | The valid fixture, with every opt-in category enabled, emits nothing at all. Same log, first line. | +| B4 | Each rule's finding is caused by that rule's own check | **Met** | For each rule, its check was replaced with one that yields nothing and its fixture re-run: in all 43 cases the finding disappeared. This is what rules out a finding produced by a *different* rule under the same ID. | +| B5 | Failure is reachable for every rule (no pass true by construction) | **Met** | B2 and B4 together: a rule whose candidate set were empty could not have produced its fixture's finding, and a rule that fired regardless of input would have fired on the valid feed. | +| B6 | Every rule is asserted by a test, not only by the corpus | **Met** | `tests/test_conformance.py::test_corpus_covers_every_rule` asserts `{fixture dirs} == {rule ids}` **and** `rule_id in expectations["invalid/" + rule_id]`, so deleting a rule, or leaving one that never fires, turns a gate red. Independently, every one of the 43 IDs appears in at least one of `test_structure/fields/references/semantics/coverage_advisory`. | +| B7 | The conformance oracle was not generated by the implementation it tests | **Met, with one residual** | `tests/fixtures/expectations.json` is committed and hand-edited; `scripts/build_conformance_corpus.py` has no write mode and `build()` **refuses to package** when current results differ (`raise RuntimeError`, line 116). Residual: the *additional* rule IDs in each fixture's expected set (e.g. `invalid/TODS-E205` also expects `TODS-I501`) were seeded from a run and are reviewed by eye. The load-bearing assertion, that a rule fires on its own fixture, is not seeded that way. | +| B8 | No documented-but-unimplemented rule, no undocumented rule | **Met** | `docs/rules.md` is generated from the registry and byte-compared in CI (`tests/test_docs_drift.py`, `make docs-check`); 43 headings, set-identical to the registry. | +| B9 | Rule thresholds exercised at their boundary | **Was not met; now met** | `TODS-I601`'s bound is six hours and its fixture spans eight. Measured: moving `_LONG_SPAN_SECONDS` to seven hours left `tests/test_conformance.py`'s TODS-I601 case **passing, exit 0**. A boundary pair one second apart now fails against the moved bound and passes against the real one. It is the only numeric threshold in the rule set, so the class is closed rather than sampled. | + +**Nothing in the rule set is decorative.** This is the one area where the +project's claims were already true, and they were true for a structural +reason: the fixture-per-rule parity assertion is not something a passing +implementation can satisfy by accident. + +## C. Gates + +| # | Item | State | Evidence | +|---|---|---|---| +| C1 | `make verify` green | **Met** | `make verify < /dev/null; echo "EXIT=$?"` -> `EXIT=0`, 13 of 13 gates PASS, 701 tests, 91.80% coverage. Requires `.venv/bin` on `PATH`, as CI does (`ci.yml` writes it to `$GITHUB_PATH`); without it six gates fail on a laptop for environment reasons. | +| C2 | The secret scan sees the working tree | **Was not met; now met** | `gitleaks detect --source .` scans commits. Measured: a root-level file holding an AWS key pair, a GitHub PAT and a Slack bot token, never added to the index, gave "283 commits scanned / no leaks found", exit **0**. With `--no-git`: "leaks found: 1", exit **1**. The recipe now runs both, each reporting its own status. | +| C3 | The gate scripts are type-checked | **Was not met; now met** | `mypy` had `files = ["src"]` while `ruff` covered `src tests scripts`, so `check_public_contract.py`, `check_npm_audit.py`, `generate_rules_doc.py` and `spec_watch.py` -- the gates themselves -- were the least verified code here. Now `files = ["src", "scripts"]`: 34 -> 44 files checked. | +| C4 | The published package ships type information | **Was not met; now met** | No `py.typed` anywhere, at this checkout or on `main`. A five-line consumer running `mypy --strict` against the installed package got `Skipping analyzing "tods_validate": ... missing library stubs or py.typed marker`, exit 1. With the marker: "Success", exit 0. Confirmed present in the built wheel. | +| C5 | The perf gate can fail for the reason it exists | **Was not met; now met** | `check_perf_budget.py` discarded `run(feed)`'s result and divided an **assumed** row count by CPU time. A validator that stopped reading the feed would burn no CPU, report an enormous rate, and pass *further inside* the budget than a correct one: failure was unreachable from below. Every repetition now counts the rows the loader actually parsed and refuses to report a rate below the floor. | +| C6 | A green `make verify` means what the Makefile says | **Was not met; now met** | Two `pull_request` CI jobs had no `make` equivalent and no mention in the header that enumerates CI-only work: `perf`, and the path-filtered VS Code extension `package` job. `tests/test_ci_gate_parity.py` now compares the header to the workflows. | +| C7 | The branch ruleset is live and is the one the repo documents | **Not met, and three documents are wrong about it** | Read live 2026-08-28: ruleset `protect-main` (id 18752857) **is** active on `refs/heads/main`. `DEFINITION_OF_DONE.md` ("Not yet enabled as a live GitHub ruleset") and `docs/v1-contract-audit.md` ("no ruleset is enabled on the repository") both say otherwise. But it is not the committed one: see the table below. | +| C8 | PyPI trusted-publisher environment scoping (CICD-06) | **Not met** | `gh api repos/ChelseaKR/tods-validate/environments` lists exactly one environment, `github-pages`. No publishing environment exists to scope to. Live settings change; maintainer only. | + +### C7 in detail: committed ruleset versus live ruleset + +`docs/rulesets/main.json` (added by #152) is checked against the *workflows* +by `tests/test_branch_ruleset.py`. Nothing checks it against *GitHub*. They +differ in three ways that matter: + +| | Committed `docs/rulesets/main.json` | Live, read 2026-08-28 | +|---|---|---| +| Name | `main` | `protect-main` | +| Rules | `deletion`, `non_fast_forward`, `required_linear_history`, `pull_request`, `required_status_checks` | `deletion`, `non_fast_forward`, `required_status_checks` | +| Required checks | 15 | 10 | +| Missing live | | `accessibility`, `action-self-test`, `citation`, `contract`, `perf` | + +Two consequences for the release decision: + +1. **`contract` is not a required check.** The gate that protects the v1 + public contract can be red on a pull request that merges. That is the one + required check a v1.0.0 story actually needs. +2. **Applying the committed payload is not a no-op.** The names differ, so + `gh api ... /rulesets` with that body creates a *second* ruleset rather + than updating the live one, and the payload replaces the live + configuration's merge requirements wholesale. Its `pull_request` rule sets + `required_approving_review_count: 1` with `require_last_push_approval: + true` and `require_code_owner_review: true`; on a repository whose + `CODEOWNERS` names one person, that is a configuration the sole maintainer + cannot satisfy. Export the live ruleset and diff it before applying + anything, rather than treating the file as the current state. + +This is a documentation defect, not a request to change any setting. No live +setting was changed by this pass, and none should be changed on the strength +of a file that does not describe what is there. + +## D. Documentation and claims + +| # | Item | State | Evidence | +|---|---|---|---| +| D1 | Every flag the README names exists | **Was not met; now met** | `--log-format` appeared nowhere in `src/`, while the Standards Conformance table asserted Observability Tier C, whose definition is "an opt-in `--log-format json` flag backed by `structlog`". No gap recorded it. The package imports `logging` in **zero** modules, so there is nothing to format. README corrected, gap opened, `tests/test_readme_claims.py` added. | +| D2 | The README's derived numbers match the registry | **Met, now guarded** | 43 rules and 16 `needs_gtfs` rules both check out; nothing had compared them. Now asserted. Separately, "16 reference checks" was reworded: 6 of the 16 are field, semantic or coverage rules, not reference rules. | +| D3 | `docs/api.md` describes the whole `Finding` | **Was not met; now met** | The page listed 7 fields and 2 helpers; the dataclass has 10 fields and 3 helpers, and `report.schema.json` **requires** the three it omitted (`data`, `caused_by`, `severity_original`) plus `fingerprint()`. Re-verified field by field and re-stamped. | +| D4 | The currency gate covers the published API surface | **Was not met; now met** | `docs/read-api.md` documents 10 of the 19 contract names and carried no currency stamp, so `make docs-check` had nothing to fail on. Added to `STAMPED`; the parametrized test now derives from `STAMPED` instead of restating it. | +| D5 | The gaps ledger has a row per applicable standard | **Was not met; now met** | No `## observability` section existed. Added, with both ways to close it and why neither is on the v1 critical path. | +| D6 | `docs/api.md` stamp names the shipped version | **Partly met** | It said "against tods-validate 0.8.0" on a 0.10.0 tree. Corrected. The gate hashes content and never compares versions, so it could not have said so; whether it should is a policy question left for the maintainer (it would force a re-verification every release). | + +## E. Release mechanics + +| # | Item | State | Evidence | +|---|---|---|---| +| E1 | `pyproject.toml` and `CITATION.cff` agree | **Met** | both `0.10.0`; `CITATION.cff` `date-released: 2026-08-21`. | +| E2 | `CHANGELOG.md` has a dated section for the release | **Pending the release** | `## Unreleased` exists and is where this pass's entries go. | +| E3 | Tag is annotated and signed | **Maintainer only** | `v0.9.0` and `v0.10.0` are `tag` objects (annotated). Creating a signed tag is not something an automated pass does. | +| E4 | No stale published refs | **Not met** | `v0` is a **lightweight tag pointing at `097427f`, the v0.5.0 release commit**. Anyone following the Actions convention and pinning `ChelseaKR/tods-validate@v0` today gets v0.5.0: four releases and two behavior-changing fixes old, including the companion-GTFS fail-open. Deleting a published ref is out of scope for a file-editing pass and is recorded as such in the gaps ledger; it is worth doing before v1.0.0 either way, because `v1` will invite exactly the same pin. | + +## F. Things that would have been embarrassing to ship, in order + +1. **A conformance claim for a flag that does not exist** (D1). The README + asserted Observability Tier C conformance and named `--log-format json` in + the same breath, in a repository whose entire argument is that it does not + claim what it has not checked. +2. **A secret-scan gate that could not see the working tree** (C2), under a + comment that said "working tree + history". +3. **A published library with no `py.typed`** (C4), freezing a + semantic-versioning promise on a public API that every downstream type + checker refuses to look at. +4. **A perf gate whose failure was unreachable from below** (C5). +5. **Two v1-contract exports that no test mentioned** (A3). +6. **`contract` not being a required status check** (C7), so the gate + protecting the contract could not block a merge. +7. **The one numeric threshold in the rule set, unexercised at its boundary** + (B9). Moving `TODS-I601`'s six-hour bound to seven left the suite green. + +## G. What is actually left + +Three items, none of them code in this repository: + +1. Cut a conformance-only release and let the snapshot go one cycle unchanged + (A2). Maintainer. +2. MobilityData PR #156 landing, so `TODS-E204` rests on published text (A4). + Upstream; open since before 2026-07-17. +3. Reconcile the branch ruleset and add a PyPI publishing environment (C7, + C8). Live settings; maintainer, one interactive session, and the ruleset + needs the diff described above before anything is applied. + +Item 2 is the only one with no date attached, and it is the one the contract +document already calls the hardest blocker. diff --git a/docs/read-api.md b/docs/read-api.md index 2523dd0..27bee89 100644 --- a/docs/read-api.md +++ b/docs/read-api.md @@ -34,8 +34,20 @@ Loads all top-level files from a directory or `.zip` file and returns a | `name` | The file name. | | `headers` | Column names, in declaration order. | | `rows` | `list[Row]`. | -| `problems` | Structural defects found while reading (bad encoding, ragged rows, and so on). | +| `problems` | `list[LoadProblem]`: structural defects found while reading (bad encoding, ragged rows, and so on). | | `column(name)` | `True` when `name` is a declared header. | +| `readable` | `False` when the file could not be parsed at all. Check it before reading anything into an empty `rows`: nothing could be read is a different fact from the file was empty. | + +## `LoadProblem` + +One entry in `FeedFile.problems`, reached through that field rather than +exported from this namespace. A dataclass: `code` (`"encoding"`, `"empty"`, +`"ragged"`, `"duplicate_header"` or `"csv_error"`), `message`, `line`, +`column` (set for `"duplicate_header"`), and `expected` / `actual` (declared +and actual value counts, set for `"ragged"`). The first three codes are the +ones that make `readable` false. This page previously documented the +`problems` field without naming its element type, which left the field +unusable from the page alone. ## `Row` @@ -78,3 +90,16 @@ namespace is intentionally curated and kept separate from the top-level package namespace (`from tods_validate import validate_feed`, documented in [api.md](api.md)) so the stability commitment stays bounded to what is re-exported here. + +--- + +Last verified: 2026-08-28, against tods-validate 0.10.0. Every member and +signature on this page was checked against `loader.py`, `gtfs_companion.py` +and `read.py`, and against `tods_validate.read.__all__`, which is the list the +v1 contract freezes. This page documents ten of the nineteen names in +`docs/v1-contract-candidate.json` and until now carried no currency stamp at +all, so `make docs-check` had nothing to fail on when it drifted. +Recheck cadence: every release, and whenever this page changes; +`make docs-check` fails if the page is edited without a fresh verification. + + diff --git a/pyproject.toml b/pyproject.toml index 11dcf3a..b510d5a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,7 +102,18 @@ max-complexity = 10 [tool.mypy] strict = true -files = ["src"] +# `scripts/` is in scope because those files *are* the gates: `make docs-check`, +# `make contract-check`, `make npm-audit`, `make i18n-check` and the weekly spec +# tripwire are all scripts/*.py. Until now `ruff check src tests scripts` covered +# them and `mypy` did not, so the merge-blocking checks were the least verified +# code in the repository. Bringing them in cost one annotation +# (scripts/spec_watch.py) once `src/tods_validate/py.typed` existed; before that +# marker every `from tods_validate import ...` in a script was an import mypy +# reported as untyped and then refused to follow, which is also why adding the +# marker and adding this path belong together. +# +# `tests/` stays out of scope; see docs/CONFORMANCE-GAPS.md. +files = ["src", "scripts"] [[tool.mypy.overrides]] # pandas is an optional extra (tods_validate.read.to_dataframe); its stubs are diff --git a/scripts/check_doc_currency.py b/scripts/check_doc_currency.py index e342e1d..d5ab892 100644 --- a/scripts/check_doc_currency.py +++ b/scripts/check_doc_currency.py @@ -3,10 +3,9 @@ DOCUMENTATION-STANDARD §6.5: a document whose correctness depends on something outside itself carries a ``Last verified: YYYY-MM-DD`` line and a -``Recheck cadence:`` line. ``docs/getting-started.md``, ``docs/api.md``, and -``docs/a11y/STATEMENT.md`` -document commands and API members that the code can move out from under, and -had neither. +``Recheck cadence:`` line. ``docs/getting-started.md``, ``docs/api.md``, +``docs/read-api.md`` and ``docs/a11y/STATEMENT.md`` document commands and API +members that the code can move out from under, and had neither. A date on its own is only a claim. This check makes the claim falsifiable: the stamp records a fingerprint of the page as it was when someone verified it, and @@ -27,7 +26,16 @@ from pathlib import Path ROOT = Path(__file__).resolve().parents[1] -STAMPED = ("docs/getting-started.md", "docs/api.md", "docs/a11y/STATEMENT.md") +# docs/read-api.md joined the list on 2026-08-28. It documents ten of the +# nineteen names the v1 contract freezes -- the whole `tods_validate.read` +# namespace -- against code that can move under it exactly as api.md's can, +# and it was the half of the published API surface with no stamp to fail on. +STAMPED = ( + "docs/getting-started.md", + "docs/api.md", + "docs/read-api.md", + "docs/a11y/STATEMENT.md", +) VERIFIED_RE = re.compile(r"^Last verified: (\d{4}-\d{2}-\d{2})\b", re.MULTILINE) CADENCE_RE = re.compile(r"^Recheck cadence: \S", re.MULTILINE) diff --git a/scripts/check_perf_budget.py b/scripts/check_perf_budget.py index cac5fc3..36c7672 100644 --- a/scripts/check_perf_budget.py +++ b/scripts/check_perf_budget.py @@ -18,6 +18,23 @@ the honest estimate of what the code can do. Noise then makes a regression under-reported rather than invented, and the budget absorbs the difference. +And one thing it now does that it did not. A throughput gate can only fire on +slowness, which means *doing less work makes it greener*. The timed call's +result was discarded and the row count was an assumed constant, so a validator +that had quietly stopped reading the feed would have burned almost no CPU, +reported an enormous rate, and passed more comfortably than a correct one -- +the failure the gate exists to catch was unreachable from below. Every +repetition now counts the rows the loader actually parsed, refuses to report a +rate when that count falls short of what the generated feed contains, and +refuses when two repetitions disagree about it. + +The rate's denominator stays the fixed ``trips * ROWS_PER_TRIP`` unit of work +rather than becoming the measured count. The two differ (the generated feed +also carries stops, vehicles and assignments), and switching would silently +raise every number by that margin, making the committed baseline look like a +speedup nobody made. The measured count is printed and checked; it is not the +divisor. + The baseline has to be recorded on the machine class the gate runs on: a number from a laptop compared against a shared CI runner is a comparison between two different things, which is how a perf gate ends up either permanently red or @@ -42,12 +59,29 @@ from benchmark import build_feed # noqa: E402 -from tods_validate.runner import run # noqa: E402 +from tods_validate.runner import run_with_coverage # noqa: E402 ROOT = Path(__file__).resolve().parents[1] BASELINE = ROOT / "perf" / "baseline.json" DEFAULT_TRIPS = 50000 DEFAULT_REPEAT = 3 +# `benchmark.build_feed` writes one trips.txt row and one run_events.txt row per +# trip, so a run that parsed fewer than this did not read the feed it was given. +# The generated package also holds calendar, stops, vehicles and +# vehicle_assignments rows, so the real count is comfortably above the floor; +# the floor is the "did any work happen" question, not a row-exact assertion. +ROWS_PER_TRIP = 2 +# A budget this large is not a loose budget, it is a retired one, and it would +# be retired in a data file rather than in a reviewed change to this script. +MAX_SANE_BUDGET = 10.0 + + +class NoWorkMeasured(RuntimeError): + """A timed repetition did not do the work the measurement assumes. + + Raised rather than returned so there is no path on which the caller can + treat "the validator read nothing" as a very fast run. + """ def measure(trips: int, repeat: int) -> float: @@ -63,20 +97,40 @@ def measure(trips: int, repeat: int) -> float: with tempfile.TemporaryDirectory() as tmp: feed = Path(tmp) / "feed" build_feed(feed, trips) - rows = trips * 2 # trips + run events dominate + rows = trips * ROWS_PER_TRIP + floor = trips * ROWS_PER_TRIP best = 0.0 + parsed_counts: set[int] = set() for attempt in range(1, repeat + 1): wall_start = time.perf_counter() cpu_start = time.process_time() - run(feed) + package, _findings, _coverage = run_with_coverage(feed) cpu = time.process_time() - cpu_start wall = time.perf_counter() - wall_start + + parsed = sum(len(f.rows) for f in package.files.values()) + parsed_counts.add(parsed) + if parsed < floor: + raise NoWorkMeasured( + f"repetition {attempt} parsed {parsed:,} rows from a {trips:,}-trip " + f"feed, below the {floor:,} this generator writes. The measurement " + "timed a run that did not read the feed, and a rate computed from " + "it would report a speedup rather than the defect." + ) + throughput = rows / cpu print( f" run {attempt}/{repeat}: {cpu:.2f}s CPU ({wall:.2f}s wall), " - f"{throughput:,.0f} rows/CPU-s" + f"{parsed:,} rows parsed, {throughput:,.0f} rows/CPU-s" ) best = max(best, throughput) + + if len(parsed_counts) != 1: + raise NoWorkMeasured( + "repetitions of the same feed parsed different row counts " + f"({sorted(parsed_counts)}). The runs are not comparable, so the best " + "of them is not a measurement of anything." + ) return best @@ -98,7 +152,11 @@ def main() -> int: repeat = args.repeat or int(baseline.get("repeat", DEFAULT_REPEAT)) print(f"measuring {trips} trips, best of {repeat}") - measured = measure(trips, repeat) + try: + measured = measure(trips, repeat) + except NoWorkMeasured as exc: + print(f"::error::{exc}") + return 1 print(f"\nmeasured: {measured:,.0f} rows/CPU-s") expected = baseline.get("rowsPerCpuSecond") @@ -114,7 +172,21 @@ def main() -> int: ) return 1 - budget = float(baseline.get("maxRegressionFactor", 2.0)) + declared_budget = baseline.get("maxRegressionFactor", 2.0) + if ( + not isinstance(declared_budget, int | float) + or isinstance(declared_budget, bool) + or not 1.0 <= float(declared_budget) <= MAX_SANE_BUDGET + ): + print( + f"::error::{BASELINE.relative_to(ROOT)} declares maxRegressionFactor " + f"{declared_budget!r}, which is not a number between 1.0 and " + f"{MAX_SANE_BUDGET}. A budget outside that range does not loosen this " + "gate, it retires it, and retiring a gate belongs in a reviewed change " + "rather than in a data file." + ) + return 1 + budget = float(declared_budget) ratio = float(expected) / measured if measured else float("inf") print( f"baseline: {float(expected):,.0f} rows/CPU-s " diff --git a/scripts/spec_watch.py b/scripts/spec_watch.py index b545e7a..f8e59e2 100644 --- a/scripts/spec_watch.py +++ b/scripts/spec_watch.py @@ -156,7 +156,11 @@ def fetch_spec_text(spec_file: str | None, spec_url: str) -> str: with urllib.request.urlopen( # noqa: S310 # nosemgrep safe_spec_url, timeout=20 ) as resp: - return resp.read().decode("utf-8") + # Annotated because urlopen's return is Any: without this the + # decoded value is Any too, and `-> str` would be a promise mypy + # never checked. + body: bytes = resp.read() + return body.decode("utf-8") except (urllib.error.URLError, TimeoutError, ValueError, OSError) as exc: raise SpecFetchError(f"could not fetch spec from {spec_url!r}: {exc}") from exc diff --git a/src/tods_validate/py.typed b/src/tods_validate/py.typed new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_ci_gate_parity.py b/tests/test_ci_gate_parity.py new file mode 100644 index 0000000..16948ed --- /dev/null +++ b/tests/test_ci_gate_parity.py @@ -0,0 +1,173 @@ +"""Everything CI runs on a pull request is either a `make verify` gate or declared. + +The Makefile's header makes a promise: "a green `make verify` is a necessary +condition for merge, not a sufficient one", and then lists what CI additionally +runs. That list is what a contributor uses to decide whether a clean local run +means anything. It was written by hand and compared to nothing, so a workflow +added later could reject a tree that `make verify` had just called green, with +no mention anywhere a contributor would look. The VS Code extension job was +exactly that: type-check, `npm audit` and a VSIX package step, on +`pull_request`, absent from both `VERIFY_GATES` and the header's list. + +The contract this pins is deliberately weak and therefore keepable: every job +in a workflow triggered by `pull_request` either runs one of the gates +`make verify` runs, or is named in the Makefile header as something CI does on +its own. It does not require CI to be reproducible on a laptop. It requires the +Makefile to stop being wrong about which parts are not. + +The workflow files are read with a small regex parser rather than PyYAML, which +is not a declared dependency here (`scripts/check_npm_audit.py` hand-parses +`waivers.yml` for the same reason). Every parse asserts it found something, so +a format change that made the parser see nothing fails instead of passing. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +_ROOT = Path(__file__).resolve().parent.parent +_WORKFLOWS = _ROOT / ".github" / "workflows" +_MAKEFILE = _ROOT / "Makefile" + +# Jobs that intentionally have no `make` equivalent, mapped to the words that +# must appear in the Makefile header so a contributor reading it learns the job +# exists. The reason is the entry's justification; the string is what is +# checked. +_CI_ONLY_JOBS = { + "action-self-test": "action's self-test", + "analyze": "CodeQL", + "semgrep": "Semgrep", + "zizmor": "zizmor", + # The baseline is recorded on the CI runner's machine class, so a laptop + # number is not comparable; `make perf-check` exists but is deliberately not + # a `verify` gate. The Makefile says so at the `perf-check` recipe. + "perf": "`perf` job", + # Path-filtered to editor/vscode/**, so it does not run on most pull + # requests -- which is why its absence went unnoticed. + "package": "VS Code extension", +} + +# Jobs that run a gate's recipe directly instead of invoking the make target. +# Equivalent, but only while the two stay in step, so the recipe is compared +# rather than assumed. +_DIRECT_GATE_JOBS = {"i18n": "i18n-check"} + + +def _workflow_text(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _triggers_on_pull_request(text: str) -> bool: + header = text.split("\njobs:", 1)[0] + return re.search(r"^\s{2}pull_request:", header, re.M) is not None + + +def _jobs_section(text: str) -> str: + return text.split("\njobs:", 1)[1] if "\njobs:" in text else "" + + +def _job_bodies(text: str) -> dict[str, str]: + """{job id: that job's text}, sliced from the match positions themselves. + + Slicing by `str.index` on the whole file looked equivalent and was not: a + job id that also occurs earlier in the file resolves to the wrong offset, so + one job's body ran on into the next and picked up its `make` invocation. + """ + section = _jobs_section(text) + matches = list(re.finditer(r"^ ([a-zA-Z0-9_-]+):\s*$", section, re.M)) + bodies: dict[str, str] = {} + for index, match in enumerate(matches): + end = matches[index + 1].start() if index + 1 < len(matches) else len(section) + bodies[match.group(1)] = _without_comments(section[match.start() : end]) + return bodies + + +def _without_comments(body: str) -> str: + """Job text with comment lines removed. + + A job's slice ends where the next job's leading comment block begins, and + those comments describe the job that follows. Leaving them in made the + `perf` job look as though it ran `make a11y`, because the paragraph + introducing the accessibility job says so -- a check that matched prose + instead of commands, which is the failure mode it exists to catch. + """ + return "\n".join(line for line in body.splitlines() if not line.lstrip().startswith("#")) + + +def _verify_gates() -> list[str]: + makefile = _MAKEFILE.read_text(encoding="utf-8") + match = re.search(r"^VERIFY_GATES := (.*?)$\n(?:\t(.*?)$)?", makefile, re.M | re.S) + assert match is not None, "Makefile no longer declares VERIFY_GATES" + joined = match.group(0).replace("VERIFY_GATES :=", "").replace("\\", " ") + gates = [word for word in joined.split() if word and not word.startswith("#")] + assert len(gates) >= 5, f"parsed only {gates} out of VERIFY_GATES; the parser is wrong" + return gates + + +def _gate_recipe_lines(gate: str) -> list[str]: + """One verify gate's recipe body.""" + makefile = _MAKEFILE.read_text(encoding="utf-8") + match = re.search(rf"^{re.escape(gate)}:\n((?:\t.*\n|\n(?=\t))*)", makefile, re.M) + assert match is not None, f"Makefile has no `{gate}:` target" + return [line.strip() for line in match.group(1).splitlines() if line.strip()] + + +def _pull_request_jobs() -> dict[str, tuple[str, str]]: + """{job id: (workflow filename, that job's text)} for pull_request workflows.""" + jobs: dict[str, tuple[str, str]] = {} + for path in sorted(_WORKFLOWS.glob("*.yml")): + text = _workflow_text(path) + if not _triggers_on_pull_request(text): + continue + bodies = _job_bodies(text) + assert bodies, f"{path.name} triggers on pull_request but no jobs were parsed" + for job, body in bodies.items(): + jobs[job] = (path.name, body) + return jobs + + +def test_the_parser_found_the_workflows_it_is_checking() -> None: + """A parse that silently found nothing would make every check below vacuous.""" + jobs = _pull_request_jobs() + assert len(jobs) >= 10, f"only parsed {sorted(jobs)}; the workflow format changed" + for expected in ("lint", "test", "secrets", "audit"): + assert expected in jobs, f"the {expected} job was not parsed out of ci.yml" + + +def test_every_pull_request_job_is_a_verify_gate_or_a_declared_exception() -> None: + header = _MAKEFILE.read_text(encoding="utf-8").split("\n.PHONY:", 1)[0] + gates = _verify_gates() + + undeclared: list[str] = [] + for job, (workflow, body) in sorted(_pull_request_jobs().items()): + runs_a_gate = any(f"make {gate}" in body for gate in gates) + if not runs_a_gate and job in _DIRECT_GATE_JOBS: + gate = _DIRECT_GATE_JOBS[job] + runs_a_gate = all(line in body for line in _gate_recipe_lines(gate)) + assert runs_a_gate, ( + f"the {job} job is recorded as running `make {gate}`'s command " + "directly, and no longer does; point it at the make target." + ) + if runs_a_gate: + continue + phrase = _CI_ONLY_JOBS.get(job) + if phrase is None or phrase not in header: + undeclared.append(f"{workflow}:{job}") + + assert not undeclared, ( + f"pull-request CI jobs that `make verify` does not cover and the Makefile " + f"header does not mention: {', '.join(undeclared)}. Either add the gate to " + "VERIFY_GATES or name the job in the header, so a green `make verify` does " + "not read as a promise CI will agree." + ) + + +def test_every_declared_exception_is_still_a_real_job() -> None: + """The exception list is the hole in the test above, so it is checked too.""" + jobs = set(_pull_request_jobs()) + stale = sorted(job for job in _CI_ONLY_JOBS if job not in jobs) + assert not stale, ( + f"{', '.join(stale)} is declared as a CI-only job but no pull_request " + "workflow defines it; drop the entry." + ) diff --git a/tests/test_contract_surface.py b/tests/test_contract_surface.py new file mode 100644 index 0000000..8171b35 --- /dev/null +++ b/tests/test_contract_surface.py @@ -0,0 +1,162 @@ +"""Every name in the v1 public contract is exercised by a test. + +`scripts/check_public_contract.py` proves the snapshot in +`docs/v1-contract-candidate.json` matches what the package exports. It says +nothing about whether any of those exports work, and two of them were reached +by no test at all before this file existed: `tods_validate.read.to_dataframe` +and `tods_validate.__version__`. Both are in the snapshot, so v1.0.0 would +have promised semantic-versioning stability for behaviour the suite had never +run. + +`to_dataframe` is the sharper of the two. Its documented contract +(`docs/read-api.md`) is a specific ImportError with an install hint when the +`dataframe` extra is absent, and `pandas` is deliberately not in the +development dependencies -- so the path most users would hit first was the one +nothing checked. +""" + +from __future__ import annotations + +import json +import sys +import tomllib +import types +from pathlib import Path +from typing import Any + +import pytest + +import tods_validate +from tods_validate.loader import FeedFile, Row +from tods_validate.read import to_dataframe, to_rows + +_ROOT = Path(__file__).resolve().parent.parent +_CONTRACT = _ROOT / "docs" / "v1-contract-candidate.json" +_TESTS = Path(__file__).resolve().parent + + +def _feed_file() -> FeedFile: + return FeedFile( + name="vehicles.txt", + headers=("vehicle_id", "vehicle_label"), + rows=[ + Row(line=2, values={"vehicle_id": "bus-1", "vehicle_label": "Old Reliable"}), + Row(line=3, values={"vehicle_id": "bus-2", "vehicle_label": "Spare"}), + ], + ) + + +# --------------------------------------------------------------------------- +# tods_validate.__version__ +# --------------------------------------------------------------------------- + + +def test_version_is_the_version_the_project_declares() -> None: + declared = tomllib.loads((_ROOT / "pyproject.toml").read_text(encoding="utf-8")) + assert tods_validate.__version__ == declared["project"]["version"] + + +def test_version_is_not_the_uninstalled_fallback() -> None: + """The fallback is correct behaviour and a wrong answer to ship. + + `__init__` falls back to "0.0.0+unknown" when the distribution metadata is + missing. A test run against an installed package that sees the fallback is + testing something other than the package under test, and every report, + SARIF document and `--stamp` footer would carry that string. + """ + assert tods_validate.__version__ != "0.0.0+unknown" + + +# --------------------------------------------------------------------------- +# tods_validate.read.to_dataframe +# --------------------------------------------------------------------------- + + +def test_to_dataframe_without_pandas_raises_the_documented_install_hint( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setitem(sys.modules, "pandas", None) + with pytest.raises(ImportError) as excinfo: + to_dataframe(_feed_file()) + assert "tods-validate[dataframe]" in str(excinfo.value), ( + "the error names no way to fix it; docs/read-api.md promises the extra" + ) + + +def test_to_dataframe_tabulates_the_same_rows_to_rows_does( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The happy path, without making pandas a development dependency. + + A stub stands in for pandas and records what it was handed. That is enough + to pin the part this project owns -- which rows reach the DataFrame + constructor -- and it deliberately does not try to test pandas. + """ + captured: list[Any] = [] + + stub = types.ModuleType("pandas") + stub.DataFrame = lambda data: captured.append(data) or "dataframe" # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "pandas", stub) + + feed = _feed_file() + result = to_dataframe(feed) + + assert result == "dataframe" + assert captured == [to_rows(feed)] + assert captured[0] == [ + {"vehicle_id": "bus-1", "vehicle_label": "Old Reliable"}, + {"vehicle_id": "bus-2", "vehicle_label": "Spare"}, + ] + + +def test_to_dataframe_of_a_missing_file_is_empty_rather_than_an_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: list[Any] = [] + stub = types.ModuleType("pandas") + stub.DataFrame = lambda data: captured.append(data) or "dataframe" # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "pandas", stub) + + to_dataframe(None) + assert captured == [[]] + + +# --------------------------------------------------------------------------- +# The floor under both of the above +# --------------------------------------------------------------------------- + + +def unreferenced_contract_exports(modules: list[Path] | None = None) -> list[str]: + """Contract exports named in none of ``modules`` (default: the whole suite). + + Exposed as a function so the check can be pointed at a subset -- which is + how it was shown to fail: run against the test modules that existed before + this file, it names `to_dataframe` and `__version__`. + """ + contract = json.loads(_CONTRACT.read_text(encoding="utf-8")) + paths = sorted(_TESTS.glob("test_*.py")) if modules is None else modules + sources = [path.read_text(encoding="utf-8") for path in paths] + if not sources: + raise AssertionError("no test modules to scan; the check would pass vacuously") + return [ + f"{module}.{name}" + for module, names in contract["pythonExports"].items() + for name in names + if not any(name in text for text in sources) + ] + + +def test_every_v1_contract_export_is_named_somewhere_in_the_suite() -> None: + """A coverage floor, not a proof of coverage. + + Being named in a test file is weak evidence that an export works. Being + named in none of them is strong evidence that it does not: a 90% line + coverage floor cannot see an export nothing imports, which is how + `to_dataframe` and `__version__` reached a release candidate untouched. + """ + assert not unreferenced_contract_exports(), ( + "public exports in the v1 contract that no test module mentions: " + f"{', '.join(unreferenced_contract_exports())}. Freezing a compatibility " + "promise around an export nothing exercises is the promise this project " + "exists to refuse." + ) diff --git a/tests/test_coverage_advisory.py b/tests/test_coverage_advisory.py index 304f0e4..124bfdf 100644 --- a/tests/test_coverage_advisory.py +++ b/tests/test_coverage_advisory.py @@ -31,3 +31,36 @@ def test_opt_in_rules_silent_on_valid_feed_even_when_enabled() -> None: _, findings = run(VALID_TODS, VALID_GTFS, enabled=_CATEGORIES) for rule_id in OPT_IN: assert rule_id not in rule_ids(findings), rule_id + + +# The advisory threshold itself. The TODS-I601 fixture spans eight hours +# against a six-hour bound, so it proves the rule fires and says nothing about +# where the bound is: moving _LONG_SPAN_SECONDS to seven hours left every test +# green. These two runs sit either side of the boundary, one second apart. + +_HEADER = ( + "service_id,run_id,event_sequence,event_type,start_location,start_time,end_location,end_time" +) + + +def _run_spanning(tmp_path, end_time: str) -> set[str]: + (tmp_path / "run_events.txt").write_text( + f"{_HEADER}\ndaily,1,10,Operator,s1,06:00:00,s1,{end_time}\n", + encoding="utf-8", + ) + _, findings = run(tmp_path, enabled=_CATEGORIES) + return rule_ids(findings) + + +def test_a_run_exactly_at_the_long_span_bound_is_not_flagged(tmp_path) -> None: + from tods_validate.rules.coverage import _LONG_SPAN_SECONDS + + assert _LONG_SPAN_SECONDS == 6 * 3600, ( + "the bound moved; update both sides of this boundary pair rather than " + "only the one that went red" + ) + assert "TODS-I601" not in _run_spanning(tmp_path, "12:00:00") + + +def test_a_run_one_second_past_the_long_span_bound_is_flagged(tmp_path) -> None: + assert "TODS-I601" in _run_spanning(tmp_path, "12:00:01") diff --git a/tests/test_doc_currency.py b/tests/test_doc_currency.py index 1a13274..acb3a95 100644 --- a/tests/test_doc_currency.py +++ b/tests/test_doc_currency.py @@ -27,11 +27,22 @@ def _checker() -> ModuleType: return module +# Derived from the checker rather than restated. The list was written out by +# hand here, so adding docs/read-api.md to STAMPED would have left the new page +# with a gate in `make docs-check` and no test proving that gate can fail. +_STAMPED = tuple(_checker().STAMPED) + + +def test_the_stamped_set_is_not_empty() -> None: + """A parametrize over an empty list reports success without running.""" + assert len(_STAMPED) >= 3, f"only {_STAMPED} are stamped; the parser or the list moved" + + def test_the_committed_stamps_are_current() -> None: assert _checker().main() == 0 -@pytest.mark.parametrize("relative", ["docs/getting-started.md", "docs/api.md"]) +@pytest.mark.parametrize("relative", _STAMPED) def test_editing_a_stamped_page_fails_until_it_is_re_verified( relative: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/test_perf_budget.py b/tests/test_perf_budget.py index 1d392a5..77b27b5 100644 --- a/tests/test_perf_budget.py +++ b/tests/test_perf_budget.py @@ -15,6 +15,8 @@ import pytest +from tods_validate.loader import FeedFile, Package, Row + ROOT = Path(__file__).parent.parent SCRIPT = ROOT / "scripts" / "check_perf_budget.py" BASELINE = ROOT / "perf" / "baseline.json" @@ -114,3 +116,147 @@ def test_measurement_uses_cpu_time_not_wall_clock() -> None: # budget that fires on someone else's build is a budget that gets muted. source = SCRIPT.read_text(encoding="utf-8") assert "time.process_time()" in source + + +# -------------------------------------------------------------------------- +# The gate can only fire on slowness, so doing less work makes it greener. +# These pin the floor underneath it: a repetition that did not read the feed +# must not be reportable as an extremely fast one. +# -------------------------------------------------------------------------- + + +def _package(rows_by_file: dict[str, int]) -> Package: + package = Package(source="synthetic") + for name, count in rows_by_file.items(): + package.files[name] = FeedFile( + name=name, + headers=("a",), + rows=[Row(line=i + 2, values={"a": str(i)}) for i in range(count)], + ) + return package + + +def _stub_run(gate: ModuleType, monkeypatch: pytest.MonkeyPatch, packages: list[Package]) -> None: + """Make each timed repetition return the next package in ``packages``.""" + monkeypatch.setattr(gate, "build_feed", lambda directory, trips: directory.mkdir(parents=True)) + remaining = list(packages) + + def _fake_run(feed: Path) -> tuple[Package, list[object], object]: + return remaining.pop(0), [], None + + monkeypatch.setattr(gate, "run_with_coverage", _fake_run) + + +def test_a_repetition_that_read_nothing_is_refused_not_reported_as_fast( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The defect this closes: run()'s result was discarded and the row count was + # an assumed constant, so a validator that stopped reading the feed burned + # no CPU, produced an enormous rate, and passed further inside the budget + # than a correct one. Failure was unreachable from below. + gate = _with_baseline( + monkeypatch, + tmp_path, + {"trips": 100, "repeat": 1, "rowsPerCpuSecond": 1000, "maxRegressionFactor": 2.0}, + ) + _stub_run(gate, monkeypatch, [_package({"trips.txt": 0})]) + with pytest.raises(gate.NoWorkMeasured): + gate.measure(100, 1) + + +def test_the_gate_exits_non_zero_when_no_work_was_measured( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + gate = _with_baseline( + monkeypatch, + tmp_path, + {"trips": 100, "repeat": 1, "rowsPerCpuSecond": 1000, "maxRegressionFactor": 2.0}, + ) + _stub_run(gate, monkeypatch, [_package({"trips.txt": 3})]) + monkeypatch.setattr(sys, "argv", ["check_perf_budget.py"]) + assert gate.main() == 1 + + +def test_a_repetition_that_did_the_work_is_accepted( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # The control. Without it, a floor set impossibly high would satisfy the two + # tests above and retire the gate instead of grounding it. + gate = _with_baseline( + monkeypatch, + tmp_path, + {"trips": 100, "repeat": 1, "rowsPerCpuSecond": 1, "maxRegressionFactor": 2.0}, + ) + full = {"trips.txt": 100, "run_events.txt": 100, "stops.txt": 10} + _stub_run(gate, monkeypatch, [_package(full)]) + assert gate.measure(100, 1) > 0 + + +def test_repetitions_that_disagree_about_the_row_count_are_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + gate = _with_baseline( + monkeypatch, + tmp_path, + {"trips": 100, "repeat": 2, "rowsPerCpuSecond": 1, "maxRegressionFactor": 2.0}, + ) + _stub_run( + gate, + monkeypatch, + [ + _package({"trips.txt": 100, "run_events.txt": 100}), + _package({"trips.txt": 100, "run_events.txt": 150}), + ], + ) + with pytest.raises(gate.NoWorkMeasured): + gate.measure(100, 2) + + +@pytest.mark.parametrize("declared", [50.0, 0.5, "2.0", None, True]) +def test_a_budget_outside_the_reviewed_range_fails_rather_than_widening_the_gate( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, declared: object +) -> None: + # maxRegressionFactor is read unbounded out of the same data file as the + # baseline. A large enough value does not loosen the gate, it retires it, + # and retiring a gate is a reviewed change rather than a data edit. + gate = _with_baseline( + monkeypatch, + tmp_path, + { + "trips": 10, + "repeat": 1, + "rowsPerCpuSecond": 1000, + "maxRegressionFactor": declared, + }, + ) + _measured(gate, monkeypatch, 400) # 2.5x slower: inside a 50x budget + monkeypatch.setattr(sys, "argv", ["check_perf_budget.py"]) + assert gate.main() == 1 + + +def test_the_committed_budget_is_inside_the_reviewed_range() -> None: + document = json.loads(BASELINE.read_text(encoding="utf-8")) + gate = _gate() + assert 1.0 <= document["maxRegressionFactor"] <= gate.MAX_SANE_BUDGET + + +def test_the_measurement_reports_the_rows_it_actually_parsed( + capsys: pytest.CaptureFixture[str], +) -> None: + """End to end on a small real feed: the count is measured, not assumed. + + Small enough to stay fast; the point is that the number printed comes from + the loader rather than from `trips * ROWS_PER_TRIP`. + """ + gate = _gate() + rate = gate.measure(50, 1) + assert rate > 0 + printed = capsys.readouterr().out + assert "rows parsed" in printed + # 50 trips + 50 run events + 100 stops + 1 calendar + 5 vehicles + 5 + # assignments. The floor the gate checks is trips * ROWS_PER_TRIP = 100, so + # the real feed clears it with room; a run that read nothing could not. The + # gap between 211 and 100 is also why the rate's denominator stays the + # fixed unit of work: switching it to the parsed count would move every + # published number by that margin. + assert "211 rows parsed" in printed, printed diff --git a/tests/test_readme_claims.py b/tests/test_readme_claims.py new file mode 100644 index 0000000..5cecb03 --- /dev/null +++ b/tests/test_readme_claims.py @@ -0,0 +1,171 @@ +"""The README's checkable claims, checked. + +The README is the product for anyone who has not installed the tool yet, and +several of its claims are mechanical: a flag name, a rule count, a severity +split. Nothing compared them to the code, and one had been wrong since the +Observability section was written -- it declared "Opt-in ``--log-format json`` +only", a flag that exists nowhere in ``src/``, while the Standards Conformance +table two paragraphs below asserted conformance with the tier that asks for it. + +These tests are deliberately narrow. They check claims that can be derived +from the implementation, and they say nothing about prose. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import click + +from tods_validate.cli import main +from tods_validate.rules import all_rules + +_README = Path(__file__).resolve().parent.parent / "README.md" + +# Long options the README names that belong to other programs, not to +# tods-validate. Kept explicit and small: an allowlist is the only part of this +# test that can be used to wave a real mismatch through, so each entry says +# whose flag it is. +_FOREIGN_FLAGS = { + "--rm": "docker run", + "--group": "pip install", + "--no-deps": "pip install", + "--require-hashes": "pip install", +} + +# Flags the README names in order to say they are *not* implemented. Naming one +# is allowed only while the gap is written down, so each entry points at the +# section of the gaps ledger that carries it, and both halves are checked +# below. Without this, "stop claiming it" and "explain why it is missing" would +# be indistinguishable to the test, and the second is the more useful README. +_DOCUMENTED_ABSENT = { + "--log-format": "observability", +} + +_GAPS = Path(__file__).resolve().parent.parent / "docs" / "CONFORMANCE-GAPS.md" + + +def _readme() -> str: + return _README.read_text(encoding="utf-8") + + +def _cli_long_options() -> set[str]: + """Every ``--option`` the CLI accepts, across the group and its subcommands.""" + found: set[str] = set() + + def walk(command: click.Command) -> None: + for param in command.params: + for opt in (*param.opts, *param.secondary_opts): + if opt.startswith("--"): + found.add(opt) + if isinstance(command, click.Group): + for sub in command.commands.values(): + walk(sub) + + walk(main) + return found + + +def test_every_flag_the_readme_names_exists_in_the_cli() -> None: + named = set(re.findall(r"(? None: + """Both halves of the exemption, so neither can rot. + + If the flag ships, the entry has to go or the README will keep explaining + an absence that ended. If the gaps ledger loses the section, the README is + pointing at nothing. + """ + gaps = _GAPS.read_text(encoding="utf-8") + options = _cli_long_options() + for flag, section in _DOCUMENTED_ABSENT.items(): + assert flag not in options, ( + f"{flag} is recorded as not implemented but the CLI now accepts it; " + "drop the entry so the flag is checked like every other one." + ) + readme = _readme() + assert flag in readme, ( + f"{flag} is recorded as documented-absent but the README no longer " + "mentions it; drop the entry." + ) + assert f"CONFORMANCE-GAPS.md#{section}" in readme, ( + f"the README names {flag} without linking the gap that tracks it. " + f"Naming an unimplemented flag is only honest next to " + f"docs/CONFORMANCE-GAPS.md#{section}; otherwise it reads as a feature." + ) + assert f"## {section}" in gaps, ( + f"{flag}'s gap link points at docs/CONFORMANCE-GAPS.md#{section}, " + "which has no such section." + ) + + +def test_the_foreign_flag_allowlist_still_earns_its_place() -> None: + """The allowlist is the hole in the test above, so it is checked too. + + An entry that the README no longer mentions is dead, and dead entries are + how an allowlist grows into a place to hide a real mismatch. + """ + readme = _readme() + unused = sorted(flag for flag in _FOREIGN_FLAGS if flag not in readme) + assert not unused, ( + f"{', '.join(unused)} is allowlisted as another program's flag but the " + "README no longer names it; drop the entry." + ) + for flag, owner in _FOREIGN_FLAGS.items(): + assert flag not in _cli_long_options(), ( + f"{flag} is allowlisted as {owner}'s flag but tods-validate now has " + "one by that name; remove it from the allowlist so it is checked." + ) + + +def test_the_readme_rule_counts_match_the_registry() -> None: + """ "43 checks" and "16 rules that read GTFS files" are derived numbers.""" + readme = _readme() + total = len(list(all_rules())) + needs_gtfs = sum(1 for rule in all_rules() if rule.needs_gtfs) + + assert f"{total} checks" in readme or f"of {total})" in readme, ( + f"the registry holds {total} rules and the README does not say so" + ) + assert f"{needs_gtfs} rules that read GTFS files" in readme, ( + f"{needs_gtfs} rules set needs_gtfs=True; the README's count disagrees" + ) + + +def test_the_readme_does_not_claim_a_logging_surface_the_package_does_not_have() -> None: + """The specific regression, pinned by its cause rather than its wording. + + ``--log-format`` is covered by the flag test above, but only while the + README spells it that way. This asserts the underlying fact: no module in + the package emits log records, so no claim about their format can be true. + """ + src = Path(__file__).resolve().parent.parent / "src" / "tods_validate" + importers = sorted( + path.relative_to(src).as_posix() + for path in src.rglob("*.py") + if re.search(r"^\s*(import logging|from logging import)", path.read_text("utf-8"), re.M) + ) + readme = _readme() + if importers: + # The package grew a logging surface. That is allowed, but then the + # Observability section's gap entry is stale and should be revisited + # rather than left saying nothing logs. + assert "--log-format" in readme, ( + f"{', '.join(importers)} now use logging, so the Observability " + "section's reason for having no --log-format flag no longer holds" + ) + else: + assert "Opt-in\n--log-format json only" not in readme, ( + "the README declares an opt-in --log-format flag while nothing in " + "the package logs anything" + ) diff --git a/tests/test_secret_scan_gate.py b/tests/test_secret_scan_gate.py new file mode 100644 index 0000000..0b6e1c3 --- /dev/null +++ b/tests/test_secret_scan_gate.py @@ -0,0 +1,173 @@ +"""The secret-scan gate must scan the working tree, not only committed history. + +`make secrets` is the merge-blocking secret scan (SEC-17/18). Its recipe used +to be one line, ``gitleaks detect --source .``, which walks commits. A file +written into the working tree and not yet committed is invisible to that scan, +so the gate reported "no leaks found" and exit 0 over a tree holding a live +key. Measured at v0.10.0 against a root-level file containing an AWS key pair, +a GitHub PAT and a Slack bot token: the history scan gave exit 0, and the same +scan with ``--no-git`` gave exit 1. + +Two tests here, doing different jobs. The first reads the recipe and always +runs, so deleting the working-tree half turns a test red on every machine and +in every CI job that runs pytest. The second drives the real binary and can +only run where it is installed; it is the one that proves the claim rather +than restating it. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parent.parent +_MAKEFILE = _ROOT / "Makefile" +_GITLEAKS_CONFIG = _ROOT / ".gitleaks.toml" +# Resolved once, absolutely: the two behavioural tests run a real binary, and an +# absolute path is both what the linter asks for and one fewer thing that can +# resolve to something other than the scanner under test. +_GITLEAKS = shutil.which("gitleaks") + +# Values that match gitleaks' default rules by shape. Nothing here is or ever +# was a credential: the AWS pair is the example AWS publishes in its own +# documentation, and the other two are digit runs. +# +# Each one is assembled from fragments rather than written out. A literal would +# make this file itself a finding, and the honest response to that is to keep +# the file inert, not to allowlist a path out of the scan the file exists to +# defend. Measured: with the Slack value written as one string, `make secrets` +# reported "leaks found: 1" against this file. +_PLANTED_SECRETS = "\n".join( + ( + "aws_access_key_id = " + "AKIA" + "IOSFODNN7EXAMPLE", + "aws_secret_access_key = " + "wJalrXUtnFEMI/K7MDENG/" + "bPxRfiCYEXAMPLEKEY", + "github_pat = " + "ghp_" + "1234567890abcdefghijklmnopqrstuvwxyzAB", + "slack_token = " + "xoxb-" + "123456789012-123456789012-abcdefghijklmnopqrstuvwx", + ) +) + + +def _scan(binary: str, target: Path) -> subprocess.CompletedProcess[str]: + """Run the working-tree scan the `secrets` gate runs, against one directory.""" + return subprocess.run( # noqa: S603 # absolute binary path, fixed argv + [ + binary, + "detect", + "--no-git", + "--source", + str(target), + "--config", + str(_GITLEAKS_CONFIG), + "--redact", + "--exit-code", + "1", + ], + capture_output=True, + text=True, + check=False, + ) + + +def _secrets_recipe() -> str: + """The body of the `secrets:` target, up to the next target or blank-line gap.""" + text = _MAKEFILE.read_text(encoding="utf-8") + match = re.search(r"^secrets:\n((?:\t.*\n|\n(?=\t))*)", text, re.M) + assert match is not None, "Makefile has no `secrets:` target" + return match.group(1) + + +def test_the_secret_gate_scans_the_working_tree_as_well_as_history() -> None: + recipe = _secrets_recipe() + assert "gitleaks" in recipe, "the secrets gate no longer invokes gitleaks" + assert "--no-git" in recipe, ( + "`make secrets` runs only the committed-history scan. `gitleaks detect " + "--source .` walks commits and cannot see an uncommitted file, so the " + "gate would pass over a working tree holding a secret. Keep the " + "`--no-git` scan." + ) + history_scans = [ + line for line in recipe.splitlines() if "gitleaks detect" in line and "--no-git" not in line + ] + assert history_scans, ( + "`make secrets` no longer scans committed history; --no-git alone cannot " + "find a secret that was committed and later deleted from the tree." + ) + + +def test_both_scans_report_independently() -> None: + """Neither scan may be short-circuited by the other's result. + + `a && b` skips b when a fails, and `a; b` throws away a's exit code. Both + have to run and both have to be able to fail the target on their own. + """ + recipe = _secrets_recipe() + assert "&&" not in recipe, ( + "the two gitleaks scans are chained with `&&`, so a failure in the first " + "one prevents the second from running at all" + ) + assert "status=1" in recipe, ( + "the recipe does not record a per-scan failure status; it cannot report " + "which of the two scans failed, or fail when only the second one did" + ) + + +def test_the_gitleaks_config_scopes_only_dependencies_and_build_output() -> None: + """The allowlist must not be a way to stop scanning project source. + + The working-tree scan walks whatever is on disk, which in the release + verification workflow includes `.venv/` and `node_modules/`. Those are + allowlisted so the gate measures this repository. An allowlist entry + covering `src/`, `tests/`, `scripts/` or the repository root would silence + the gate instead of scoping it. + """ + assert _GITLEAKS_CONFIG.exists(), ".gitleaks.toml is missing" + config = _GITLEAKS_CONFIG.read_text(encoding="utf-8") + paths = re.findall(r"'''(.+?)'''", config) + assert paths, "the allowlist has no path entries" + for entry in paths: + assert re.search(r"src|tests|scripts|examples|web|docs", entry) is None, ( + f"allowlist entry {entry!r} covers project source, which would stop " + "the gate scanning the files it exists to scan" + ) + for required in (".venv", "node_modules"): + assert any(required in entry for entry in paths), ( + f"{required} is not allowlisted; the release verification workflow " + "populates it before running `make verify`, so the working-tree scan " + "would report third-party findings this project cannot fix" + ) + + +@pytest.mark.skipif(_GITLEAKS is None, reason="gitleaks is not installed") +def test_a_planted_secret_is_found_in_an_uncommitted_file(tmp_path: Path) -> None: + """The behavioural half: the working-tree scan finds what history cannot. + + A directory that is not a git repository at all is the sharpest version of + "not committed". The history scan has nothing to walk; the working-tree + scan has the file. + """ + (tmp_path / "config.env").write_text(_PLANTED_SECRETS, encoding="utf-8") + + assert _GITLEAKS is not None + working_tree = _scan(_GITLEAKS, tmp_path) + assert working_tree.returncode == 1, ( + "gitleaks did not flag a planted credential in an uncommitted file; " + f"stdout={working_tree.stdout!r} stderr={working_tree.stderr!r}" + ) + + +@pytest.mark.skipif(_GITLEAKS is None, reason="gitleaks is not installed") +def test_the_working_tree_scan_is_clean_on_a_file_with_no_secret(tmp_path: Path) -> None: + """The control. Without it, a scanner that fails on everything would pass + the test above and tell us nothing.""" + (tmp_path / "config.env").write_text("region = us-west-2\nretries = 3\n", encoding="utf-8") + + assert _GITLEAKS is not None + result = _scan(_GITLEAKS, tmp_path) + assert result.returncode == 0, ( + f"gitleaks reported a finding on a file with no secret in it; " + f"stdout={result.stdout!r} stderr={result.stderr!r}" + ) diff --git a/tests/test_typing_marker.py b/tests/test_typing_marker.py new file mode 100644 index 0000000..b7a97f0 --- /dev/null +++ b/tests/test_typing_marker.py @@ -0,0 +1,89 @@ +"""The published package must advertise its own type information (PEP 561). + +`mypy --strict` runs over `src/` on every pull request, and the v1 contract +reserves semantic-versioning guarantees for the public Python exports. Neither +of those reaches anyone downstream without a `py.typed` marker in the +distribution: without it, type checkers treat an installed `tods_validate` as +untyped and refuse to look inside it, so a caller running mypy over their own +code gets `import-untyped` and no checking of this library's API at all. + +Measured at v0.10.0, before the marker was added, on a five-line consumer that +imports `validate_feed`: + + error: Skipping analyzing "tods_validate": module is installed, but + missing library stubs or py.typed marker [import-untyped] + +and with the marker in place, "Success: no issues found in 1 source file". +""" + +from __future__ import annotations + +import importlib.util +import subprocess +import sys +import textwrap +from pathlib import Path + +_ROOT = Path(__file__).resolve().parent.parent +_MARKER = _ROOT / "src" / "tods_validate" / "py.typed" + + +def test_the_source_package_carries_a_py_typed_marker() -> None: + assert _MARKER.is_file(), ( + "src/tods_validate/py.typed is missing. Without it every downstream " + "type checker treats this package as untyped, whatever mypy --strict " + "says about it here." + ) + + +def test_the_marker_is_where_an_installed_consumer_looks_for_it() -> None: + """The marker has to sit next to the installed package, not only in the repo. + + `find_spec` resolves the same way an importing consumer does, so this + passes for an editable install and for a wheel install and fails if the + file stops being packaged. + """ + spec = importlib.util.find_spec("tods_validate") + assert spec is not None + assert spec.origin is not None + installed = Path(spec.origin).parent + assert (installed / "py.typed").is_file(), ( + f"tods_validate is installed at {installed} with no py.typed beside it; " + "the marker exists in the repository but is not being packaged" + ) + + +def test_a_consumer_can_type_check_against_this_package(tmp_path: Path) -> None: + """The property, rather than a restatement of the file's existence. + + Run from `tmp_path` so mypy does not pick up this repository's own + configuration (`files = ["src"]`) and check `src/` instead of the consumer. + """ + consumer = tmp_path / "consumer.py" + consumer.write_text( + textwrap.dedent( + """\ + from pathlib import Path + + from tods_validate import validate_feed + + report = validate_feed(Path("feed")) + print(report) + """ + ), + encoding="utf-8", + ) + + result = subprocess.run( + [sys.executable, "-m", "mypy", "--strict", "consumer.py"], + cwd=tmp_path, + capture_output=True, + text=True, + ) + assert "import-untyped" not in result.stdout, ( + "a consumer type-checking against tods_validate is told the package is " + f"untyped:\n{result.stdout}" + ) + assert result.returncode == 0, ( + f"mypy --strict failed on a minimal consumer:\n{result.stdout}\n{result.stderr}" + )