Repository navigation
fix: restore the last features dropped by merges, and take the suite green - #102
Merged
srpatcha merged 6 commits intoSep 1, 2026
Conversation
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>
…ined rule
Generating a build file fails outright on master:
NameError: name '_ninja_path' is not defined
_ninja_path was renamed escape_ninja_path, and three of its call sites
were not updated -- the executable edge, the static_library edge and the
shared_library edge. Between them that is every target type the backend
emits, so `configure`/`build` on the ninja backend produce no build.ninja
at all.
Those three calls were also double-escaping: `out`, `obj_files` and
`dep_archives` are each already escaped where they are built, so the
wrapper is not just undefined but redundant. Dropped rather than renamed.
With that fixed the generated file is still not usable for a
shared_library, because the edge names a rule the file never defines:
build build/libmylib.dylib: link_shared build/obj/mylib/lib.o
$ ninja
ninja: error: unknown rule 'link_shared'
Adds the rule, carrying the shared-object flag so no edge has to repeat
it, and _shared_flag() to pick it: -dynamiclib on macOS, -shared on ELF.
Emitting the ELF spelling on macOS is what the two tests in this area
were failing on.
End to end, a shared_library project now builds:
[1/2] CC build/obj/mylib/lib.o
[2/2] LINK_SHARED build/libmylib.dylib
$ file build/libmylib.dylib
Mach-O 64-bit dynamically linked shared library arm64
tests/unit/test_ninja_backend.py referenced the pre-rename name in five
assertions; updated to the exported one. 16/16 in the two ninja modules,
and the suite goes from 474 to 498 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…green The six remaining failures were all the same shape as the ones already fixed: work that landed as tests while its implementation was discarded during conflict resolution, or two implementations of one thing left side by side with the wrong one wired in. * backend precedence (embeddedos-org#96). The 22 lines that stop detect_backend() outranking a build.yaml that declares targets were gone; the test file survived. A Makefile kept for `make flash` still won, the dispatcher ran the external tool, nothing declared was built, and the build reported success. Restored from bba6f75. * dispatch errors. Three mechanisms sat on top of each other: BackendError + _validate_backend, and UnknownBackendError + a _unknown_backend() -- twice, the class and the function each defined two ways, the later shadowing the earlier. _validate_backend ran first and preempted the per-step error with a wrong type and a message that contradicted itself by listing ninja as supported while rejecting it. Consolidated on the surviving pair; BackendError stays as an alias so callers that catch it keep working. The guard moved ahead of the mkdir it used to precede, so rejecting a backend no longer leaves a build directory behind. * registry ordering. registry.py carried both version_sort_key and a private _version_sort_key, and every caller used the private one, which ranks any non-numeric version below every numeric one -- so v2.9.3, littlefs's own tag format, sorted below 1.10.0 and get() picked the wrong latest. Dropped the duplicate; the callers now use the tested public key. One assertion in tests/ebuild encoded the private key's ordering and is updated, which is the one judgement call here. * system_config. Parsed and validated in load_config(), then never passed to ProjectConfig, so the whole [system] section was inert. * footprint on macOS. Apple size(1) prints "__TEXT __DATA __OBJC others", and _SIZE_LINE matched that row too -- reading __OBJC, always 0, as bss. Every footprint measured on a Mac silently reported RAM containing no zero-initialised data. Falls back to `size -m` and sums sections, so bss is the real 4096 rather than 0. * doctor (embeddedos-org#72) and package (embeddedos-org#77). Both commands were absent from the group; ebuild/system/doctor.py and ebuild/build/firmware_image.py were already on master, only the CLI registration was lost. Restoring `package` surfaced a genuine collision: integration.py defines another `package`, and register_commands() runs last, so it had been silently replacing the .efw one. Renamed that to `package-deliverable` -- see the PR description, this one wants a maintainer's eye. Also adds a missing `import glob`: _serial_ports() raised NameError on every call, the same defect as the missing `import re`. 525 passed, 0 failed. mypy goes from 23 errors to 15; ruff is unchanged. Verified end to end: `ebuild doctor` reports the environment, and `ebuild build` links a running Mach-O executable and prints a footprint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
With the suite collecting again, seven tests fail on Windows only. They had never run there -- the module would not import -- so none of this was visible. Six are the tests' own portability: * tests/unit/test_package_efw.py wrote its efwtool stand-in as a /bin/sh script. Windows cannot execute one, so every test in TestPack died with "WinError 193: %1 is not a valid Win32 application". Rewritten to drive a Python script through sys.executable, behind a .bat on Windows and a shell shim elsewhere, with the behaviours it needs -- exit code, stderr, recorded argv, written output -- as keyword arguments rather than shell snippets. * test_footprint measured `gcc -o m`. gcc on Windows appends .exe when -o names no extension, so there was nothing at the path it measured. * test_build_dir_resolution asserted "Generated _build/build.ninja" while the message carries the platform separator. The seventh is not the test's fault. The ninja backend names an executable edge after the target with no extension, so on Windows gcc writes app.exe while the edge declares app: ninja never sees its own output, the target is dirty on every run, and it relinks each time. Give the edge the suffix the compiler driver actually produces -- the same thing the shared_library branch already does for .dll/.dylib/.so. No change on POSIX, where the suffix is empty. 525 passed locally, mypy unchanged, and `ebuild build` still links a running executable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mypy reports both as no-redef; neither is caught by the suite, because a shadowed definition fails quietly. tests/unit/test_ninja_backend.py declared TestNinjaPathEscaping twice, on lines 84 and 248, from two PRs that each added path-escaping coverage. The later class replaced the earlier one, so its four tests -- including the only assertion that every generated build statement carries exactly one unescaped colon -- had never run. Renamed the first; both now run, and all four pass. 525 -> 529 collected. ninja_backend.py defined _object_path twice, byte for byte identical. Behaviour was never affected; the copy is just dead. Removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 1, 2026
Codecov reported 68.66% patch coverage on embeddedos-org#102: 47 lines of new code with no test behind them. The gaps were not evenly spread -- they were the success paths and the error paths, with the refusals already covered. ebuild/cli/commands.py 61.53% -> 100% ebuild/build/footprint.py 80.55% -> 100% What was uncovered, and what now covers it: `_measure_macho` -- the Mach-O section parser ran only on the macOS leg, and even there missed the non-__TEXT branch, because the fixture binary (`static char buf[4096]` and nothing else) has no initialised data, so Segment __DATA held only a zerofill section. The parser is now driven from captured `size -m` output instead of a compiled binary, so all three platforms exercise it: section-to-region attribution, the __PAGEZERO and __LINKEDIT exclusions, and the raise that stops a parser which stopped matching from reporting a 0-byte image. `_run_size` -- neither the OSError nor the TimeoutExpired arm was run. The 60 s timeout only converts a hung `size` into a clean build-time message if the expiry arrives as FootprintError, so that arm is now tested rather than assumed. `_board_config` -- only the "no board.yaml" branch ran. Added: the round trip, and the three ways a bad file degrades to the reference table instead of failing a build that linked. `ebuild test` -- every line after subprocess.run was unexercised, which is exactly the code `_ran_no_tests` exists for: the helper was well tested, but nothing checked that the command consults it. Now driven end-to-end through a stubbed runner. `ebuild package` -- the three refusals were covered; the path a developer actually takes was not. Added the pack/verify round trip, --output, the verdict echo, and the two config-error exits. Per TESTING.md, each new test was checked by breaking the code it covers: 15 mutations applied, 15 detected. The first pass found a defect in one of these tests -- `ebuild test` echoes its argv, so a marker inlined into the stub's `-c` appeared in the output whether or not the command echoed what the runner printed. The canned output now goes through a file, and the mutation is caught. No implementation changes. 561 passed. Patch coverage 153/153 lines, no partial branches. The three ruff findings left in these files are pre-existing; one (`Path` unused in test_footprint.py) is resolved as a side effect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
srpatcha
approved these changes
Sep 1, 2026
This was referenced Sep 3, 2026
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.
Every remaining failure was the same shape as the ones already fixed: work that landed as tests while its implementation was discarded in conflict resolution, or two implementations of one thing left side by side with the wrong one wired in.
detect_backend()outranking abuild.yamlthat declares targets were gone; the 240-line test file survived. AMakefilekept formake flashstill won, the dispatcher ran the external tool, nothing declared was built, and the build reported success. Restored verbatim frombba6f75.BackendError+_validate_backend, andUnknownBackendError+_unknown_backend— the class and the function each defined twice, the later shadowing the earlier._validate_backendran first and preempted the per-step error with the wrong type and a message that listedninjaas supported while rejecting it. Consolidated;BackendErrorkept as an alias.registry.pycarried bothversion_sort_keyand a private_version_sort_key, and every caller used the private one, which ranks any non-numeric version below every numeric one — sov2.9.3, littlefs's own tag format, sorted below1.10.0andget()picked the wrong latest.system_configload_config(), then never passed toProjectConfig— the whole[system]section was inert.size(1)prints__TEXT __DATA __OBJC others;_SIZE_LINEmatched that row too and read__OBJC— always 0 — as bss. Every footprint measured on a Mac silently reported RAM with no zero-initialised data in it.doctor(#72) /package(#77)ebuild/system/doctor.pyandebuild/build/firmware_image.pywere already on master — only the ~30 and ~67 line CLI registrations were lost.import glob_serial_ports()raisedNameErroron every call. Same defect as the missingimport re.The footprint bug, concretely
Two judgement calls, flagged
1.
packagecollides.integration.pydefines a secondpackage("deliverable ZIP for a hardware target") andregister_commands()runs last — so it has been silently replacing the.efwcommand from #77, which is whytests/unit/test_package_efw.pygot a signature wanting--target. Two deliverables cannot share one name. I keptpackagefor the eFirmware image (it has 14 dedicated behavioural tests and is the step in §29's device flow) and renamed the other topackage-deliverableso nothing is lost. Both are reachable;tests/unit/test_integration_commands_registered.pyonly checks the name is present, so it passes either way — which is exactly why the collision went unnoticed. If you'd rather the ZIP keep the name, say so and I'll swap them.2. One assertion updated.
tests/ebuild/test_package_registry.pyencoded the private key's ordering (["1.2.13-1", "v2.9.3", "1.9.0", "1.10.0"]). Under the public key that the other test file exercises,v2.9.3orders as2.9.3. Updated with a comment explaining why; the two files had contradictory specs and only one key can survive.Validation
Line endings verified unchanged per file (
commands.pyandintegration.pystay CRLF; the rest LF; no stray\r).End to end: