From 65d56ba9cd34084f51e21843823bf790d5ca5f8b Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Wed, 30 Sep 2026 21:47:00 -0500 Subject: [PATCH 1/2] test: fail fast under --no-build when a needed binary is missing; add --no-chemistry Under --no-build, build() is a no-op, so a case whose build variant was never compiled failed only when it ran. Chemistry is the usual one: ./mfc.sh build does not produce the chemistry variant, so a full suite ran for its whole length before the chemistry cases failed at the end. Before running any case, check that every (target, slug) the selected cases need has an installed binary. If any are missing, list each with an example case and the expected path, and exit non-zero. When every affected case uses chemistry, suggest --no-chemistry. --no-chemistry skips cases with chemistry = 'T'. It keys on the parameter, not the trace, because Example cases built on reacting examples need a chemistry build but carry no "Chemistry" trace element. --- docs/documentation/testing.md | 2 + toolchain/mfc/cli/commands.py | 7 ++ toolchain/mfc/test/test.py | 69 ++++++++++++++-- toolchain/mfc/test/test_nobuild_preflight.py | 87 ++++++++++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 toolchain/mfc/test/test_nobuild_preflight.py diff --git a/docs/documentation/testing.md b/docs/documentation/testing.md index 4063999473..b2b35eceb6 100644 --- a/docs/documentation/testing.md +++ b/docs/documentation/testing.md @@ -18,6 +18,8 @@ A test is considered passing when our error tolerances are met in order to maint - `--percent` (`%`) to specify a percentage of the test suite to select at random and test - `--max-attempts` (`-m`) the maximum number of attempts to make on a test before considering it failed - `--no-examples` skips the testing of cases in the examples folder +- `--no-chemistry` skips every case that uses chemistry (``chemistry = 'T'``), including reacting example cases +- `--no-build` runs against existing binaries without rebuilding. Some cases (chemistry, analytic initial conditions) need their own build, which `./mfc.sh build` does not produce; build everything the suite needs with `./mfc.sh test --dry-run `. If any required binary is missing, `--no-build` stops before running any case and lists what is missing. - `--rdma-mpi` runs additional tests where RDMA MPI is enabled. To specify a computer, pass the `-c` flag to `./mfc.sh run` like so: diff --git a/toolchain/mfc/cli/commands.py b/toolchain/mfc/cli/commands.py index e177ec81f0..11077e2730 100644 --- a/toolchain/mfc/cli/commands.py +++ b/toolchain/mfc/cli/commands.py @@ -444,6 +444,13 @@ default=False, dest="no_examples", ), + Argument( + name="no-chemistry", + help="Do not test cases that use chemistry (chemistry = T).", + action=ArgAction.STORE_TRUE, + default=False, + dest="no_chemistry", + ), Argument( name="case-optimization", help="(GPU Optimization) Compile MFC targets with some case parameters hard-coded.", diff --git a/toolchain/mfc/test/test.py b/toolchain/mfc/test/test.py index f6320aa976..863f2b3674 100644 --- a/toolchain/mfc/test/test.py +++ b/toolchain/mfc/test/test.py @@ -323,6 +323,54 @@ def is_uuid(term): return selected_cases, skipped_cases +def _uses_chemistry(case: TestCase) -> bool: + return case.params.get("chemistry", "F") == "T" + + +def _drop_chemistry_cases(cases, skipped_cases): + """--no-chemistry: skip every case that sets chemistry = T. + + Keyed on the parameter, not the trace: Example cases built on reacting + examples need a chemistry build but carry no "Chemistry" trace element. + """ + chem = [case for case in cases if _uses_chemistry(case)] + return [case for case in cases if not _uses_chemistry(case)], skipped_cases + chem + + +def find_unbuilt(cases, codes) -> typing.List[dict]: + """Return one entry per (target, build slug) the cases need whose binary is not installed.""" + unbuilt = {} + checked = set() + for case, code in itertools.product(cases, codes): + input_file = case.to_input_file() + key = (code.name, code.get_slug(input_file)) + if key in unbuilt: + unbuilt[key]["cases"].append(case) + continue + if key in checked: + continue + checked.add(key) + binpath = code.get_install_binpath(input_file) + if not os.path.isfile(binpath): + unbuilt[key] = {"target": code.name, "slug": key[1], "binpath": binpath, "cases": [case]} + return list(unbuilt.values()) + + +def unbuilt_message(unbuilt: typing.List[dict]) -> str: + n_cases = len({case.get_uuid() for entry in unbuilt for case in entry["cases"]}) + lines = [f"--no-build was given, but {len(unbuilt)} build(s) needed by {n_cases} test case(s) are missing:"] + for entry in unbuilt: + cases = entry["cases"] + more = f" (+{len(cases) - 1} more)" if len(cases) > 1 else "" + lines.append(f" {entry['target']} [{entry['slug']}]: {len(cases)} case(s), e.g. {cases[0].trace} ({cases[0].get_uuid()}){more}") + lines.append(f" expected {entry['binpath']}") + lines.append("Build them by rerunning this command with --dry-run in place of --no-build, or drop --no-build to build and test in one go.") + lines.append("Note that ./mfc.sh build alone does not build case-specific variants such as chemistry.") + if all(_uses_chemistry(case) for entry in unbuilt for case in entry["cases"]): + lines.append("All of the affected cases use chemistry; pass --no-chemistry to skip them.") + return console_safe("\n".join(lines)) + + def test(): global nFAIL, nPASS, nSKIP, total_test_count # noqa: PLW0603 global errors, failed_tests, test_start_time # noqa: PLW0603 @@ -368,6 +416,8 @@ def test(): cases, skipped_cases = __filter(cases) cases = [_.to_case() for _ in cases] + if ARG("no_chemistry"): + cases, skipped_cases = _drop_chemistry_cases(cases, skipped_cases) total_test_count = len(cases) if ARG("list"): @@ -387,12 +437,19 @@ def test(): # Analytically defined patches, and --case-optimization. Here, we build all # the unique versions of MFC we need to run cases. codes = [PRE_PROCESS, SIMULATION] + ([POST_PROCESS] if ARG("test_all") else []) - unique_builds = set() - for case, code in itertools.product(cases, codes): - slug = code.get_slug(case.to_input_file()) - if slug not in unique_builds: - build(code, case.to_input_file()) - unique_builds.add(slug) + if ARG("no_build"): + # build() is a no-op under --no-build, so a missing binary would otherwise + # surface only when its cases run, one failure at a time, often at the end. + unbuilt = find_unbuilt(cases, codes) + if unbuilt: + raise MFCException(unbuilt_message(unbuilt)) + else: + unique_builds = set() + for case, code in itertools.product(cases, codes): + slug = code.get_slug(case.to_input_file()) + if slug not in unique_builds: + build(code, case.to_input_file()) + unique_builds.add(slug) cons.print() diff --git a/toolchain/mfc/test/test_nobuild_preflight.py b/toolchain/mfc/test/test_nobuild_preflight.py new file mode 100644 index 0000000000..2c4d69c27e --- /dev/null +++ b/toolchain/mfc/test/test_nobuild_preflight.py @@ -0,0 +1,87 @@ +"""Tests for the --no-build preflight and --no-chemistry filter. + +Under --no-build, build() is a no-op, so a case whose build variant was never +compiled (chemistry is the common one: ./mfc.sh build does not produce it) used +to fail only when it ran, typically at the end of a full suite. +""" + +from mfc.test.test import _drop_chemistry_cases, find_unbuilt, unbuilt_message + + +class FakeCase: + def __init__(self, trace, uuid, params, slug): + self.trace = trace + self.uuid = uuid + self.params = params + self.slug = slug + + def get_uuid(self): + return self.uuid + + def to_input_file(self): + return self + + +class FakeTarget: + def __init__(self, name, installed_slugs): + self.name = name + self.installed_slugs = installed_slugs + + def get_slug(self, case): + return f"{self.name}-{case.slug}" + + def get_install_binpath(self, case): + return f"/nonexistent/{self.get_slug(case)}/bin/{self.name}" if case.slug not in self.installed_slugs else __file__ + + +PLAIN = FakeCase("1D -> bc=-1", "AAAAAAAA", {}, "plain") +PLAIN2 = FakeCase("1D -> bc=-2", "BBBBBBBB", {"chemistry": "F"}, "plain") +CHEM = FakeCase("1D -> Chemistry -> Perfect Reactor", "CCCCCCCC", {"chemistry": "T"}, "chem") +REACTING_EXAMPLE = FakeCase("Example -> 2D -> shock_flame", "DDDDDDDD", {"chemistry": "T"}, "chem2") + + +def test_nothing_is_reported_when_every_build_exists(): + codes = [FakeTarget("pre_process", {"plain", "chem"}), FakeTarget("simulation", {"plain", "chem"})] + assert find_unbuilt([PLAIN, PLAIN2, CHEM], codes) == [] + + +def test_a_missing_chemistry_build_is_reported_once_per_target_with_all_its_cases(): + codes = [FakeTarget("pre_process", {"plain"}), FakeTarget("simulation", {"plain"})] + unbuilt = find_unbuilt([PLAIN, CHEM, PLAIN2, REACTING_EXAMPLE], codes) + + assert [(e["target"], e["slug"]) for e in unbuilt] == [ + ("pre_process", "pre_process-chem"), + ("simulation", "simulation-chem"), + ("pre_process", "pre_process-chem2"), + ("simulation", "simulation-chem2"), + ] + assert all(len(e["cases"]) == 1 for e in unbuilt) + + +def test_cases_sharing_a_missing_build_are_grouped(): + codes = [FakeTarget("simulation", set())] + unbuilt = find_unbuilt([PLAIN, PLAIN2], codes) + + assert len(unbuilt) == 1 + assert unbuilt[0]["cases"] == [PLAIN, PLAIN2] + + +def test_message_suggests_no_chemistry_only_when_every_missing_case_uses_chemistry(): + codes = [FakeTarget("simulation", {"plain"})] + assert "--no-chemistry" in unbuilt_message(find_unbuilt([PLAIN, CHEM], codes)) + + codes = [FakeTarget("simulation", set())] + assert "--no-chemistry" not in unbuilt_message(find_unbuilt([PLAIN, CHEM], codes)) + + +def test_message_counts_distinct_cases_across_targets(): + codes = [FakeTarget("pre_process", set()), FakeTarget("simulation", set())] + assert "needed by 2 test case(s)" in unbuilt_message(find_unbuilt([PLAIN, CHEM], codes)) + + +def test_no_chemistry_keys_on_the_parameter_not_the_trace(): + # The reacting example has no "Chemistry" trace element but still needs a chemistry build. + kept, skipped = _drop_chemistry_cases([PLAIN, CHEM, PLAIN2, REACTING_EXAMPLE], ["already-skipped"]) + + assert kept == [PLAIN, PLAIN2] + assert skipped == ["already-skipped", CHEM, REACTING_EXAMPLE] From c0de563578a54ce4a54e892b0a95707019c2b949 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Wed, 30 Sep 2026 22:12:03 -0500 Subject: [PATCH 2/2] test: require an executable binary; keep skipped_cases homogeneous Address review: the --no-build preflight now also requires the binary to be executable, and --no-chemistry appends the skipped cases' builders rather than their converted TestCases, so skipped_cases holds one type as __filter leaves it. --- toolchain/mfc/test/test.py | 16 ++++++----- toolchain/mfc/test/test_nobuild_preflight.py | 30 ++++++++++++++++++-- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/toolchain/mfc/test/test.py b/toolchain/mfc/test/test.py index 863f2b3674..2cf766fa6f 100644 --- a/toolchain/mfc/test/test.py +++ b/toolchain/mfc/test/test.py @@ -327,14 +327,16 @@ def _uses_chemistry(case: TestCase) -> bool: return case.params.get("chemistry", "F") == "T" -def _drop_chemistry_cases(cases, skipped_cases): +def _drop_chemistry_cases(builders, cases, skipped_cases): """--no-chemistry: skip every case that sets chemistry = T. Keyed on the parameter, not the trace: Example cases built on reacting examples need a chemistry build but carry no "Chemistry" trace element. + The parameter is only known after to_case(), so the builders are passed + alongside to keep skipped_cases a list of builders like the rest. """ - chem = [case for case in cases if _uses_chemistry(case)] - return [case for case in cases if not _uses_chemistry(case)], skipped_cases + chem + kept = [case for case in cases if not _uses_chemistry(case)] + return kept, skipped_cases + [builder for builder, case in zip(builders, cases) if _uses_chemistry(case)] def find_unbuilt(cases, codes) -> typing.List[dict]: @@ -351,7 +353,7 @@ def find_unbuilt(cases, codes) -> typing.List[dict]: continue checked.add(key) binpath = code.get_install_binpath(input_file) - if not os.path.isfile(binpath): + if not (os.path.isfile(binpath) and os.access(binpath, os.X_OK)): unbuilt[key] = {"target": code.name, "slug": key[1], "binpath": binpath, "cases": [case]} return list(unbuilt.values()) @@ -414,10 +416,10 @@ def test(): build_coverage_map(common.MFC_ROOT_DIR, all_cases, n_jobs=int(ARG("jobs"))) return - cases, skipped_cases = __filter(cases) - cases = [_.to_case() for _ in cases] + builders, skipped_cases = __filter(cases) + cases = [_.to_case() for _ in builders] if ARG("no_chemistry"): - cases, skipped_cases = _drop_chemistry_cases(cases, skipped_cases) + cases, skipped_cases = _drop_chemistry_cases(builders, cases, skipped_cases) total_test_count = len(cases) if ARG("list"): diff --git a/toolchain/mfc/test/test_nobuild_preflight.py b/toolchain/mfc/test/test_nobuild_preflight.py index 2c4d69c27e..4fc6b87dcf 100644 --- a/toolchain/mfc/test/test_nobuild_preflight.py +++ b/toolchain/mfc/test/test_nobuild_preflight.py @@ -5,6 +5,8 @@ to fail only when it ran, typically at the end of a full suite. """ +import sys + from mfc.test.test import _drop_chemistry_cases, find_unbuilt, unbuilt_message @@ -31,7 +33,7 @@ def get_slug(self, case): return f"{self.name}-{case.slug}" def get_install_binpath(self, case): - return f"/nonexistent/{self.get_slug(case)}/bin/{self.name}" if case.slug not in self.installed_slugs else __file__ + return f"/nonexistent/{self.get_slug(case)}/bin/{self.name}" if case.slug not in self.installed_slugs else sys.executable PLAIN = FakeCase("1D -> bc=-1", "AAAAAAAA", {}, "plain") @@ -81,7 +83,29 @@ def test_message_counts_distinct_cases_across_targets(): def test_no_chemistry_keys_on_the_parameter_not_the_trace(): # The reacting example has no "Chemistry" trace element but still needs a chemistry build. - kept, skipped = _drop_chemistry_cases([PLAIN, CHEM, PLAIN2, REACTING_EXAMPLE], ["already-skipped"]) + cases = [PLAIN, CHEM, PLAIN2, REACTING_EXAMPLE] + builders = ["builder-plain", "builder-chem", "builder-plain2", "builder-example"] + kept, skipped = _drop_chemistry_cases(builders, cases, ["already-skipped"]) assert kept == [PLAIN, PLAIN2] - assert skipped == ["already-skipped", CHEM, REACTING_EXAMPLE] + # Skipped entries stay builders, matching what __filter puts there. + assert skipped == ["already-skipped", "builder-chem", "builder-example"] + + +def test_a_non_executable_file_is_not_an_installed_binary(tmp_path): + binpath = tmp_path / "simulation" + binpath.write_text("") + binpath.chmod(0o644) + + class Target: + name = "simulation" + + def get_slug(self, case): + return case.slug + + def get_install_binpath(self, _case): + return str(binpath) + + assert [e["target"] for e in find_unbuilt([PLAIN], [Target()])] == ["simulation"] + binpath.chmod(0o755) + assert find_unbuilt([PLAIN], [Target()]) == []