Repository navigation
fix(cli): a test run that executed nothing is not a pass - #89
Merged
Merged
Conversation
`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) <noreply@anthropic.com>
| That is worse than having no test command, because it actively reassures. | ||
| """ | ||
|
|
||
| import pytest |
srpatcha
pushed a commit
that referenced
this pull request
Sep 1, 2026
`import ebuild.cli.commands` raises NameError on master, so the `ebuild` command does not start and pytest cannot collect nine test modules -- the suite reports "9 errors during collection" and runs zero tests. Four PRs landed their tests while their implementation was discarded during conflict resolution, and the merges that did it were not gated by a run: f5209ac, current master, has no CI runs at all. #64 (6a22e36) tests/unit/test_golden_path_commands.py landed; _workspace_repo_paths, _cached_repo_libraries, _record_board_selection, _EOS_PROJECT_CONFIG and the `configure --board` option did not. #89 (32348f0) commands.py gained five `re.` uses, no `import re`. #90 (d3958f7) the initramfs test kept its assertions but lost its `_newc_members` helper and its `os` / `stat` imports. #92 registry.version_key was renamed version_sort_key without updating its caller in the tests. Restoring them, taken verbatim from the branches that were merged: * commands.py: `import re`; `_workspace_repo_paths`, `_cached_repo_libraries`, `_record_board_selection`, `_EOS_PROJECT_CONFIG`; the `--board` option on `configure` and the call that persists it; and the `{**_workspace_repo_paths(), **...}` merge at both `_install_packages` call sites. Without that last hunk a scaffolded project that `use`s eos or eboot gets no include or link paths from ~/.ebuild/repos, which is the "fatal error: eos/hal.h: No such file or directory" that #64 fixed. Without the option, the documented golden path -- `configure --board` then a bare `build` -- fails with "no such option: --board". * the initramfs test: `os` / `stat` imports and the `_newc_members` newc parser the assertions call. * test_package_registry.py: import the name the module now exports. Collection goes from 9 errors / 0 tests to 446 passing. The 12 that still fail are pre-existing and out of scope here: `doctor` (#72) and `package` (#77) are two more features whose implementations the same merges dropped and which should be restored by their authors, and the rest are separate bugs -- see the PR description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kartikey1306
added a commit
to Kartikey1306/ebuild
that referenced
this pull request
Sep 1, 2026
master does not parse. `ebuild` cannot start and pytest collects nothing:
$ python -c "import ebuild.cli.commands"
File "ebuild/cli/commands.py", line 855
"""ebuild — A unified embedded OS build system."""
^
SyntaxError: invalid character '—' (U+2014)
Line 855 is a normal docstring. The real damage is 300 lines earlier:
_board_config() lost its body and its closing quotes to a copy of
_resolve_backend_request()'s last line, spliced in during a conflict
resolution:
def _board_config() -> Optional[Dict[str, Any]]:
"""The project's own board description, if it ships one.
return resolved_backend, backend_config
Its docstring never closes, so every subsequent triple-quote toggles the
wrong way and the first em-dash the tokenizer reaches outside a string is
reported instead -- at line 855, in a function that is fine.
_resolve_backend_request itself still has its own return, so the line is
a duplicate rather than a move.
Restores the body from f5209ac, the last commit where this file parsed.
The same resolution also dropped the whole of embeddedos-org#89 -- _NO_TESTS_MARKERS,
_TEST_COUNT_PATTERNS, _ran_no_tests, _parse_test_counts and both call
sites -- leaving its tests behind and `test` back on:
log.success("All tests passed.")
reached unconditionally on a zero exit. ctest exits 0 when it finds
nothing to run, so a project with enable_testing() and no add_test()
reported a pass having executed nothing. Restored from the same commit.
Collection goes from "cannot import" to 474 passing. The 51 that fail are
pre-existing and were invisible while the module would not parse; they
are not all one cause and are listed in the PR description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ebuild testreports success for a project with no tests.ctest exits
0when it finds nothing to run. ACMakeLists.txtwithenable_testing()and noadd_test()still produces aCTestTestfile.cmake, so_resolve_test_runner()finds ctest, ctest printsNo tests were found!!!, exits 0, and the wrapper calls it a pass.Measured on current
master, against a project with a configured build tree andzero registered tests:
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.
The change
An empty run is 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
distinguish "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.
makehas no standard summary format, so its countsstay unknown and the exit status carries the verdict.
Verified
All three outcomes, on real CMake projects:
Also here: the docs must describe commands that exist
A check that every
ebuildcommand shown in the docs is one the CLI registers.Reads only fenced code blocks — prose says things like "ebuild is a unified build
system", and a plain grep over the README yields
ebuild and,ebuild can,ebuild is.Verified it fails rather than merely existing: putting
ebuild verifyin theREADME turns it red.
It also asserts the eight §7.1 golden-path steps directly, so the path cannot be
quietly broken by editing the README instead of the CLI.
Supersedes most of #84
#84 added
testandmonitorwhen neither existed. Both have since landed onmasterfrom another PR, so that work is redundant — this keeps only the partsthat are not: the empty-run guard, the count reporting, and the docs check. I
will close #84 once this is reviewed.