test(dispatch): cover configure no-op backends

[Tanisha1723]

Summary

Add regression coverage for eBuild configure backends that intentionally have no separate configure step.

Changes

• Added coverage for cargo, make, and kbuild configure backends.
• Verify these backends do not invoke subprocess.run() during configuration.
• Added a dedicated configure section in the test file for clearer organization.
• ninja is intentionally not included because BackendDispatcher.configure() does not support configuring eBuild's own ninja backend.

Testing

• python -m pytest tests/ebuild/test_dispatch.py -v — 30 passed
• git diff --check — passed

This PR is test-only; no production behavior is changed.

[srpatcha]

Review — ebuild#108 "Fix configure backend dispatch"

head: 39d1d2a author: Tanisha1723 ci: none reported (checks.txt is 0 bytes)

Verdict: The three tests this adds are correct, correctly targeted and cover something that was uncovered. But the change the title and body describe is not in the PR: changedFiles is 1 and it is a test file, ebuild/build/dispatch.py is untouched, and the else branch the body says was removed is still on master. One of the four backends the body claims to have verified would have failed the very test being added, and the reported test count matches neither master nor this branch.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	— (PR body vs. diff)	The described code change is absent. The title is "Fix configure backend dispatch" and the body's first line is "Fix BackendDispatcher.configure() so that supported build backends without dedicated configure steps are handled as intentional no-ops instead of falling through to invalid control flow", with "Removed the unreachable/invalid else branch in BackendDispatcher.configure()" listed under Changes. pr.json reports changedFiles: 1, additions: 12, deletions: 0, and files.txt is a single line: tests/ebuild/test_dispatch.py. I applied the patch to origin/master and grep -n "raise _unknown_backend" ebuild/build/dispatch.py still returns line 196 — the else is exactly where it was. So this PR fixes nothing; it adds tests to unchanged code. Worth adding, in fairness to the author, that the intended change would not have been a fix either: line 173 already rejects anything outside CONFIGURE_BACKENDS before the mkdir, and CONFIGURE_BACKENDS (line 50) is precisely the five backends the if/elif chain enumerates, so line 196 is unreachable defensive code, not "invalid control flow". Removing it is a tidy-up worth doing; it does not change behaviour, and the comment above line 173 already records why the upfront guard was put there. The createdAt / updatedAt gap (17:40:59Z → 18:55:02Z) suggests a force-push that dropped a commit.	Either push the dispatch.py commit so the title is true, or retitle to test(dispatch): cover configure no-op backends and rewrite the body to describe what is here. The second is the smaller and, given that no behaviour changes either way, probably the right one.
2	Medium	tests/ebuild/test_dispatch.py:70	ninja is listed in the body as covered and verified, and it is neither in the test nor a valid input. The Changes section enumerates "supported backends that do not require a configure step: cargo, make, kbuild, ninja" and adds "Verified that these backends do not invoke subprocess.run() during configuration." The @pytest.mark.parametrize list is ["cargo", "make", "kbuild"] — three, not four. Had ninja been included it would have failed, not passed: CONFIGURE_BACKENDS at dispatch.py:50 is {"cmake", "meson", "cargo", "make", "kbuild"}, so configure("ninja") raises at line 174 before reaching any no-op. I ran it with ninja added: 1 failed, 30 passed, with UnknownBackendError: Unknown build backend 'ninja'. BackendDispatcher can configure: cargo, cmake, kbuild, make, meson. ebuild's own ninja backend is invoked directly rather than through BackendDispatcher, and requires 'targets' in build.yaml. The existing error message already states the correct model, so the claim contradicts something the code says out loud.	Drop ninja from the body. If the intent was to cover it, the right test is the opposite assertion — that configure("ninja") raises UnknownBackendError — and it belongs in TestUnknownBackend below, where ninja is the most interesting case precisely because it is a supported build backend that this dispatcher deliberately does not configure.
3	Medium	— (checks.txt is empty; PR body "Testing")	No CI ran, the quoted test count does not match this repository, and a whitespace check is offered as verification of a behavioural change. checks.txt is 0 bytes while ebuild#103 and #104 in this batch carry 24 and 30 checks. The body reports python -m pytest tests/ebuild/test_dispatch.py -v → "25 passed in 0.22s". On origin/master that file is 27 passed; with this patch applied it is 30 passed (27 + 3 parametrised cases). 25 matches neither, so the run was against some other tree — plausibly one that predates recent additions to the file, which fits a branch that was not rebased. The second verification listed is git diff --check → "No whitespace errors reported", which checks trailing whitespace and conflict markers and says nothing about the change. Under the brief's rule that an unsupported "verified" is itself the finding, both lines are it.	A maintainer approves the workflow runs. Rebase on master, re-run, and quote the real tail. Drop git diff --check from the Testing section — it is not evidence of anything the PR claims.
4	Low	tests/ebuild/test_dispatch.py:66-68	The new class is inserted under a section banner that belongs to the class below it. # ── BackendDispatcher — unknown backend ───── now sits immediately above TestConfigureBackends, with TestUnknownBackend — the class the banner names — following it. A reader scanning the file by banner will attribute the wrong tests to the wrong section, and the file uses these banners consistently enough that they are load-bearing for navigation.	Move TestConfigureBackends above the banner and give it its own — # ── BackendDispatcher — configure ───── — matching the surrounding style.

