From e9c47c101d966bd8b29dfaa756cb4bf9d6b4bd91 Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Tue, 20 Oct 2026 17:33:21 -0500 Subject: [PATCH 1/2] build: add fail-closed local verification and agent guidance --- .pre-commit-config.yaml | 1 + AGENTS.md | 25 +++- DEVELOPING.md | 13 +- scripts/README.md | 52 ++++++++ scripts/test/test_verification.py | 191 ++++++++++++++++++++++++++++++ scripts/verify.sh | 71 +++++++++++ 6 files changed, 345 insertions(+), 8 deletions(-) create mode 100644 scripts/test/test_verification.py create mode 100644 scripts/verify.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index aaa525bf2..99dd1850f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,6 +51,7 @@ repos: - pytest - PyYAML files: ^(scripts/|ci/|\.devcontainer/) + types_or: [python, shell] pass_filenames: false - repo: https://github.com/BlankSpruce/gersemi-pre-commit rev: 0.28.0 diff --git a/AGENTS.md b/AGENTS.md index ddbac032f..07381a545 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,19 +93,28 @@ PREKCOMMAND=$(command -v prek || command -v pre-commit || echo "") ## Build and Test +For local verification, use [scripts/verify.sh](scripts/README.md#build-and-test-verification) +by default. It builds first, lists the tests, and runs the same selection. It stops +on failure and rejects an empty test selection. Use a normal configured build; +local verification does not require coverage collection or a CI run. + ```bash # Configure (always pass -B explicitly; presets don't set binaryDir) cmake --preset default -B build # standard build, FORM enabled cmake --preset coverage-gcc -B build-coverage-gcc cmake --preset clang-tidy -B build-clang-tidy -# Build -ninja -C build # or: cmake --build build -j $(nproc) +# Local verification: MPD workspace (run from the workspace root) +bash srcs/phlex/scripts/verify.sh --project phlex-work-dir +bash srcs/phlex/scripts/verify.sh --project phlex-work-dir --tests '^scripts:unit$' --verbose + +# Local verification: standalone repository +bash scripts/verify.sh --build-dir build +bash scripts/verify.sh --build-dir build --tests '^identifier$' --verbose -# Test -ctest --test-dir build -j $(nproc) --output-on-failure -ctest --test-dir build -j $(nproc) --test-timeout 90 # recommended guard -ctest --test-dir build -R "regex" # specific tests +# Test only: deliberate rerun when the build is already current +ctest --test-dir build -R '^identifier$' -N && \ + ctest --test-dir build -R '^identifier$' --output-on-failure --no-tests=error --timeout 90 # Coverage ./scripts/coverage.sh all # full workflow @@ -117,6 +126,10 @@ $PREKCOMMAND run --all-files # before opening a PR $PREKCOMMAND run # changed files only ``` +For MPD test-only runs, follow the same-shell project selection and test commands +in [scripts/README.md](scripts/README.md#build-and-test-verification). Test-only +runs do not establish that the build is current. + CMake build options: `PHLEX_USE_FORM`, `ENABLE_TSAN`, `ENABLE_ASAN`, `ENABLE_COVERAGE`, `ENABLE_CLANG_TIDY`, `ENABLE_PERFETTO`. diff --git a/DEVELOPING.md b/DEVELOPING.md index 1bf68e243..3a1fa2a81 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -12,6 +12,16 @@ Except in rare circumstances, changes to the `phlex` repository are proposed and > - Both external contributors and framework developers are expected to abide by this contribution policy. > - Even if you are a contributor with merge privileges, your PR should be reviewed by someone else before being merged into the `main` branch. +## Build and Test Verification + +Use [`scripts/verify.sh`](scripts/README.md#build-and-test-verification) to build +the project and then run all tests or only those matching a supplied test-name +pattern. Tests run only after a successful build. The command returns a nonzero +exit status if the build fails, no tests match the pattern, or any test fails. +For an MPD project, it discovers the build directory automatically; for a +standalone build, supply the build directory explicitly. Test-only runs do not +establish build freshness; use them only when the build is already current. + ## Building with Coverage Phlex supports comprehensive code coverage measurement using CMake's built-in coverage support: @@ -21,8 +31,7 @@ Phlex supports comprehensive code coverage measurement using CMake's built-in co cmake -DCMAKE_BUILD_TYPE=Coverage -DENABLE_COVERAGE=ON /path/to/phlex/source # Build and run tests -cmake --build . -j $(nproc) -ctest -j $(nproc) +bash /path/to/phlex/scripts/verify.sh --build-dir . # Generate coverage reports ninja coverage-xml # XML report for CI/Codecov diff --git a/scripts/README.md b/scripts/README.md index b6b54973c..00b7e8c09 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -146,6 +146,58 @@ ninja -C build - This is informational - builds will still work - Install Ninja for faster builds: `apt install ninja-build` +## Build and Test Verification + +### verify.sh + +Use this command by default for local verification, whether you or an agent runs +the tests. Run it in an initialized build environment with a configured build. +It builds first, lists the selected CTests, and executes the same selection with +`--no-tests=error`. A failed discovery, project selection, build, listing, or test +stops verification and preserves the failing command's exit status. Build and test +output is passed through without truncation. + +Standalone builds and CTest execution request parallel jobs using the tools' +defaults; no platform-specific core-count command is required. + +This command runs the selected CTests locally; it does not start a CI workflow. +It does not enable coverage collection; use a normal configured build for local +testing without coverage. + +```bash +# MPD: discover the build directory and select/build in the same shell. +bash srcs/phlex/scripts/verify.sh --project phlex-work-dir --tests '^identifier$' + +# MPD: build and run the scripts unit test. +bash srcs/phlex/scripts/verify.sh --project phlex-work-dir --tests '^scripts:unit$' + +# Standalone: use an explicitly selected configured build directory. +bash scripts/verify.sh --build-dir build --tests '^identifier$' --verbose + +# Omit --tests to build and run the entire suite. +bash scripts/verify.sh --build-dir build +``` + +Use `--verbose` for detailed CTest evidence. A misspelled test pattern fails even +though CTest's listing-only mode reports success for an empty selection. The MPD +mode requires Spack on `PATH`; initialize its shell support before invoking the +command. + +Use direct CTest or MPD test commands only for a deliberate test-only run when the +build is already current. They do not rebuild. List the intended selection first, +then execute the same selection with `--no-tests=error`: + +```bash +# Standalone: test only, using a current build. +ctest --test-dir build -R '^identifier$' -N && \ + ctest --test-dir build -R '^identifier$' --output-on-failure --no-tests=error + +# MPD: select the project and run both commands in the same shell invocation. +spack mpd select --project phlex-work-dir && \ + spack mpd test -- -R '^scripts:unit$' -N && \ + spack mpd test -- -R '^scripts:unit$' --output-on-failure --no-tests=error +``` + ## Coverage Testing ### coverage.sh diff --git a/scripts/test/test_verification.py b/scripts/test/test_verification.py new file mode 100644 index 000000000..738230573 --- /dev/null +++ b/scripts/test/test_verification.py @@ -0,0 +1,191 @@ +"""Regression tests for fail-closed build and CTest verification.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).parent.parent + + +@pytest.fixture +def tools(tmp_path: Path) -> tuple[Path, dict[str, str]]: + """Provide recording command doubles without a Spack installation or build.""" + build = tmp_path / "build with spaces" + build.mkdir() + tool = tmp_path / "tool.py" + tool.write_text( + f"#!{sys.executable}\n" + "import json, os, sys\n" + "from pathlib import Path\n" + "name = Path(sys.argv[0]).name\n" + "args = sys.argv[1:]\n" + "if name == 'spack':\n" + " phase = {'info': 'discover', 'select': 'select', 'build': 'build'}[args[1]]\n" + "elif name == 'cmake':\n" + " phase = 'build'\n" + "elif name == 'nproc':\n" + " print('2')\n" + " sys.exit(0)\n" + "else:\n" + " phase = 'list' if '-N' in args else 'test'\n" + "with open(os.environ['COMMAND_LOG'], 'a') as log:\n" + " log.write(json.dumps([phase, name, args, os.getppid()]) + '\\n')\n" + "if phase == 'discover':\n" + " print(os.environ['TEST_BUILD_DIR'])\n" + "else:\n" + " print(phase + ' stdout')\n" + " print(phase + ' stderr', file=sys.stderr)\n" + "if phase == 'test' and '^misspelled$' in args and '--no-tests=error' in args:\n" + " sys.exit(8)\n" + "sys.exit(int(os.environ.get('FAIL_' + phase.upper(), '0')))\n", + encoding="utf-8", + ) + tool.chmod(0o755) + for name in ("spack", "cmake", "ctest", "nproc"): + (tmp_path / name).symlink_to(tool) + env = { + "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", + "COMMAND_LOG": str(tmp_path / "commands.jsonl"), + "TEST_BUILD_DIR": str(build), + } + return build, env + + +def _commands(env: dict[str, str]) -> list[list]: + log = Path(env["COMMAND_LOG"]) + return [json.loads(line) for line in log.read_text().splitlines()] if log.exists() else [] + + +def _verify(env: dict[str, str], *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["bash", str(SCRIPTS / "verify.sh"), *args], + env=env, + capture_output=True, + text=True, + check=False, + ) + + +def test_mpd_build_precedes_identical_test_selections(tools: tuple[Path, dict[str, str]]) -> None: + """Discover, select, build, list and run in one shell without losing output.""" + build, env = tools + result = _verify(env, "--project", "phlex-work-dir", "--tests", "^identifier$", "--verbose") + assert result.returncode == 0, result.stderr + commands = _commands(env) + assert [c[0] for c in commands] == ["discover", "select", "build", "list", "test"] + assert commands[0][2] == ["mpd", "info", "--build", "phlex-work-dir"] + assert commands[1][2] == ["mpd", "select", "--project", "phlex-work-dir"] + assert commands[2][2] == ["mpd", "build"] + assert commands[1][3] == commands[2][3] == commands[4][3] + selection = ["--test-dir", str(build), "-R", "^identifier$"] + assert commands[3][2] == [*selection, "-N"] + assert commands[4][2] == [ + *selection, + "-V", + "--parallel", + "--output-on-failure", + "--no-tests=error", + ] + for phase in ("build", "list", "test"): + assert f"{phase} stdout" in result.stdout + assert f"{phase} stderr" in result.stderr + + +def test_explicit_build_directory_needs_no_spack(tools: tuple[Path, dict[str, str]]) -> None: + """A standalone configured build is built and its entire suite is run.""" + build, env = tools + result = _verify(env, "--build-dir", str(build)) + assert result.returncode == 0, result.stderr + commands = _commands(env) + assert [c[1] for c in commands] == ["cmake", "ctest", "ctest"] + assert commands[0][2] == ["--build", str(build), "--parallel"] + assert "-R" not in commands[1][2] + assert "-R" not in commands[2][2] + + +@pytest.mark.parametrize("phase", ["discover", "select", "build", "list", "test"]) +def test_command_failure_stops_verification( + tools: tuple[Path, dict[str, str]], phase: str +) -> None: + """Propagate each failing command's status and skip all subsequent commands.""" + _, env = tools + env[f"FAIL_{phase.upper()}"] = "23" + result = _verify(env, "--project", "phlex-work-dir") + assert result.returncode == 23 + phases = ["discover", "select", "build", "list", "test"] + assert [c[0] for c in _commands(env)] == phases[: phases.index(phase) + 1] + + +def test_misspelled_selection_fails(tools: tuple[Path, dict[str, str]]) -> None: + """CTest execution rejects a selection even when empty discovery succeeds.""" + build, env = tools + result = _verify(env, "--build-dir", str(build), "--tests", "^misspelled$") + assert result.returncode == 8 + assert [c[0] for c in _commands(env)] == ["build", "list", "test"] + + +@pytest.mark.parametrize( + "args", + [ + (), + ("--project",), + ("--tests",), + ("--unknown",), + ("--tests", ""), + ("--project", "one", "--build-dir", "two"), + ], +) +def test_invalid_arguments_run_no_commands( + tools: tuple[Path, dict[str, str]], args: tuple[str, ...] +) -> None: + """Invalid or ambiguous input fails before discovery, building or testing.""" + _, env = tools + result = _verify(env, *args) + assert result.returncode != 0 + assert _commands(env) == [] + + +@pytest.mark.parametrize("scenario", ["pass", "empty", "test_failure", "build_failure"]) +def test_verification_with_real_cmake_and_ctest(tmp_path: Path, scenario: str) -> None: + """Validate success and failure paths using an actual miniature CMake project.""" + if not shutil.which("cmake") or not shutil.which("ctest"): + pytest.skip("CMake and CTest are required for the integration test") + source = tmp_path / "source" + source.mkdir() + (source / "CMakeLists.txt").write_text( + "cmake_minimum_required(VERSION 3.20)\n" + "project(verification NONE)\n" + "enable_testing()\n" + "add_test(NAME passing COMMAND ${CMAKE_COMMAND} -E true)\n" + "add_test(NAME failing COMMAND ${CMAKE_COMMAND} -E false)\n" + "if(FAIL_BUILD)\n" + " add_custom_target(broken ALL COMMAND ${CMAKE_COMMAND} -E false)\n" + "endif()\n", + encoding="utf-8", + ) + build = tmp_path / "build with spaces" + subprocess.run( + [ + "cmake", + "-S", + str(source), + "-B", + str(build), + f"-DFAIL_BUILD={'ON' if scenario == 'build_failure' else 'OFF'}", + ], + check=True, + capture_output=True, + text=True, + ) + pattern = {"empty": "^misspelled$", "test_failure": "^failing$"}.get(scenario, "^passing$") + result = _verify(dict(os.environ), "--build-dir", str(build), "--tests", pattern) + assert (result.returncode == 0) == (scenario == "pass"), result.stdout + result.stderr + if scenario == "build_failure": + assert "Test project" not in result.stdout diff --git a/scripts/verify.sh b/scripts/verify.sh new file mode 100644 index 000000000..dd08a6771 --- /dev/null +++ b/scripts/verify.sh @@ -0,0 +1,71 @@ +#!/bin/bash + +set -euo pipefail + +usage() { + printf '%s\n' \ + 'Usage: bash scripts/verify.sh (--project NAME | --build-dir DIR) [--tests REGEX] [--verbose]' \ + 'Build a configured project, list the selected CTests, and run them fail closed.' \ + 'Omit --tests to run the entire suite. Initialize the build environment first.' +} + +project="" +build_dir="" +pattern="" +verbose=0 +while [[ $# -gt 0 ]]; do + case "$1" in + --project|--build-dir|--tests) + if [[ $# -lt 2 || -z "$2" || "$2" == --* ]]; then + printf 'Missing value for %s\n' "$1" >&2 + exit 2 + fi + case "$1" in + --project) project="$2" ;; + --build-dir) build_dir="$2" ;; + --tests) pattern="$2" ;; + esac + shift 2 + ;; + --verbose) + verbose=1 + shift + ;; + --help|-h) + usage + exit 0 + ;; + *) + printf 'Unknown argument: %s\n' "$1" >&2 + usage >&2 + exit 2 + ;; + esac +done + +if [[ -z "$project" && -z "$build_dir" || -n "$project" && -n "$build_dir" ]]; then + usage >&2 + exit 2 +fi + +if [[ -n "$project" ]]; then + build_dir="$(spack mpd info --build "$project")" + if [[ ! -d "$build_dir" ]]; then + printf 'MPD build directory does not exist: %s\n' "$build_dir" >&2 + exit 1 + fi + spack mpd select --project "$project" + spack mpd build +else + cmake --build "$build_dir" --parallel +fi + +selection=(--test-dir "$build_dir") +if [[ -n "$pattern" ]]; then + selection+=(-R "$pattern") +fi +ctest "${selection[@]}" -N +if [[ "$verbose" == 1 ]]; then + selection+=(-V) +fi +ctest "${selection[@]}" --parallel --output-on-failure --no-tests=error From c03528a729a7f573b5c1de18c8471212fd1c5397 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:26:11 +0000 Subject: [PATCH 2/2] Run MPD verification tests in project environment Co-authored-by: marcpaterno <181471+marcpaterno@users.noreply.github.com> --- scripts/test/test_verification.py | 13 ++++++++++--- scripts/verify.sh | 12 +++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/scripts/test/test_verification.py b/scripts/test/test_verification.py index 738230573..16ab79c99 100644 --- a/scripts/test/test_verification.py +++ b/scripts/test/test_verification.py @@ -27,7 +27,10 @@ def tools(tmp_path: Path) -> tuple[Path, dict[str, str]]: "name = Path(sys.argv[0]).name\n" "args = sys.argv[1:]\n" "if name == 'spack':\n" - " phase = {'info': 'discover', 'select': 'select', 'build': 'build'}[args[1]]\n" + " if args[1] == 'test':\n" + " phase = 'list' if '-N' in args else 'test'\n" + " else:\n" + " phase = {'info': 'discover', 'select': 'select', 'build': 'build'}[args[1]]\n" "elif name == 'cmake':\n" " phase = 'build'\n" "elif name == 'nproc':\n" @@ -84,9 +87,13 @@ def test_mpd_build_precedes_identical_test_selections(tools: tuple[Path, dict[st assert commands[1][2] == ["mpd", "select", "--project", "phlex-work-dir"] assert commands[2][2] == ["mpd", "build"] assert commands[1][3] == commands[2][3] == commands[4][3] - selection = ["--test-dir", str(build), "-R", "^identifier$"] - assert commands[3][2] == [*selection, "-N"] + selection = ["-R", "^identifier$"] + assert commands[3][1] == commands[4][1] == "spack" + assert commands[3][2] == ["mpd", "test", "--", *selection, "-N"] assert commands[4][2] == [ + "mpd", + "test", + "--", *selection, "-V", "--parallel", diff --git a/scripts/verify.sh b/scripts/verify.sh index dd08a6771..e68995242 100644 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -60,12 +60,18 @@ else cmake --build "$build_dir" --parallel fi -selection=(--test-dir "$build_dir") +if [[ -n "$project" ]]; then + test_command=(spack mpd test --) + selection=() +else + test_command=(ctest) + selection=(--test-dir "$build_dir") +fi if [[ -n "$pattern" ]]; then selection+=(-R "$pattern") fi -ctest "${selection[@]}" -N +"${test_command[@]}" "${selection[@]}" -N if [[ "$verbose" == 1 ]]; then selection+=(-V) fi -ctest "${selection[@]}" --parallel --output-on-failure --no-tests=error +"${test_command[@]}" "${selection[@]}" --parallel --output-on-failure --no-tests=error