diff --git a/.github/test-count-baseline b/.github/test-count-baseline new file mode 100644 index 00000000..b22803ae --- /dev/null +++ b/.github/test-count-baseline @@ -0,0 +1 @@ +1957 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f12acfc..6fba7e15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,25 @@ jobs: uv run --frozen pytest --cov=cgis --cov=scripts --cov-report=term-missing --cov-report=xml + - name: Test-count floor + # Deliberately a workflow step and not a test (#405). On 2026-08-17 a + # stale-tree push deleted 24 test files and 5 modules under src/cgis, + # and the step above passed: the tests that would have failed went with + # them. Every floor the repository had lived inside a test file, so the + # push that tripped them also removed them. + # + # The floor is read from the *base branch*, not from this checkout — a + # stale tree carries a stale baseline, and comparing the branch against + # its own copy passes the exact incident above. `fetch-depth: 0` on the + # checkout is what makes origin/ readable here. + # + # `github.base_ref` is set on pull_request only; on a push to the trunk + # it is empty and ref_name is the branch being pushed, which is the + # right comparison there too. + run: > + uv run --frozen python scripts/check_test_count.py + --base-ref "origin/${{ github.base_ref || github.ref_name }}" + - name: Project version for Sonar id: version # Without this every analysis records the version as the literal string diff --git a/scripts/check_test_count.py b/scripts/check_test_count.py new file mode 100644 index 00000000..0836a93c --- /dev/null +++ b/scripts/check_test_count.py @@ -0,0 +1,317 @@ +"""A floor under the size of the test suite itself (#405). + +On 2026-08-17 a bot pushed a stale worktree over a pull request. It committed a +whole tree rather than a diff, so a week of `main` read as deletion: 62 files, +5 modules under `src/cgis` and 24 test files. **Python Verification passed** — +because the tests that would have failed were deleted by the same commit. To +CI, "the module and its tests are gone" and "both still pass" are one +observation. + +The corpus floors already in the repository (`>= 72` in +`test_recordings_from_corpus.py`, `>= 16` in +`test_backfill_calibration_fingerprint.py`) were written against exactly this +shape of silence and could not help: they live inside test files, and this +failure deletes the file. A guard shipped inside the thing it guards is removed +by the event it exists to catch. So this runs from `ci.yml`, not from pytest. + +**The baseline is read twice, from two places, and that is the load-bearing +part.** The floor comes from the *base branch*; only the ceiling consults the +copy in the pull request. A stale tree carries a stale baseline — in the +incident, a tree from six days earlier held roughly the count it had then, so a +check against the branch's own file would have compared 1264 against ~1264 and +passed. The base branch held 1932. + +The point generalises past this repository: a reference value stored inside the +subject cannot bound the subject. +""" + +import argparse +import re +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +#: One line, one integer. Under `.github/` rather than `tests/` so that deleting +#: the test tree does not also delete the record of how large it was. +BASELINE_PATH = Path(".github/test-count-baseline") + +#: How far the real count may run ahead of the recorded baseline before the +#: baseline must be raised. Without a ceiling the floor rots: a suite that grows +#: to 3000 against a baseline of 1932 would let a thousand tests be deleted +#: silently, which is the failure this file exists to prevent, arriving late. +MAX_DRIFT = 150 + +#: `pytest --collect-only -q` ends in one of three shapes, measured on this +#: repository rather than assumed: +#: +#: 1932 tests collected in 1.83s +#: 137/1932 tests collected (1785 deselected) in 1.95s +#: no tests collected in 0.00s +#: +#: The third yields no number at all. It must raise rather than fall back to a +#: default, because every plausible default (0, or "skip the check") turns a +#: broken command into a passing one — and a floor that cannot fail is worse +#: than no floor, since it reads as protection. +_COLLECTED = re.compile(r"^(?:(\d+)/)?(\d+) tests? collected", re.M) + + +class CannotCountError(RuntimeError): + """Raised when the number of tests cannot be established. + + Its own type so a caller cannot confuse "the suite shrank" with "we never + found out how big it is". The first is a finding; the second is a broken + check, and reporting one as the other is the whole defect class here. + """ + + +def parse_collected(output: str) -> int: + """The total pytest collected, from the summary line. + + Returns the *total*, not the selected count: under `-k` the line reads + `137/1932`, and 137 is how many matched a filter rather than how many exist. + A floor fed the filtered number would fail on every filtered run and, worse, + would pass a real deletion whenever the filter happened to be narrow. + + The **last** match wins. pytest's summary is the final line, so anything + else matching came earlier and is not the answer — a plugin or a warning + printing something count-shaped would otherwise be read as the count. + Raised in review of #409. + """ + matches = list(_COLLECTED.finditer(output)) + if not matches: + _msg = ( + "Could not find a collected-test count in pytest's output. The last line is " + "normally ' tests collected'; 'no tests collected' means collection failed " + "and is not a count of zero.\n" + f"--- output ---\n{output.strip()[-2000:]}" + ) + raise CannotCountError(_msg) + return int(matches[-1].group(2)) + + +def _run(command: list[str], repo_root: Path) -> subprocess.CompletedProcess[str]: + """Run a command, turning a missing executable into this module's refusal. + + Without this, an absent `uv` or `git` raises `FileNotFoundError`, Python + exits 1, and **1 is the code for "the suite shrank"** — so a machine without + the toolchain would report as a deletion. Telling "we could not find out" + apart from "tests are missing" is the entire job of this file, and it cannot + make that distinction everywhere except in its own plumbing. Raised in + review of #409. + """ + try: + return subprocess.run(command, cwd=repo_root, capture_output=True, text=True, check=False) + except FileNotFoundError as exc: + _msg = f"Could not run {command[0]!r}: {exc.strerror}. It must be on PATH." + raise CannotCountError(_msg) from exc + + +def collect_count(repo_root: Path) -> int: + """How many tests pytest can collect right now.""" + # Not `check=True`: a non-zero exit with a usable summary line is possible, + # and the exit code is not the question being asked. The parse decides, and + # it refuses loudly when there is nothing to parse. + result = _run(["uv", "run", "--frozen", "pytest", "--collect-only", "-q"], repo_root) + # stdout first, and only then the two joined. pytest writes its summary to + # stdout while `uv` writes its own chatter to stderr, and stderr is appended + # *after* — so "take the last match" is right within a stream and wrong + # across them. The joined form is still what a failure reports, so the + # refusal carries both. + try: + return parse_collected(result.stdout) + except CannotCountError: + return parse_collected(result.stdout + result.stderr) + + +#: What may be handed to git as a revision. Narrower than git's own rules, but +#: wide enough for the forms people actually type: `main`, `origin/main`, `HEAD`, +#: a sha, `release/1.2`, and the relative ones — `HEAD~1`, `HEAD^`, `main@{u}` +#: minus the braces. The relative characters were added in review of #409; +#: `origin/` is what CI passes, but `HEAD~1` is what a person debugging +#: this locally reaches for, and `~ ^ @` introduce no option and no shell. +#: +#: The leading dash is the point, and the pattern now says so directly. An +#: earlier version required the first character to be alphanumeric, which is a +#: stricter rule wearing the same clothes — it also rejected `@`, a legal +#: revision meaning HEAD, as its own test discovered. Git validates the rest; +#: what git cannot do is tell that an argument was meant as a revision, because +#: a value beginning with `-` reaches it as a *flag*. Same argument-injection +#: shape `evidence.py` guards against on changed-file paths. +#: +#: Nothing here is passed through a shell, so this is not about quoting; it is +#: that a string which never looked like a ref has no business reaching a +#: subprocess, exploitable or not (pythonsecurity:S8705). +_REF = re.compile(r"^(?!-)[A-Za-z0-9._/~^@-]+$") + + +def _validated_ref(ref: str) -> str: + """`ref` unchanged, or a refusal — never a sanitised version of it. + + Rejected rather than stripped: silently rewriting `--upload-pack=x` into + something git accepts would compare against a ref nobody asked for, and this + whole file exists to stop a check reporting on the wrong subject. + """ + if not _REF.match(ref): + _msg = ( + f"{ref!r} is not a usable git revision. It must not begin with `-`, and may hold " + f"only letters, digits, and `._/~^@-` — a value beginning with `-` reaches git as " + f"an option rather than a revision." + ) + raise CannotCountError(_msg) + return ref + + +def baseline_on(ref: str, repo_root: Path) -> int | None: + """The baseline as recorded on `ref`, or None when `ref` predates the file. + + None is not a fallback and not a silent pass. It says there is no prior + count to compare against, and a shrink is undetectable without a prior — the + honest report of that is "this rule cannot fire", not a number. It happens + for exactly two reasons, both self-limiting: the commit that introduces the + baseline, and branches cut before it. Removing the file later cannot get + here, because a branch that deletes it fails on `baseline_here` first. + + A ref that does not resolve at all is a different thing entirely — a broken + workflow, not a young repository — and raises. The two are told apart by + resolving the ref first rather than by matching git's error text, which + would not distinguish "unknown ref" from "path not in this tree". + """ + ref = _validated_ref(ref) + resolved = _run(["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], repo_root) + if resolved.returncode: + _msg = ( + f"{ref} does not resolve. The floor has to come from the base branch, so a " + f"missing base ref is a broken workflow rather than a young repository — " + f"falling back to this branch's own copy is the evasion this check closes." + ) + raise CannotCountError(_msg) + result = _run(["git", "show", f"{ref}:{BASELINE_PATH.as_posix()}"], repo_root) + if result.returncode: + return None + return _as_count(result.stdout, f"{ref}:{BASELINE_PATH}") + + +def baseline_here(repo_root: Path) -> int: + """The baseline as it stands in the working tree — the copy a branch may raise.""" + path = repo_root / BASELINE_PATH + if not path.is_file(): + _msg = f"{path} is missing. It records how large the suite is expected to be." + raise CannotCountError(_msg) + return _as_count(path.read_text(encoding="utf-8"), str(path)) + + +def _as_count(raw: str, source: str) -> int: + """One integer, or a refusal naming where the unusable value came from.""" + text = raw.strip() + if not text.isdigit(): + _msg = f"{source} does not hold a plain integer: {text[:80]!r}" + raise CannotCountError(_msg) + return int(text) + + +def problems(actual: int, floor: int | None, here: int) -> list[str]: + """Every rule broken, in the order a reader wants them. + + Four rules, each closing a different route: + + 1. `actual >= floor` — the suite may not shrink below what the base branch + recorded. This is the anti-deletion rule, and it uses the base branch's + number so that editing the file in the branch cannot defeat it. + 2. `here >= floor` — the recorded baseline may only go up. Otherwise a + branch lowers it, merges, and every later branch inherits a weaker floor. + 3. `here <= actual` — the baseline may not claim more tests than exist, or + the next branch starts red through no fault of its own. + 4. `actual - here <= MAX_DRIFT` — the baseline has to be kept current, or it + stops bounding anything. + + Rules 3 and 4 are about maintaining the file and are suppressed while rule 1 + is broken. When tests really have gone missing, "the baseline claims more + than exists" is a consequence of the deletion, not a second problem — and + its remedy reads as *lower the baseline*, which is precisely the wrong move + to put beside "668 tests have gone missing". + + A `floor` of None disables rules 1 and 2, and nothing else. There is no + prior count on the base branch, so a shrink is not merely undetected but + undefined — the caller says so on stderr rather than letting a green step + imply a floor was applied. + """ + found: list[str] = [] + if floor is not None: + if actual < floor: + found.append( + f"The suite collects {actual} tests; {floor} were recorded on the base " + f"branch. {floor - actual} have gone missing. Deleting a module together " + f"with its tests leaves CI green, which is why this is checked separately " + f"from whether the tests pass." + ) + if here < floor: + found.append( + f"{BASELINE_PATH} was lowered from {floor} to {here}. It may only rise: a " + f"lowered floor is inherited by every branch cut afterwards." + ) + if actual < floor: + return found + if here > actual: + found.append( + f"{BASELINE_PATH} records {here} but only {actual} tests are collected, so the " + f"next branch would start red. Lower it to {actual} or restore the tests." + ) + elif actual - here > MAX_DRIFT: + found.append( + f"The suite has grown to {actual}, more than {MAX_DRIFT} past the recorded " + f"{here}. Raise {BASELINE_PATH} to {actual} so the floor keeps bounding " + f"something." + ) + return found + + +def check(base_ref: str, repo_root: Path) -> tuple[list[str], bool]: + """Problems to report, and whether the anti-deletion floor was actually in force. + + The flag travels with the result rather than being inferred by the caller. + A clean run with no floor and a clean run with a floor are different + outcomes, and only one of them means the suite did not shrink. + """ + floor = baseline_on(base_ref, repo_root) + found = problems(collect_count(repo_root), floor, baseline_here(repo_root)) + return found, floor is not None + + +def main() -> int: + """Print every problem and return a shell exit code.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--base-ref", + default="origin/main", + help="where the floor is read from; must not be the branch under test", + ) + parser.add_argument("--repo-root", type=Path, default=REPO_ROOT) + args = parser.parse_args() + try: + found, floor_applied = check(args.base_ref, args.repo_root) + except CannotCountError as exc: + # Reported as a failure, never as a pass. "We could not tell" and "the + # suite is intact" are different answers, and only one of them is this + # check's job to give. + print(f"Test-count floor could not run: {exc}", file=sys.stderr) + return 2 + if not floor_applied: + # Said out loud on every such run. A step that goes green while the + # anti-deletion rule never ran looks identical in the log to one where + # it ran and passed, and that is the confusion this whole file exists + # to remove. + print( + f"NOTE: {args.base_ref} carries no {BASELINE_PATH}, so there is no prior count " + f"and the anti-deletion floor did not apply. Expected only while the baseline " + f"is being introduced, or on a branch cut before it.", + file=sys.stderr, + ) + for problem in found: + print(problem, file=sys.stderr) + return 1 if found else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_check_test_count.py b/tests/unit/test_check_test_count.py new file mode 100644 index 00000000..29e94d25 --- /dev/null +++ b/tests/unit/test_check_test_count.py @@ -0,0 +1,427 @@ +"""The floor under the suite's own size (#405). + +These tests cover the script; they are not the guard. The guard is the step in +`ci.yml`, because a check that ships inside the test tree is deleted by the push +it exists to catch — which is what happened on 2026-08-17, when 24 test files +and 5 modules went missing and Python Verification passed. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent / "scripts")) + +import check_test_count +from check_test_count import ( + BASELINE_PATH, + MAX_DRIFT, + CannotCountError, + _as_count, + _validated_ref, + baseline_here, + baseline_on, + check, + collect_count, + main, + parse_collected, + problems, +) + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _commit_repo(root: Path, baseline: str | None = None) -> None: + """A one-commit git repository, with or without the baseline file in it.""" + subprocess.run(["git", "init", "-q"], cwd=root, check=True) + subprocess.run(["git", "config", "user.email", "t@t"], cwd=root, check=True) + subprocess.run(["git", "config", "user.name", "t"], cwd=root, check=True) + (root / "seed.txt").write_text("x", encoding="utf-8") + if baseline is not None: + (root / BASELINE_PATH.parent).mkdir(parents=True, exist_ok=True) + (root / BASELINE_PATH).write_text(baseline, encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=root, check=True) + subprocess.run(["git", "commit", "-qm", "seed"], cwd=root, check=True) + + +class TestParsingPytestsSummary: + """Three shapes, measured on this repository rather than assumed.""" + + def test_the_ordinary_line(self) -> None: + assert parse_collected("1932 tests collected in 1.83s") == 1932 + + def test_a_filtered_run_reports_the_total_not_the_selection(self) -> None: + """`137/1932` under `-k`: 137 matched a filter, 1932 exist. + + Reading the left number would fail every filtered run, and — worse — + would pass a real deletion whenever the filter happened to be narrow + enough that the shrunken suite still cleared the floor. + """ + assert parse_collected("137/1932 tests collected (1785 deselected) in 1.95s") == 1932 + + def test_a_single_test_still_parses(self) -> None: + """pytest writes 'test' rather than 'tests' at one, so the plural is optional.""" + assert parse_collected("1 test collected in 0.01s") == 1 + + def test_no_tests_collected_refuses_rather_than_returning_zero(self) -> None: + """The line that carries no number at all. + + Zero would be a count, and a count of zero compared against a floor + fails loudly — which sounds safe until the caller catches it. The real + danger is the other plausible default, "skip the check", so this refuses + with its own exception type instead of returning anything. + """ + with pytest.raises(CannotCountError, match="not a count of zero"): + parse_collected("no tests collected in 0.00s") + + def test_the_last_match_wins(self) -> None: + """pytest's summary is the final line; anything earlier is not the answer. + + Raised in review of #409. A plugin or a warning printing something + count-shaped would otherwise be read as the count, and the floor would + then bound the wrong number in whichever direction that number happened + to fall. + """ + output = "10 tests collected\nsome plugin chatter\n1932 tests collected in 1.83s" + assert parse_collected(output) == 1932 + + def test_the_refusal_carries_the_output_it_could_not_parse(self) -> None: + """A "could not run" with no output is a bug report nobody can action.""" + with pytest.raises(CannotCountError, match="ImportError: cannot import name"): + parse_collected("ImportError: cannot import name 'x' from 'y'") + + +class TestReadingTheBaselines: + """Where each number comes from, and what happens when it cannot be had.""" + + def test_the_working_tree_copy_is_read(self, tmp_path: Path) -> None: + (tmp_path / BASELINE_PATH.parent).mkdir(parents=True) + (tmp_path / BASELINE_PATH).write_text("1932\n", encoding="utf-8") + assert baseline_here(tmp_path) == 1932 + + def test_a_missing_file_refuses(self, tmp_path: Path) -> None: + with pytest.raises(CannotCountError, match="is missing"): + baseline_here(tmp_path) + + @pytest.mark.parametrize("body", ["", " ", "about 1932", "1932 tests", "-5"]) + def test_anything_but_a_plain_integer_refuses(self, body: str) -> None: + """Including the near-misses, which are the ones that would parse wrong.""" + with pytest.raises(CannotCountError, match="does not hold a plain integer"): + _as_count(body, "somewhere") + + def test_a_base_ref_that_does_not_resolve_refuses(self, tmp_path: Path) -> None: + """A broken workflow, not a young repository. + + No falling back to the branch's own copy: a stale tree carries a stale + baseline, so comparing a branch against itself passes the exact incident + this check was written for. + """ + subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True) + with pytest.raises(CannotCountError, match="does not resolve"): + baseline_on("origin/main", tmp_path) + + def test_a_ref_that_predates_the_baseline_returns_none_rather_than_raising( + self, tmp_path: Path + ) -> None: + """The bootstrap commit, and every branch cut before it. + + Told apart from the unresolvable ref above by resolving the ref first, + not by matching git's error text — which would not distinguish "unknown + ref" from "path not in this tree", the same defect #375 found in a + `git show` reader. + """ + _commit_repo(tmp_path) + assert baseline_on("HEAD", tmp_path) is None + + def test_a_ref_that_carries_the_baseline_returns_it(self, tmp_path: Path) -> None: + _commit_repo(tmp_path, baseline="1932\n") + assert baseline_on("HEAD", tmp_path) == 1932 + + @pytest.mark.parametrize( + "ref", + [ + pytest.param("--upload-pack=touch /tmp/x", id="reads-as-an-option"), + pytest.param("-main", id="leading-dash"), + pytest.param("main; rm -rf /", id="shell-metacharacters"), + pytest.param("main$(id)", id="substitution"), + pytest.param("", id="empty"), + ], + ) + def test_a_ref_that_is_not_a_ref_never_reaches_git(self, ref: str) -> None: + """Refused before the subprocess, and refused rather than sanitised. + + Nothing here goes through a shell, so the metacharacter cases are not + exploitable — but the leading-dash case is real: git reads it as an + option. Stripping it instead would compare against a revision nobody + asked for, which is this file's own failure mode. + """ + with pytest.raises(CannotCountError, match="not a usable git revision"): + baseline_on(ref, REPO_ROOT) + + @pytest.mark.parametrize( + "ref", + [ + "main", + "origin/main", + "HEAD", + "release/1.2", + "98380bc", + # Relative forms, admitted in review of #409. CI passes + # `origin/`, but these are what a person debugging locally + # types, and none of them introduces an option or a shell. + "HEAD~1", + "HEAD^", + "origin/main~2", + "@", + ], + ) + def test_the_refs_actually_used_are_accepted(self, ref: str) -> None: + """The guard must not be so tight that it rejects the real inputs.""" + assert _validated_ref(ref) == ref + + +class TestTheFourRules: + """Each rule closes a different route, so each is asserted on its own.""" + + def test_a_suite_at_its_floor_is_clean(self) -> None: + assert problems(actual=1932, floor=1932, here=1932) == [] + + def test_growth_within_the_drift_allowance_is_clean(self) -> None: + assert problems(actual=1932 + MAX_DRIFT, floor=1932, here=1932) == [] + + def test_a_shrunken_suite_is_reported_with_the_delta(self) -> None: + """The incident, in miniature: 1932 recorded, 1264 collected.""" + found = problems(actual=1264, floor=1932, here=1932) + assert len(found) == 1 + assert "668 have gone missing" in found[0] + + def test_the_shrink_is_caught_even_when_the_branch_lowered_its_own_baseline(self) -> None: + """The load-bearing case, and the reason the floor is read from the base branch. + + A stale tree carries a stale baseline. Checked against its own copy this + is 1264 against 1264 and passes; against the base branch's 1932 it + fails. Both rules fire here — the deletion and the lowering. + """ + found = problems(actual=1264, floor=1932, here=1264) + assert len(found) == 2 + assert any("gone missing" in p for p in found) + assert any("may only rise" in p for p in found) + + def test_lowering_the_baseline_alone_is_refused(self) -> None: + """Tests intact, floor quietly weakened for every branch cut afterwards.""" + found = problems(actual=1932, floor=1932, here=1900) + assert len(found) == 1 + assert "may only rise" in found[0] + + def test_a_baseline_above_the_real_count_is_refused(self) -> None: + """Otherwise the next branch starts red through no fault of its own.""" + found = problems(actual=1932, floor=1900, here=1950) + assert len(found) == 1 + assert "would start red" in found[0] + + def test_a_baseline_left_far_behind_is_refused(self) -> None: + """The anti-rot rule: a floor 1000 below reality bounds nothing.""" + found = problems(actual=1932 + MAX_DRIFT + 1, floor=1932, here=1932) + assert len(found) == 1 + assert "keeps bounding something" in found[0] + + def test_no_prior_disables_the_first_two_rules_and_nothing_else(self) -> None: + """`floor=None` is "undefined", not "zero" and not "fine". + + A shrink cannot be detected without a prior count, so rules 1 and 2 go + quiet — but the file must still be honest about the suite in front of + it, or the bootstrap commit could record any number at all and the floor + would start life wrong. + """ + assert problems(actual=1932, floor=None, here=1932) == [] + assert problems(actual=10, floor=None, here=99) != [] + assert problems(actual=1932 + MAX_DRIFT + 1, floor=None, here=1932) != [] + + def test_drift_is_not_reported_when_the_baseline_is_already_too_high(self) -> None: + """The two are mutually exclusive by construction, and both name a fix. + + Reported together they would tell the operator to raise and lower the + same number in one message. + """ + found = problems(actual=100, floor=50, here=400) + assert len(found) == 1 + + +class _Ran: + """A stand-in for `subprocess.run`'s result, carrying only what is read.""" + + def __init__(self, stdout: str = "", stderr: str = "", returncode: int = 0) -> None: + self.stdout = stdout + self.stderr = stderr + self.returncode = returncode + + +class _stub_subprocess: # noqa: N801 — stands in for a module, so it is named like one + """A `subprocess` module whose `run` always returns the same result.""" + + def __init__(self, result: _Ran) -> None: + self._result = result + + def run(self, *_args: object, **_kwargs: object) -> _Ran: + return self._result + + +class TestCollectingTheCount: + """The one place that shells out to pytest.""" + + def test_the_summary_is_taken_from_either_stream(self, monkeypatch: pytest.MonkeyPatch) -> None: + """stdout and stderr are joined before parsing. + + pytest writes the summary to stdout, but `uv run` prepends its own + chatter on stderr and a plugin can move things about. Reading only one + stream would turn a working checkout into "could not count". + """ + monkeypatch.setattr( + check_test_count.subprocess, "run", lambda *_a, **_k: _Ran(stderr="7 tests collected") + ) + assert collect_count(Path()) == 7 + + def test_stdout_outranks_stderr_when_both_carry_a_count( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Why "take the last match" is right within a stream and wrong across two. + + pytest writes its summary to stdout and `uv` writes its chatter to + stderr, and the two are joined with stderr last — so a naive last-match + over the joined text would let stderr outrank the real summary. Reading + stdout first removes the question. + """ + monkeypatch.setattr( + check_test_count, + "subprocess", + _stub_subprocess(_Ran(stdout="1932 tests collected", stderr="7 tests collected")), + ) + assert collect_count(Path()) == 1932 + + def test_a_missing_executable_is_a_could_not_tell_not_a_shrink( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Absent `uv` or `git` must not exit 1, which is the code for a deletion. + + Raised in review of #409, and the sharpest of the six: an uncaught + FileNotFoundError exits 1, and 1 means "tests have gone missing". A + machine without the toolchain would have reported as a deletion — the + one confusion this module exists to prevent, in its own plumbing. + """ + + def _absent(*_a: object, **_k: object) -> None: + raise FileNotFoundError(2, "No such file or directory") + + monkeypatch.setattr(check_test_count.subprocess, "run", _absent) + root = Path() + with pytest.raises(CannotCountError, match="must be on PATH"): + collect_count(root) + + def test_a_broken_collection_refuses_and_shows_the_output( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Exit code is not the question; whether a number came back is. + + A non-zero exit with a usable summary is possible, and a zero exit with + none is too — so the parse decides, and the refusal carries the text so + the failure is actionable from the CI log alone. + """ + monkeypatch.setattr( + check_test_count.subprocess, + "run", + lambda *_a, **_k: _Ran(stdout="no tests collected in 0.00s", returncode=4), + ) + root = Path() + with pytest.raises(CannotCountError, match="not a count of zero"): + collect_count(root) + + +class TestTheCliContract: + """Exit codes, and the line that says whether the floor was in force.""" + + def _wire( + self, + monkeypatch: pytest.MonkeyPatch, + *, + actual: int, + floor: int | None, + here: int, + ) -> None: + monkeypatch.setattr(check_test_count, "collect_count", lambda _r: actual) + monkeypatch.setattr(check_test_count, "baseline_on", lambda _ref, _r: floor) + monkeypatch.setattr(check_test_count, "baseline_here", lambda _r: here) + monkeypatch.setattr(sys, "argv", ["check_test_count.py"]) + + def test_check_reports_whether_the_floor_applied(self, monkeypatch: pytest.MonkeyPatch) -> None: + """The flag travels with the result instead of being re-derived. + + A clean run with no floor and a clean run with a floor are different + outcomes, and both produce an empty problem list. + """ + self._wire(monkeypatch, actual=1957, floor=1932, here=1932) + assert check("origin/main", Path()) == ([], True) + self._wire(monkeypatch, actual=1957, floor=None, here=1932) + assert check("origin/main", Path()) == ([], False) + + def test_a_healthy_suite_exits_zero_and_says_nothing( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + self._wire(monkeypatch, actual=1957, floor=1932, here=1932) + assert main() == 0 + assert capsys.readouterr().err == "" + + def test_a_shrunken_suite_exits_one_and_prints_the_delta( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + self._wire(monkeypatch, actual=1264, floor=1932, here=1932) + assert main() == 1 + assert "668 have gone missing" in capsys.readouterr().err + + def test_a_missing_prior_is_announced_on_an_otherwise_green_run( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """Green because nothing is wrong, and loud because nothing was checked. + + Without the notice this run is indistinguishable in the log from one + where the anti-deletion rule ran and passed — which is the confusion the + whole file exists to remove. + """ + self._wire(monkeypatch, actual=1957, floor=None, here=1957) + assert main() == 0 + assert "the anti-deletion floor did not apply" in capsys.readouterr().err + + def test_being_unable_to_count_exits_two_rather_than_zero( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """ "We could not tell" is not "the suite is intact". + + Its own exit code so a caller cannot read the failure as a pass, and so + it is distinguishable from a genuine shrink at exit 1. + """ + + def _boom(_r: Path) -> int: + _msg = "pytest exploded" + raise CannotCountError(_msg) + + self._wire(monkeypatch, actual=0, floor=1932, here=1932) + monkeypatch.setattr(check_test_count, "collect_count", _boom) + assert main() == 2 + assert "could not run" in capsys.readouterr().err + + +def test_the_repository_currently_satisfies_its_own_floor() -> None: + """The recorded baseline is not above what this checkout actually collects. + + Deliberately not a call to `collect_count`: that shells out to a second full + collection, and the number is already known to the run collecting this test. + What can be checked cheaply is the invariant that breaks first — a baseline + committed ahead of reality, which would redden the next branch. + """ + recorded = baseline_here(REPO_ROOT) + assert recorded > 0 + assert recorded >= 1932, ( + f"{BASELINE_PATH} records {recorded}; it may only rise, and 1932 was the count when " + f"the floor was introduced." + )