Verified clean, and worth recording because the tests themselves are the good part of this PR:

• The three new cases pass and are real coverage. pytest tests/ebuild/test_dispatch.py goes from 27 passed on origin/master to 30 passed with this patch, out of tree in /tmp/eb108. Nothing in the file previously asserted that the no-op backends are no-ops.
• The mock target is correct, which is the part that could easily have been wrong. @patch("ebuild.build.dispatch.subprocess") only proves anything if the subprocess call is reached through that module's own name. dispatch.py:13 is import subprocess, and _run_or_log() — the single helper every executing branch of configure() routes through — is defined in dispatch.py:112 and calls subprocess.run from that namespace. So mock_sub.run.assert_not_called() is a meaningful assertion rather than a vacuous one. Had _run_or_log lived in a sibling module, the test would have passed for the wrong reason.
• The assertion tests the right property. cargo, make and kbuild reach pass # These backends have no separate configure step, so "did not shell out" is exactly the behaviour worth pinning: if someone later gives one of them a configure command, this test is what will notice.
• tmp_path keeps it hermetic. BackendDispatcher(tmp_path, tmp_path / "build") means the self.build_dir.mkdir(parents=True, exist_ok=True) at dispatch.py:172 writes into pytest's temp directory rather than the repository.

Architecture conformance

Conforms. §21 Tier 1 — Foundation (ebuild). tests/ebuild/ is the right home per .ai/architect.md; nothing crosses a tier, nothing is a runtime dependency, and §5.1 is untouched. ebuild/build/dispatch.py is host-side build orchestration, which is §9.1's "eBuild engine — Toolchains / Packages / Targets → Dependency Graph → Configure / Build", so configure() sits directly on the design's central developer path.

One observation rather than a finding, because it is not this PR's to answer: ALL_BACKENDS (line 26) and CLEAN_BACKENDS (line 54) include ninja while CONFIGURE_BACKENDS (line 50) and BUILD_BACKENDS (line 52) do not, because ebuild's own ninja backend is driven through ebuild/build/ninja_backend.py rather than through the dispatcher. That asymmetry is deliberate and the error message explains it, which is better than most such splits manage — it is also exactly what finding 2's author appears to have missed, and a test asserting the asymmetry would document it where the next person will look.

No proposal appended. Nothing in the master design is wrong or silent here — §9.1 and §9.2 already describe the eBuild engine's structure, and this is ordinary repository work against it.

Proposed changes

1. Decide what this PR is (finding 1): push the missing dispatch.py commit, or retitle and rewrite the body as a test-only change. Nothing else should be settled before this, because the answer changes what the rest of the review is about.
2. Remove the ninja claim, and consider adding the opposite assertion in TestUnknownBackend (finding 2).
3. Rebase, get the workflow runs approved, and quote a real test tail; drop git diff --check (finding 3).
4. Move the class above the section banner (finding 4).

