Skip to content

fix(cli): a test run that executed nothing is not a pass - #89

Merged
srpatcha merged 2 commits into
masterfrom
fix/test-command-empty-run-is-not-a-pass
Sep 1, 2026
Merged

srpatcha merged 2 commits into
masterfrom
fix/test-command-empty-run-is-not-a-pass

Conversation

@srpatcha

Copy link
Copy Markdown
Member

ebuild test reports success for a project with no tests.

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 it a pass.

Measured on current master, 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. 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. make has no standard summary format, so its counts
stay unknown and the exit status carries the verdict.

Verified

All three outcomes, on real CMake projects:

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
tests   287 -> 307 passed, 1 skipped, 0 failed

Also here: the docs must describe commands that exist

A check that every ebuild command 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 verify in the
README turns it red.

FAILED test_a_documented_command_exists[verify-README.md]

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 test and monitor when neither existed. Both have since landed on
master from another PR, so that work is redundant — this keeps only the parts
that are not: the empty-run guard, the count reporting, and the docs check. I
will close #84 once this is reviewed.

`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
srpatcha merged commit f5209ac into master Sep 1, 2026
17 of 26 checks passed
@srpatcha
srpatcha deleted the fix/test-command-empty-run-is-not-a-pass branch September 1, 2026 10:04
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant