From 3f6bd4a86a5f4d98e76a293b27f1d6e13e3f0d4e Mon Sep 17 00:00:00 2001 From: minibwa-distro-bot Date: Sat, 22 Aug 2026 18:55:30 +0000 Subject: [PATCH] sync: assemble 0.7-nh13.dev+06ab16c on upstream f0e1174 --- .github/workflows/distro-sync.yml | 32 +++++-- README.md | 7 +- dist-manifest.json | 2 +- minibwa.h | 2 +- minibwa_dist/cli.py | 29 +++++++ minibwa_dist/features.toml | 4 + minibwa_dist/gates.py | 106 +++++++++++++++++++++++ minibwa_dist/manifest.py | 7 ++ minibwa_dist/tests/test_cli.py | 59 +++++++++++++ minibwa_dist/tests/test_gates.py | 120 ++++++++++++++++++++++++++- minibwa_dist/tests/test_manifest.py | 40 +++++++++ minibwa_dist/tests/test_workflows.py | 64 +++++++++++++- 12 files changed, 460 insertions(+), 12 deletions(-) diff --git a/.github/workflows/distro-sync.yml b/.github/workflows/distro-sync.yml index e885dc8..dfb02e9 100644 --- a/.github/workflows/distro-sync.yml +++ b/.github/workflows/distro-sync.yml @@ -158,14 +158,23 @@ jobs: # `--assembly` scopes the reported coverage to what actually merged: an # optional feature that conflicted is not in this binary, and a gate that # names it as covered contradicts the drop list this same job publishes. + # `--repo .` is the assembled checkout: each merged feature's declared + # `tests` directory is run there, which is the only positive-path coverage + # the distribution has -- byte-identity is blind to any path a feature + # turns ON, because those change output on purpose. run: | make -j"$(nproc)" + # The feature suites shell out to these probes. Built here rather than + # inherited from the earlier build step: a gate that depends on another + # step's side effect reports "probe not built" -- a build error wearing a + # test failure's clothes -- the first time the steps are reordered. + make -j"$(nproc)" -C api-test git worktree add /tmp/stock master make -C /tmp/stock -j"$(nproc)" python -m minibwa_dist.cli gates \ --candidate ./minibwa --stock /tmp/stock/minibwa \ --fixtures ./test --workdir /tmp/gates \ - --assembly /tmp/assembly.json + --assembly /tmp/assembly.json --repo . - name: Re-parent dist-next onto dist # dist and dist-next are otherwise INDEPENDENT assemblies of the same @@ -249,7 +258,13 @@ jobs: fi - name: File an issue for dropped features - if: steps.assemble.outputs.dropped != '[]' && steps.assemble.outputs.dropped != '' + # `always()`, because a REGRESSION fails the assemble step and would + # otherwise skip the one issue naming which features dropped and what they + # conflict in. That step writes its outputs before exiting precisely so + # this can still run. A regression therefore files this issue AND the + # generic `sync: run failed` one: different titles, different information, + # neither dedups against the other. + if: always() && steps.assemble.outputs.dropped != '[]' && steps.assemble.outputs.dropped != '' env: GH_TOKEN: ${{ github.token }} run: | @@ -273,8 +288,13 @@ jobs: # the open issue instead of skipping, the way distro-reconcile.yml does, # and refresh the TITLE too: it names the set, so a stale one misreports # which features are actually missing from the build. - NUM=$(gh issue list --state open --search "sync: dropped in:title" --json number \ - --jq '.[0].number // empty') + # Token-matched search, then an exact client-side shape check: `gh issue + # edit` rewrites title AND body, so a loose match would clobber an + # unrelated issue that merely shares two common words. The previous form + # was immune only because a false hit just skipped the update. + NUM=$(gh issue list --state open --search "sync: dropped in:title" --json number,title \ + --jq '[.[] | select((.title | startswith("sync: dropped ")) and + (.title | endswith(" -- would not merge")))][0].number // empty') if [ -n "$NUM" ]; then gh issue edit "$NUM" --title "$TITLE" --body "$BODY" else @@ -347,6 +367,8 @@ jobs: fi git worktree add /tmp/stock "$BASE" make -C /tmp/cand -j"$(nproc)" + # The feature suites shell out to these; see the x86 gate step. + make -C /tmp/cand/api-test -j"$(nproc)" make -C /tmp/stock -j"$(nproc)" # dist-manifest.json carries this build's `merged` list, so coverage names # only the features actually in the binary -- same scoping the x86 gate @@ -354,4 +376,4 @@ jobs: python -m minibwa_dist.cli gates \ --candidate /tmp/cand/minibwa --stock /tmp/stock/minibwa \ --fixtures /tmp/cand/test --workdir /tmp/gates \ - --assembly /tmp/cand/dist-manifest.json + --assembly /tmp/cand/dist-manifest.json --repo /tmp/cand diff --git a/README.md b/README.md index 39b0b9c..595e243 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,10 @@ structural changes anyway. Minibwa is designed for mapping short reads and accurate long reads. It does not support spliced alignment and has not been tuned for aligning long contigs. -For now, minibwa does not properly work with alternate contigs in the reference -genome. Please use a version of the reference without such contigs. +Alternate contigs are supported: when a `.alt` file sits beside the index +it is loaded automatically and ALT hits are reconciled with their primary +counterparts, so an ALT twin no longer collapses the primary's mapping quality. +Pass `--no-alt` to ignore it and align exactly as if no ALT file existed. ### Installation @@ -161,7 +163,6 @@ include GPL'd source code. * Minibwa does not work with noisy long reads or spliced RNA-seq reads. * Minibwa does not support undirectional bisulfite sequencing data. -* Minibwa does not recognize alternate haplotypes. [apache2]: https://github.com/lh3/minibwa/tree/Apache2 [zlib]: https://zlib.net/ diff --git a/dist-manifest.json b/dist-manifest.json index f782f3a..1841d49 100644 --- a/dist-manifest.json +++ b/dist-manifest.json @@ -1 +1 @@ -{"base": "f0e117436c28addc359b67123d2353f0d4a1f9e8", "head": "6ca35d1650f845604e2f1e0b560ec83e9ff8f518", "version": "0.7-nh13.dev+6ca35d1", "merged": ["ops-distro", "ll-affine-reassoc", "ksw2-extension-kernels", "single-copy-parser", "index-threads-v2", "extd2-avx512", "inline-appenders", "parallel-encode", "meth-cleanups", "meth-sam-tags", "soft-clip-penalty", "submem-ablation", "alt-liftgroup"], "dropped": [], "regressed": []} +{"base": "f0e117436c28addc359b67123d2353f0d4a1f9e8", "head": "06ab16caf2925c7f0963218293f6877127af420a", "version": "0.7-nh13.dev+06ab16c", "merged": ["ops-distro", "ll-affine-reassoc", "ksw2-extension-kernels", "single-copy-parser", "index-threads-v2", "extd2-avx512", "inline-appenders", "parallel-encode", "meth-cleanups", "meth-sam-tags", "soft-clip-penalty", "submem-ablation", "alt-liftgroup"], "dropped": [], "regressed": []} diff --git a/minibwa.h b/minibwa.h index 3cefa01..c0ba3d4 100644 --- a/minibwa.h +++ b/minibwa.h @@ -3,7 +3,7 @@ #include -#define MB_VERSION "0.7-nh13.dev+6ca35d1" +#define MB_VERSION "0.7-nh13.dev+06ab16c" #define MB_F_PAF (0x1LL) // output in the PAF format #define MB_F_NO_UNMAP (0x2LL) // output unmapped query sequences diff --git a/minibwa_dist/cli.py b/minibwa_dist/cli.py index 446cd8d..f3f12c6 100644 --- a/minibwa_dist/cli.py +++ b/minibwa_dist/cli.py @@ -107,6 +107,18 @@ def _gates(args: argparse.Namespace) -> int: A subcommand rather than two hand-copied heredocs in two workflows: the aggregation (`all(...)`) is the whole point of the step, and neither copy was reachable by `ruff check minibwa_dist/` or `pytest minibwa_dist/tests`. + + `--repo` is where each merged feature's declared test suites are run. It has + no default -- it is not always the working directory, and guessing wrong + points the suites at a tree that does not carry them -- but it is optional + rather than required, and its absence is reported instead of raising. + + That last part is not politeness. The engine and the workflow file update on + different schedules: every sync checks out the freshly assembled `dist-next` + and runs THAT `minibwa_dist`, while the YAML executing comes from `dist` and + changes only when a build is shipped. A newly required argument would be + demanded by the engine a full sync before the workflow could supply it, and + the gates step would die on argparse before the ship that would fix it. """ manifest = load_manifest(Path(args.manifest)) merged: tuple[str, ...] | None = None @@ -119,9 +131,16 @@ def _gates(args: argparse.Namespace) -> int: manifest, Path(args.workdir), merged=merged, + repo=Path(args.repo) if args.repo else None, ) for result in results: print(f"{'PASS' if result.passed else 'FAIL'} {result.name}: {result.detail}") + unrun = [f.name for f in manifest.features if f.tests and (merged is None or f.name in merged)] + if args.repo is None and unrun: + # Not a gate result: it did not run, so it neither passed nor failed. + # Said out loud all the same -- unrun coverage that looks like absent + # coverage is how a gate quietly stops meaning anything. + print(f"NOTE feature suites not run (--repo not given): {', '.join(unrun)}") return 0 if all(result.passed for result in results) else 1 @@ -241,6 +260,16 @@ def main(argv: list[str] | None = None) -> int: default=None, help="assembly JSON, so coverage names only the features this binary contains", ) + # No default: the two gate jobs lay their workspaces out differently -- one + # runs from the assembled checkout, the other from the tooling checkout with + # the candidate in a worktree -- so any default is wrong for one of them. A + # "." default aimed the arm64 gate at the tooling checkout, which carries no + # suites. Optional rather than required, though: see _gates(). + p.add_argument( + "--repo", + default=None, + help="the assembled checkout; each merged feature's declared test suites are run in it", + ) p.set_defaults(func=_gates) p = sub.add_parser("stamp", help="set MB_VERSION to an exact version string") diff --git a/minibwa_dist/features.toml b/minibwa_dist/features.toml index e65e3e9..fe09051 100644 --- a/minibwa_dist/features.toml +++ b/minibwa_dist/features.toml @@ -157,6 +157,10 @@ required = false output = "conditional" condition = "the index has ALT contigs" negative = "align the chrM fixture (no ALT contigs); SAM must be byte-identical to stock" +# The negative case above is all byte-identity can prove: with no ALT contigs the +# feature is inert. Its ~1,350 active lines only run when a .alt is present, and +# these suites are the only thing that exercises them. +tests = "test/altlg" summary = "ALT-aware mapping via post-extension liftover groups" upstream = { status = "unsubmitted" } diff --git a/minibwa_dist/gates.py b/minibwa_dist/gates.py index 2085bf9..b31fa5e 100644 --- a/minibwa_dist/gates.py +++ b/minibwa_dist/gates.py @@ -54,6 +54,26 @@ ) +# Bounded by the job, not by generosity: both gate jobs in distro-sync.yml cap at +# `timeout-minutes: 60`, and this budget applies to each of a feature's suites in +# turn. A value the whole set can outlast is a guard that never fires -- the +# runner kills the job first and prints nothing at all. 120s x 20 suites is 40 +# minutes, leaving the builds and the byte-identity comparison the rest; the +# in-tree ALT suites finish in seconds, so this is still enormous slack per +# suite. `test_the_suite_timeout_fits_inside_the_job_timeout` holds the bound. +_SUITE_TIMEOUT_S = 120 + +# Enough failures to see the pattern, not so many that one gate line is a log +# dump: 15 failing suites at 800 characters each is a 12KB single line. +_MAX_REPORTED_FAILURES = 3 + + +def _tail(text: str, limit: int = 800) -> str: + """The last `limit` characters, which is where a shell suite says why it failed.""" + text = text.strip() + return text if len(text) <= limit else "..." + text[-limit:] + + def _gate_name(label: str) -> str: """Gate name for a mode label. @@ -155,6 +175,88 @@ def _modes_are_distinct(stock_digests: dict[str, str]) -> GateResult: ) +def run_feature_suites( + repo: Path, manifest: Manifest, merged: tuple[str, ...] | None +) -> list[GateResult]: + """Run the `test-*.sh` suites of every merged feature that declares `tests`. + + This is the positive-path coverage `run_gates` deliberately leaves out. Its + byte-identity comparison proves the merge did not disturb stock behaviour, + which is the property rerere can silently break -- but it is blind to every + path a feature turns ON, because those change output on purpose. A feature's + own suites are the only thing that looks at them, and until this existed + nothing ran them: `distro-test` covers the Python engine, and the sync + compiled the aligner without ever invoking `make test`. + + Scoped to `merged` for the same reason the identity coverage line is: a + dropped feature's scripts are not in the tree, so a bare `make test` step + would fail with "no rule to make target" rather than a test failure, and + reporting it as covered would overstate what this build proved. + + Suites run with `repo` as both the working directory and their sole + argument, matching the `[]` convention the in-tree suites + already use to locate the binary they exercise. + """ + results: list[GateResult] = [] + for feature in manifest.features: + if feature.tests is None: + continue + if merged is not None and feature.name not in merged: + continue + name = f"tests:{feature.name}" + directory = repo / feature.tests + if not directory.is_dir(): + results.append( + GateResult( + name=name, + passed=False, + detail=f"declared suite directory '{feature.tests}' is not in the build", + ) + ) + continue + scripts = sorted(directory.glob("test-*.sh")) + if not scripts: + # Zero scripts is a pass over nothing, which is worse than no gate: + # it reports coverage that does not exist. + results.append( + GateResult(name=name, passed=False, detail=f"no test-*.sh in '{feature.tests}'") + ) + continue + failures: list[str] = [] + for script in scripts: + try: + proc = subprocess.run( + ["sh", str(script), str(repo)], + cwd=repo, + capture_output=True, + text=True, + timeout=_SUITE_TIMEOUT_S, + ) + ok, output = proc.returncode == 0, proc.stdout + proc.stderr + except subprocess.TimeoutExpired: + ok, output = False, f"timed out after {_SUITE_TIMEOUT_S}s" + if not ok: + failures.append(f"{script.name}: {_tail(output)}") + results.append( + GateResult( + name=name, + passed=not failures, + detail=( + f"{len(scripts)} suite(s) passed" + if not failures + else f"{len(failures)} of {len(scripts)} failed -- " + + " | ".join(failures[:_MAX_REPORTED_FAILURES]) + + ( + f" (+{len(failures) - _MAX_REPORTED_FAILURES} more)" + if len(failures) > _MAX_REPORTED_FAILURES + else "" + ) + ), + ) + ) + return results + + def run_gates( candidate_bin: Path, stock_bin: Path, @@ -162,6 +264,7 @@ def run_gates( manifest: Manifest, workdir: Path, merged: tuple[str, ...] | None = None, + repo: Path | None = None, ) -> list[GateResult]: """Run every gate implied by the manifest. @@ -229,6 +332,9 @@ def in_build(name: str) -> bool: ) ) + if repo is not None: + results.extend(run_feature_suites(repo, manifest, merged)) + for feature in manifest.features: # A dropped changes-output feature is not in the binary, so it has # nothing to block: failing the release for code this build does not diff --git a/minibwa_dist/manifest.py b/minibwa_dist/manifest.py index 9b62a38..800487e 100644 --- a/minibwa_dist/manifest.py +++ b/minibwa_dist/manifest.py @@ -42,6 +42,11 @@ class Feature: upstream: Upstream condition: str | None = None negative: str | None = None + # Directory of `test-*.sh` suites this feature ships, run by the output gates + # when -- and only when -- the feature is in the build. This is the positive- + # path coverage the byte-identity gate deliberately does not provide: those + # paths change output on purpose, so identity says nothing about them. + tests: str | None = None # False for work that is ours to carry and never upstream's to take -- the # distribution's own tooling. Defaults true: a feature is a candidate for # upstreaming unless the manifest says otherwise. @@ -125,6 +130,7 @@ def _parse_feature(table: dict[str, object], index: int) -> Feature: condition = table.get("condition") negative = table.get("negative") + tests = table.get("tests") if output == "conditional" and not (condition and negative): # Documentation, and required as such: the gate runs one default-flags # comparison that stands in for every conditional feature's negative @@ -146,6 +152,7 @@ def _parse_feature(table: dict[str, object], index: int) -> Feature: upstream=Upstream(status=status, pr=int(pr_raw) if pr_raw is not None else None), condition=str(condition) if condition else None, negative=str(negative) if negative else None, + tests=str(tests) if tests else None, upstreamable=_optional_bool(table, "upstreamable", where, default=True), ) diff --git a/minibwa_dist/tests/test_cli.py b/minibwa_dist/tests/test_cli.py index d1e1f19..3ba4df7 100644 --- a/minibwa_dist/tests/test_cli.py +++ b/minibwa_dist/tests/test_cli.py @@ -511,6 +511,8 @@ def gates(candidate: Path, workdir: str) -> int: str(fixtures), "--workdir", str(tmp_path / workdir), + "--repo", + str(tmp_path), ] ) @@ -552,6 +554,8 @@ def test_gates_scope_coverage_to_the_features_this_build_contains( str(tmp_path / "work"), "--assembly", str(assembly), + "--repo", + str(tmp_path), ] ) @@ -657,3 +661,58 @@ def test_sync_without_the_flag_reports_no_regressions( assert rc == 0 assert json.loads(capsys.readouterr().out)["regressed"] == [] + + +def test_gates_without_repo_reports_that_the_suites_did_not_run( + repo: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """`--repo` must stay optional, and its absence must be visible. + + The engine and the workflow file update at different times: every sync + checks out the freshly assembled `dist-next` and runs THAT `minibwa_dist`, + while the YAML executing comes from `dist` and only changes when a build is + shipped. So a new required argument is demanded by the engine one sync + before the workflow can supply it, and the gates step dies on argparse + before the ship that would fix it. Optional-and-loud is the only shape that + survives that gap. + """ + manifest = _write_manifest( + tmp_path, + """ +[[feature]] +name = "x" +branch = "feat/x" +required = false +output = "identical" +tests = "test/x" +summary = "x" +upstream = { status = "unsubmitted" } +""", + ) + sam = "@SQ\tSN:chrM\nr1\t0\tchrM\t1\t60\t10M\n" + binary = stub_aligner(tmp_path / "mb", sam) + fixtures = tmp_path / "fx" + fixtures.mkdir() + for f in ("chrM-human.fa.gz", "chrM-read_1.fa.gz", "chrM-read_2.fa.gz"): + (fixtures / f).write_bytes(b"") + + rc = main( + [ + "--manifest", + str(manifest), + "gates", + "--candidate", + str(binary), + "--stock", + str(binary), + "--fixtures", + str(fixtures), + "--workdir", + str(tmp_path / "wd"), + ] + ) + + assert rc == 0, "a missing --repo must not fail the build" + out = capsys.readouterr().out + assert "tests:x" not in out + assert "--repo not given" in out, "skipping the suites silently is the failure being avoided" diff --git a/minibwa_dist/tests/test_gates.py b/minibwa_dist/tests/test_gates.py index 1431741..2a62e7f 100644 --- a/minibwa_dist/tests/test_gates.py +++ b/minibwa_dist/tests/test_gates.py @@ -4,7 +4,7 @@ import pytest -from minibwa_dist.gates import GateResult, run_gates, sam_digest +from minibwa_dist.gates import GateResult, run_feature_suites, run_gates, sam_digest from minibwa_dist.manifest import Feature, Manifest, Upstream from minibwa_dist.tests.conftest import stub_aligner @@ -264,3 +264,121 @@ def test_binary_failure_is_a_gate_failure_not_a_traceback(tmp_path: Path) -> Non tmp_path, ) assert any(not r.passed and "could not run" in r.detail for r in results) + + +# --- feature test suites: the positive-path coverage byte-identity cannot give --- + + +def _suite(directory: Path, name: str, *, exit_code: int, message: str = "") -> Path: + directory.mkdir(parents=True, exist_ok=True) + path = directory / name + path.write_text(f"#!/bin/sh\necho '{message}'\nexit {exit_code}\n") + path.chmod(0o755) + return path + + +def _with_tests(name: str, tests: str | None) -> Feature: + return Feature( + name=name, + branch=f"feat/{name}", + required=False, + output="identical", + summary=name, + upstream=Upstream(status="unsubmitted"), + tests=tests, + ) + + +def test_a_merged_features_suites_run_and_pass(tmp_path: Path) -> None: + _suite(tmp_path / "test/x", "test-a.sh", exit_code=0) + _suite(tmp_path / "test/x", "test-b.sh", exit_code=0) + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + + results = run_feature_suites(tmp_path, manifest, ("x",)) + + assert [r.name for r in results] == ["tests:x"] + assert results[0].passed + assert "2" in results[0].detail + + +def test_a_failing_suite_fails_the_gate_and_is_named(tmp_path: Path) -> None: + _suite(tmp_path / "test/x", "test-ok.sh", exit_code=0) + _suite(tmp_path / "test/x", "test-bad.sh", exit_code=1, message="boom") + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + + results = run_feature_suites(tmp_path, manifest, ("x",)) + + assert not results[0].passed + assert "test-bad.sh" in results[0].detail + assert "test-ok.sh" not in results[0].detail, "only the failures are worth naming" + assert "boom" in results[0].detail, "the failure's output is what makes CI diagnosable" + + +def test_a_dropped_features_suites_are_not_run(tmp_path: Path) -> None: + """Its scripts are not even in the tree, and crediting them would overstate + coverage the same way the byte-identity gate refuses to.""" + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + + results = run_feature_suites(tmp_path, manifest, ()) + + assert results == [] + + +def test_a_feature_without_a_suite_directory_produces_no_gate(tmp_path: Path) -> None: + manifest = Manifest(features=(_with_tests("x", None),), withdrawn=()) + + assert run_feature_suites(tmp_path, manifest, ("x",)) == [] + + +def test_a_merged_feature_whose_suite_directory_is_missing_fails(tmp_path: Path) -> None: + """The feature merged, so its scripts should be in the tree. A missing + directory is a wrong manifest path, not an absence of coverage -- and + passing silently is exactly how a gate stops meaning anything.""" + manifest = Manifest(features=(_with_tests("x", "test/nope"),), withdrawn=()) + + results = run_feature_suites(tmp_path, manifest, ("x",)) + + assert not results[0].passed + assert "test/nope" in results[0].detail + + +def test_an_empty_suite_directory_fails(tmp_path: Path) -> None: + """Zero scripts would otherwise report as a pass over nothing.""" + (tmp_path / "test/x").mkdir(parents=True) + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + + results = run_feature_suites(tmp_path, manifest, ("x",)) + + assert not results[0].passed + assert "no test-*.sh" in results[0].detail + + +def test_suites_run_with_the_repo_as_the_working_directory(tmp_path: Path) -> None: + """The suites locate the binary relative to the tree they ship in.""" + d = tmp_path / "test/x" + d.mkdir(parents=True) + script = d / "test-cwd.sh" + script.write_text('#!/bin/sh\ntest "$(pwd)" = "$1" || exit 1\n') + script.chmod(0o755) + # The runner passes the repo root as $1, matching test/altlg/'s own convention. + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + + assert run_feature_suites(tmp_path, manifest, ("x",))[0].passed + + +def test_run_gates_includes_the_feature_suites(tmp_path: Path) -> None: + """Wired into run_gates, not left as a function nothing calls.""" + _suite(tmp_path / "test/x", "test-a.sh", exit_code=0) + manifest = Manifest(features=(_with_tests("x", "test/x"),), withdrawn=()) + stock = stub_aligner(tmp_path / "stock", "@SQ\tSN:chrM\nr1\t0\tchrM\t1\t60\t10M\n") + cand = stub_aligner(tmp_path / "cand", "@SQ\tSN:chrM\nr1\t0\tchrM\t1\t60\t10M\n") + fixtures = tmp_path / "fx" + fixtures.mkdir() + for f in ("chrM-human.fa.gz", "chrM-read_1.fa.gz", "chrM-read_2.fa.gz"): + (fixtures / f).write_bytes(b"") + + results = run_gates( + cand, stock, fixtures, manifest, tmp_path / "wd", merged=("x",), repo=tmp_path + ) + + assert any(r.name == "tests:x" for r in results) diff --git a/minibwa_dist/tests/test_manifest.py b/minibwa_dist/tests/test_manifest.py index 332f363..afa872e 100644 --- a/minibwa_dist/tests/test_manifest.py +++ b/minibwa_dist/tests/test_manifest.py @@ -134,3 +134,43 @@ def test_real_manifest_is_valid() -> None: assert all(f.required is False for f in manifest.features[1:]), ( "only the tooling entry may abort a build" ) + + +def test_a_feature_can_declare_a_test_suite_directory(tmp_path: Path) -> None: + manifest = load_manifest( + _write( + tmp_path, + """ +[[feature]] +name = "a" +branch = "feat/a" +required = false +output = "identical" +tests = "test/altlg" +summary = "a" +upstream = { status = "unsubmitted" } +""", + ) + ) + + assert manifest.features[0].tests == "test/altlg" + + +def test_the_test_suite_directory_is_optional(tmp_path: Path) -> None: + """Most features ship no suite of their own; that must stay legal.""" + manifest = load_manifest( + _write( + tmp_path, + """ +[[feature]] +name = "a" +branch = "feat/a" +required = false +output = "identical" +summary = "a" +upstream = { status = "unsubmitted" } +""", + ) + ) + + assert manifest.features[0].tests is None diff --git a/minibwa_dist/tests/test_workflows.py b/minibwa_dist/tests/test_workflows.py index b4d493c..86e6446 100644 --- a/minibwa_dist/tests/test_workflows.py +++ b/minibwa_dist/tests/test_workflows.py @@ -600,6 +600,68 @@ def test_a_hard_assembly_failure_exits_before_the_jq_parsing() -> None: is different: it DOES emit valid JSON, so it must still reach the outputs. """ text = _executable(_run(_sync(), "assemble", "Assemble dist-next")) - guard = 'jq -e . /tmp/assembly.json' + guard = "jq -e . /tmp/assembly.json" assert guard in text, "a nonzero run with no parsable JSON must exit before jq" assert text.index(guard) < text.index("dropped=$(jq") + + +def test_the_gates_step_builds_the_api_probes_it_depends_on() -> None: + """Feature suites shell out to the api-test probes, so the gates step must + build them rather than inherit them from an earlier step's side effect. + Without this the gate fails with "probe not built" -- a build error dressed + up as a test failure -- if the steps are ever reordered or split. + """ + text = _executable(_run(_sync(), "assemble", "Output gates")) + assert "make -C api-test" in text or "-C api-test" in text + + +def test_both_gate_jobs_point_repo_at_the_assembled_tree() -> None: + """`--repo` selects where the feature suites are run, and the two jobs lay + their workspaces out differently: the x86 job runs from the assembled + checkout, while the arm64 job checks out `ops/distro` and builds the + candidate in a worktree. A default would silently aim the arm64 gate at the + tooling checkout, which carries no suites at all. + """ + x86 = _executable(_run(_sync(), "assemble", "Output gates")) + assert "--repo ." in x86 + + arm = _executable(_run(_sync(), "gate-arm64", "Re-gate the published assembly on arm64")) + assert "--repo /tmp/cand" in arm, "the arm64 gate must run the suites in the candidate tree" + assert "/tmp/cand/api-test" in arm, "and must build the probes those suites shell out to" + + +def test_the_drop_issue_edit_targets_an_exactly_shaped_title() -> None: + """`gh issue edit` rewrites title AND body, so the target cannot be chosen by + a token search alone -- `sync: dropped in:title` matches on two ordinary + words, and `.[0]` would hand the bot whatever came back first. The previous + code was safe only because a false hit merely skipped the update. + """ + text = _executable(_run(_sync(), "assemble", "File an issue for dropped features")) + assert 'startswith("sync: dropped ")' in text + assert 'endswith(" -- would not merge")' in text + + +def test_the_drop_issue_step_runs_even_when_the_assembly_failed() -> None: + """A regression exits the assemble step nonzero, and the implicit `success()` + would skip the one issue that names which features dropped. That step writes + its outputs before exiting precisely so this one can still consume them. + """ + step = yaml_lite.step(_sync(), "assemble", name="File an issue for dropped features") + condition = str(step.get("if")) + assert condition.startswith("always()"), f"expected always() guard, got {condition!r}" + assert "dropped != '[]'" in condition, "still only when something actually dropped" + + +def test_the_suite_timeout_fits_inside_the_job_timeout() -> None: + """A per-suite budget the whole set can outlast is a guard that never fires: + the runner kills the job first and prints nothing. + """ + from minibwa_dist.gates import _SUITE_TIMEOUT_S + + jobs = _sync()["jobs"] + caps = [int(job["timeout-minutes"]) for job in jobs.values() if "timeout-minutes" in job] + assert caps, "the gate jobs must declare timeout-minutes for this bound to mean anything" + # 15 in-tree ALT suites today; leave room for a set that grows. + assert _SUITE_TIMEOUT_S * 20 <= min(caps) * 60, ( + f"{_SUITE_TIMEOUT_S}s x 20 suites exceeds the {min(caps)}min job cap" + )