Items 2–4 are independent and small. The three tests themselves I would take as they are.

Not checked

• Nothing ran in CI, so nothing here is independently corroborated. All counts are from this host, using a uv-managed pytest that needed PYTHONPATH=/usr/lib/python3/dist-packages for yaml. The full suite was not run for this PR; it has a pre-existing failure on this host from a missing ninja module, unrelated.
• I did not establish why the body describes a change that is not present. The createdAt/updatedAt gap is consistent with a dropped commit, but I did not fetch the branch's commit list or reflog and the explanation is a hypothesis, not a finding. If a dispatch.py commit exists somewhere, finding 1 becomes a mechanical problem rather than a substantive one.
• ebuild/build/ninja_backend.py was not read. The observation about the ninja asymmetry rests on dispatch.py's constant sets and its error message, plus the module's existence. Whether ninja's own path has an equivalent configure step, and whether it is tested, I do not know.
• No caller of configure() was examined. The three new tests exercise the dispatcher directly; whether ebuild configure in the CLI reaches these branches with these backend names, and whether the no-op is the right behaviour at that level, is outside what I checked.
• dispatch.py was recently rewritten by #102 (27 insertions, 63 deletions in that area) and #102 is unreviewed by this system — the ledger has no ebuild entries before this run. So the code this PR tests went in without review, and my reading of configure() is a first reading rather than a re-reading.
• mergeStateStatus: BLOCKED, mergeable: MERGEABLE, reviewDecision: REVIEW_REQUIRED. No merge attempted. tests/ebuild/test_dispatch.py is also touched by #103's and #104's stacked diffs, but those hunks are already on master via #102, so a conflict is unlikely.
• The local ebuild checkout is dirty and was skipped by the sync step, and sits on branch v90. I read origin/master through git show and git archive; the working tree was not touched and all patched trees are under /tmp.

---

Automated architecture review of 39d1d2adc398 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Review — ebuild#108 "test(dispatch): cover configure no-op backends"

head: b9bd31b author: Tanisha1723 ci: pending

Verdict: Test-only change (16 added lines, one file) that adds a parametrised no-op assertion for configure(). The tests pass and the change is architecturally harmless, but the PR body describes work that is not in the diff and reports a result that does not match the file, and the coverage it adds is almost entirely already present on master.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	(PR body)	Body claims "Removed the unreachable/invalid else branch in BackendDispatcher.configure()". The diff touches only tests/ebuild/test_dispatch.py; ebuild/build/dispatch.py is unchanged and the else: raise branch is still present at dispatch.py:195-196 on this head. Verified: git diff --stat origin/master...<head> → 1 file changed, 16 insertions(+).	Either include the dispatch.py change or remove the claim from the body. Do not describe unmade edits.
2	Medium	(PR body)	Body lists ninja among "supported backends that do not require a configure step" and says it "Verified that these backends do not invoke subprocess.run()". ninja is deliberately excluded from CONFIGURE_BACKENDS (ebuild/build/dispatch.py:50, with the reason in the comment at :46-49), so configure("ninja") raises UnknownBackendError — an assertion that it is a silent no-op would fail, and master already has test_configure_ninja_raises_instead_of_silently_passing (tests/ebuild/test_dispatch.py:214) asserting the opposite. The added parametrize list correctly contains only cargo, make, kbuild; the body is wrong, not the test.	Drop ninja from the body's list.
3	Low	(PR body)	Body reports pytest tests/ebuild/test_dispatch.py -v → "25 passed". Running that file at this head collects 30 tests, not 25 (30 passed in 0.06s). The stated evidence does not correspond to the committed tree.	Re-run and paste current output.
4	Low	tests/ebuild/test_dispatch.py:69-79	Duplicate coverage. test_no_configure_step_backends_stay_noops already exists on origin/master at tests/ebuild/test_dispatch.py:230 and already loops ("cargo", "make", "kbuild") through configure() asserting no raise. The only new information in TestConfigureBackends is the mock_sub.run.assert_not_called() assertion.	Add the @patch("ebuild.build.dispatch.subprocess") + assert_not_called() assertion to the existing test at :230 and delete the new class, rather than carrying two tests with the same name-in-spirit and the same parameter list.
5	Low	tests/ebuild/test_dispatch.py:214-222	Coverage gap the PR was well placed to close and did not. dispatch.py:169-177 documents a specific past regression: the pre-guard code raised only from the else branch, after self.build_dir.mkdir(...), "left a stray build directory behind for a backend this step never handles". The guard at :173 now runs before the mkdir at :177, but no test asserts the side-effect-free property — test_configure_ninja_raises_instead_of_silently_passing only asserts that it raises. The mkdir could be moved back above the guard without any test failing.	Extend the ninja test: d = BackendDispatcher(tmp_path, tmp_path / "build"); with pytest.raises(UnknownBackendError): d.configure("ninja"); then assert not (tmp_path / "build").exists().
6	Low	(CI)	No checks reported on fix/dispatch-configure-syntax; statusCheckRollup is empty and mergeStateStatus is BLOCKED. ci.yml does declare pull_request: branches: [master, main], so the trigger is correct — this is a fork PR (isCrossRepository: true, head owner Tanisha1723) whose workflow runs are awaiting maintainer approval. Not the author's defect, but the PR carries no CI evidence at all right now.	Maintainer: approve the workflow run so the test matrix actually gates this.

