From cef43ecd7dc63d58898345310d4a273a72410535 Mon Sep 17 00:00:00 2001 From: srpatcha Date: Sun, 30 Aug 2026 18:19:33 -0700 Subject: [PATCH] fix(cli): a test run that executed nothing is not a pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ebuild test` trusted the runner's exit status: result = subprocess.run(argv, cwd=str(cwd)) if result.returncode != 0: raise SystemExit(result.returncode) log.success("All tests passed.") ctest exits 0 when it finds nothing to run. A CMakeLists.txt with enable_testing() and no add_test() still produces a CTestTestfile.cmake, so _resolve_test_runner() finds ctest, ctest prints "No tests were found!!!", exits 0, and the wrapper calls that success. Measured on master, project with a configured build tree and zero tests: $ ctest --test-dir _build No tests were found!!! ctest exit=0 $ ebuild test [ok] All tests passed. exit=0 That is worse than having no test command, because it actively reassures. Anyone wiring this into CI gets a green gate over a repository that has stopped testing itself, and nothing anywhere says so. An empty run is now a failure. Two independent checks, because neither is reliable alone: the marker phrase catches ctest, which prints no summary at all in that case, and a zero total catches a runner that prints a well-formed summary adding up to nothing. Output is now captured rather than inherited — the exit status alone cannot tell "every test passed" from "there were no tests" — and echoed so the terminal reads as it did before. Counts are reported when the runner printed a summary this parser recognises, and withheld when it did not. A number derived from a zero exit status is a guess presented as a measurement. make has no standard summary format, so its counts stay unknown and the exit status carries the verdict. zero tests [error] ctest completed without running a single test. exit=1 1 pass 1 fail [error] Tests failed (ctest exited 8). exit=8 1 pass 0 fail [ok] All tests passed (1 passed, 0 failed). exit=0 Also adds a check that every ebuild command shown in the docs exists. Reads only fenced code blocks, since prose says things like "ebuild is a unified build system" and a plain grep yields "ebuild and", "ebuild can". Verified it fails rather than merely existing: putting `ebuild verify` in the README turns it red. tests 287 -> 307 passed, 1 skipped, 0 failed Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 12 +++ ebuild/cli/commands.py | 81 +++++++++++++++- tests/unit/test_documented_commands_exist.py | 95 +++++++++++++++++++ .../unit/test_empty_test_run_is_not_a_pass.py | 87 +++++++++++++++++ 4 files changed, 273 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_documented_commands_exist.py create mode 100644 tests/unit/test_empty_test_run_is_not_a_pass.py diff --git a/README.md b/README.md index 1fc890d2..421a3ae7 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ not enough, because ebuild invokes `python -m ninja`. ```bash ebuild info # show what ebuild parsed from build.yaml ebuild build # resolve and build (auto-detects the backend) +ebuild test # run the project's tests ebuild build -j 4 # build independent packages concurrently ebuild clean # remove build artifacts ebuild --version @@ -95,6 +96,17 @@ Additional commands: `configure`, `install`, `add`, `list-packages`, (`status`, `update`, `set-url`, `set-branch`, `link`, `unlink`). Run `ebuild --help` for the full list. + +`ebuild test` reports the counts the underlying runner printed, and reports none +when it printed nothing recognisable — a number inferred from a zero exit status +is a guess presented as a measurement. + +It also treats a run that executed **no tests** as a failure. `ctest` exits `0` +when it finds nothing to run, and a `CMakeLists.txt` with `enable_testing()` and +no `add_test()` still produces a `CTestTestfile.cmake` — so the runner is found, +ctest prints `No tests were found!!!`, and trusting the exit status would report +a green suite for a project with no tests at all. + ## Test ```bash diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index 3c4fed9e..cb61c308 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -2225,7 +2225,10 @@ def test(log: Logger, config_path: str, build_dir: str, log.info(" ".join(argv)) try: - result = subprocess.run(argv, cwd=str(cwd)) + # Captured rather than inherited, because the exit status alone cannot + # distinguish "every test passed" from "there were no tests". The + # output is echoed below so the terminal reads as it did before. + result = subprocess.run(argv, cwd=str(cwd), capture_output=True, text=True) except FileNotFoundError: log.error( f"{name} is not installed or not on PATH, so the tests cannot be " @@ -2233,11 +2236,31 @@ def test(log: Logger, config_path: str, build_dir: str, ) raise SystemExit(1) + output = (result.stdout or "") + (result.stderr or "") + if output: + click.echo(output.rstrip()) + if result.returncode != 0: log.error(f"Tests failed ({name} exited {result.returncode}).") raise SystemExit(result.returncode) - log.success("All tests passed.") + # ctest exits 0 when it finds nothing to run. A CMakeLists with + # enable_testing() and no add_test() produces a CTestTestfile.cmake, so the + # runner is found, ctest prints "No tests were found!!!", exits 0, and the + # only honest reading of that is not "All tests passed". + if _ran_no_tests(name, output): + log.error(f"{name} completed without running a single test.") + log.info(" A pass here would mean nothing; treating it as a failure.") + raise SystemExit(1) + + counts = _parse_test_counts(name, output) + if counts is not None: + passed, failed = counts + log.success(f"All tests passed ({passed} passed, {failed} failed).") + else: + # No recognised summary. Report the verdict without inventing a number + # the runner did not print. + log.success("All tests passed.") def _run_native_tests( @@ -2300,6 +2323,60 @@ def _run_native_tests( log.success(f"All {len(selected)} test targets passed.") +#: What each runner prints when it completed having executed nothing. ctest's is +#: the one that matters: it pairs the message with a zero exit status. +_NO_TESTS_MARKERS = { + "ctest": ("No tests were found",), + "meson test": ("No tests defined",), + "cargo test": ("running 0 tests",), +} + +#: Each runner's own summary line, anchored to the phrasing it prints so that a +#: format change shows up as "no counts" rather than as a wrong number. +_TEST_COUNT_PATTERNS = { + "ctest": re.compile( + r"tests passed,\s*(?P\d+)\s+tests? failed out of\s*(?P\d+)"), + "meson test": re.compile( + r"^Ok:\s*(?P\d+).*?^Fail:\s*(?P\d+)", re.S | re.M), + "cargo test": re.compile( + r"test result:.*?(?P\d+) passed;\s*(?P\d+) failed"), +} + + +def _ran_no_tests(name: str, output: str) -> bool: + """True when the runner finished having executed nothing. + + Checked two ways because neither is reliable alone: the marker phrase + catches ctest, which prints no summary at all in this case, and the counts + catch a runner that prints a well-formed summary totalling zero. + """ + for marker in _NO_TESTS_MARKERS.get(name, ()): + if marker in output: + return True + counts = _parse_test_counts(name, output) + return counts is not None and counts[0] + counts[1] == 0 + + +def _parse_test_counts(name: str, output: str): + """(passed, failed) from the runner's own summary, or None. + + `make test` has no standard summary format. Rather than invent one, its + counts stay unknown and the exit status carries the verdict. + """ + pattern = _TEST_COUNT_PATTERNS.get(name) + if pattern is None: + return None + match = pattern.search(output) + if not match: + return None + groups = match.groupdict() + failed = int(groups["failed"]) + if groups.get("passed") is not None: + return int(groups["passed"]), failed + # ctest reports failures out of a total; passed is the remainder. + return int(groups["total"]) - failed, failed + + def _resolve_test_runner( source_dir: Path, build_dir: Path, diff --git a/tests/unit/test_documented_commands_exist.py b/tests/unit/test_documented_commands_exist.py new file mode 100644 index 00000000..375debe6 --- /dev/null +++ b/tests/unit/test_documented_commands_exist.py @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Every ebuild command shown in the docs must exist. + +§8 lists "consistent documentation generated from tested examples" as an MLP +requirement. Generating prose from examples is a large change; verifying that +the examples we already publish actually run is the part that stops the docs +lying, and it is cheap. + +The failure this prevents is real and has bitten this organisation. EoStudio +carried this line for the tool it is a front end for: + + f"ebuild --platform {platform} --config board.yaml {source_dir}" + +ebuild has no top-level --platform or --config. The string was stored in a dict +and never executed, so nothing found out it could not work. A published command +nobody runs decays the same way, and a developer following the README is the one +who discovers it. + +Only fenced code blocks are read. Prose says things like "ebuild is a unified +build system", and treating "is" as a subcommand would make this fail for no +reason. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +DOCS = ["README.md", "demo.md"] + +#: A shell line invoking ebuild, e.g. "ebuild configure --board stm32f4" or +#: "$ ebuild build". Captures the subcommand only. +_INVOCATION = re.compile(r"^\s*(?:\$\s*)?ebuild\s+([a-z][a-z0-9-]*)", re.M) + +#: Long options are not subcommands. +_NOT_A_SUBCOMMAND = {"--help", "--version"} + + +def _code_blocks(text: str): + """The contents of every fenced code block.""" + return re.findall(r"```[a-zA-Z]*\n(.*?)```", text, re.S) + + +def _documented_commands(): + """(subcommand, source file) for every ebuild invocation in the docs.""" + found = [] + for name in DOCS: + path = REPO_ROOT / name + if not path.is_file(): + continue + text = path.read_text(encoding="utf-8", errors="replace") + for block in _code_blocks(text): + for sub in _INVOCATION.findall(block): + if sub not in _NOT_A_SUBCOMMAND: + found.append((sub, name)) + return found + + +def _real_commands(): + """Every subcommand the CLI actually registers.""" + from ebuild.cli.commands import cli + return set(cli.commands) + + +def test_the_docs_actually_show_commands(): + # A guard on the guard: if the extraction silently stopped matching, every + # assertion below would pass vacuously and the check would be worthless. + documented = _documented_commands() + assert len(documented) >= 5, ( + "expected several ebuild invocations in the docs, found %d — the " + "extraction is probably broken rather than the docs being empty" + % len(documented)) + + +@pytest.mark.parametrize("subcommand,source", sorted(set(_documented_commands()))) +def test_a_documented_command_exists(subcommand, source): + real = _real_commands() + assert subcommand in real, ( + "%s documents `ebuild %s`, which the CLI does not provide. " + "Available: %s" % (source, subcommand, ", ".join(sorted(real)))) + + +def test_every_golden_path_step_is_a_real_command(): + # §7.1 names these eight verbatim. They are asserted directly rather than + # via the docs so the golden path cannot be quietly broken by editing the + # README instead of the CLI. + real = _real_commands() + for step in ("setup", "new", "configure", "build", + "test", "flash", "monitor"): + assert step in real, "golden path step `ebuild %s` is missing" % step diff --git a/tests/unit/test_empty_test_run_is_not_a_pass.py b/tests/unit/test_empty_test_run_is_not_a_pass.py new file mode 100644 index 00000000..ca3037e0 --- /dev/null +++ b/tests/unit/test_empty_test_run_is_not_a_pass.py @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""`ebuild test` must not report success for a project with no tests. + +ctest exits 0 when it finds nothing to run. A CMakeLists.txt with +`enable_testing()` and no `add_test()` still produces a CTestTestfile.cmake, so +`_resolve_test_runner()` finds ctest, ctest prints "No tests were found!!!", +exits 0, and trusting that exit status reports a green suite for a project +containing no tests at all. + +Measured on master before this change, against a project with a configured +build tree and zero registered tests: + + $ ctest --test-dir _build + No tests were found!!! + ctest exit=0 + + $ ebuild test + [ok] All tests passed. + exit=0 + +That is worse than having no test command, because it actively reassures. +""" + +import pytest + +from ebuild.cli.commands import _parse_test_counts, _ran_no_tests + + +class TestEmptyRunDetection: + def test_ctest_no_tests_marker(self): + assert _ran_no_tests("ctest", "No tests were found!!!") is True + + def test_meson_no_tests_marker(self): + assert _ran_no_tests("meson test", "No tests defined.") is True + + def test_cargo_zero_tests_marker(self): + assert _ran_no_tests("cargo test", "running 0 tests") is True + + def test_zero_totals_are_caught_without_a_marker(self): + # A runner that prints a well-formed summary adding up to nothing is + # the same situation by a different route. + assert _ran_no_tests("ctest", + "100% tests passed, 0 tests failed out of 0") is True + + def test_a_real_run_is_not_flagged(self): + assert _ran_no_tests("ctest", + "100% tests passed, 0 tests failed out of 17") is False + + def test_a_failing_run_is_not_flagged(self): + # Failures are already handled by the exit status; this must not + # reclassify them as "nothing ran". + assert _ran_no_tests("ctest", + "50% tests passed, 3 tests failed out of 6") is False + + def test_make_has_no_marker_and_is_not_flagged(self): + assert _ran_no_tests("make test", "anything at all") is False + + +class TestCountParsing: + """Counts come from the runner's own summary, never from the exit status.""" + + def test_ctest_summary(self): + assert _parse_test_counts( + "ctest", "100% tests passed, 0 tests failed out of 17") == (17, 0) + + def test_ctest_summary_with_failures(self): + assert _parse_test_counts( + "ctest", "50% tests passed, 3 tests failed out of 6") == (3, 3) + + def test_cargo_summary(self): + assert _parse_test_counts( + "cargo test", "test result: ok. 12 passed; 0 failed; 0 ignored") == (12, 0) + + def test_meson_summary(self): + out = "Ok: 12\nExpected Fail: 0\nFail: 2\n" + assert _parse_test_counts("meson test", out) == (12, 2) + + def test_make_has_no_standard_summary(self): + # Rather than invent a format for make, the counts stay unknown and the + # exit status carries the verdict. A made-up number would be worse than + # none. + assert _parse_test_counts("make test", "anything at all") is None + + def test_unrecognised_output_yields_no_counts(self): + assert _parse_test_counts("ctest", "something else entirely") is None