Architecture conformance

Conforms. Master design §21 places ebuild in Tier 1 — Foundation, and §5.1 requires that eBuild "understands the complete graph but is not a runtime dependency". This diff adds a test inside tests/ebuild/, introduces no import, no target_link_libraries entry and no manifest dependency, and touches nothing outside the repo's own test tree — so no dependency direction is affected and no tier boundary is crossed. §9.2's "integrated test" rule is served, weakly, by the added case.

No weakened check: nothing is skipped, xfailed or deleted; the diff is purely additive (16+ 0-).

Proposed changes

Smallest sequence that keeps things working:

1. Fix the PR body — remove the dispatch.py/else-branch claim, remove ninja, replace the "25 passed" line with real current output. Nothing in the tree needs to change for this.
2. Fold the new assertion into the existing test and drop the duplicate class:

# tests/ebuild/test_dispatch.py — replace the existing test at :230
    @patch("ebuild.build.dispatch.subprocess")
    def test_no_configure_step_backends_stay_noops(self, mock_sub, tmp_path):
        """cargo/make/kbuild are accepted-and-skipped, not errors."""
        d = BackendDispatcher(tmp_path, tmp_path / "build")
        for backend in ("cargo", "make", "kbuild"):
            d.configure(backend)  # must not raise
        mock_sub.run.assert_not_called()

3. Close finding 5 by asserting the no-side-effect property on the rejection path (snippet in the table).
4. If the intent really was to remove the dead else at dispatch.py:195-196, do that in a separate commit in this PR and say so — it is unreachable because :173 already rejects everything outside CONFIGURE_BACKENDS and the if/elif chain covers all five members of that set. It is dead code, not a defect, so it is not urgent.

Not checked

• CI. Nothing ran. checks.txt is empty (0 bytes) and gh pr checks 108 reports "no checks reported on the 'fix/dispatch-configure-syntax' branch". I did not trigger a run.
• Test result is mine, not CI's, and on one interpreter only. I extracted the head tree with git archive into a temp directory and ran pytest tests/ebuild/test_dispatch.py -q → 30 passed in 0.06s on Python 3.14.4. The repo's matrix targets 3.10/3.11/3.12 on ubuntu-22.04, macos-latest and windows-2022; none of those combinations were exercised.
• The rest of the suite. I ran only tests/ebuild/test_dispatch.py. I did not run tests/unit/test_dispatch.py or tests/unit/test_backend_dispatch.py, which also exist in this tree and may overlap further with the added case.
• The local ebuild clone has a dirty working tree (4 files, reported by the sync step) and was left untouched; every statement above about master comes from origin/master and every statement about the PR from the fetched head object, not from the working tree.

---

Automated architecture review of b9bd31b76331 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.