From c7c9d03926f5471f05cae046534230b481180f6a Mon Sep 17 00:00:00 2001 From: Critsium Date: Tue, 3 Nov 2026 18:14:23 +0800 Subject: [PATCH 1/3] Governance: require Autotest cases for integration tests Integration tests are Autotest case directories with result.ref, listed in CASES_*.txt. Recent PRs instead register standalone Python drivers with add_test() that launch ABACUS, rewrite fixtures at run time, and assert on output themselves. Add AGENTS.md rule 22 and an "Integration Test Drivers" section that keep Autotest as the default, allow a standalone driver only for checks Autotest cannot express, and set placement, fixture, optional-interpreter, and MPI requirements for those drivers. Add a matching human-confirmation row to the rule grading matrix; no mechanical check is introduced. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 14 +++++++ docs/developers_guide/agent_governance.md | 46 +++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 56606c2ff40..48a86ee03c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,20 @@ rules. Read the complete governance document before making or reviewing changes: guards and `#ifdef`/`#ifndef` usages of existing macros are exempt. 21. Do not initialize member variables from `PARAM` in `.h`/`.hpp` files; pass the value through the constructor or a setter instead. + 22. Write integration tests as Autotest cases: a case directory with + `INPUT`/`STRU`/`KPT` and `result.ref`, listed in the suite's + `CASES_*.txt` and run by `Autotest.sh`. Do not register a standalone + script (Python, shell, or otherwise) with `add_test()` that launches + ABACUS, rewrites a fixture at run time, and asserts on the output + itself. Python stays available as a comparison helper called from + `tests/integrate/validation_tools/` (e.g., `CompareFile.py`). A + standalone driver is allowed only for a check that Autotest cannot + express, such as an expected nonzero exit or per-rank stderr. Such a + driver must live in `tests/integrate/validation_tools/`, keep its + inputs in a dedicated fixture directory, treat the interpreter as + optional (`find_program` plus `if()`, never + `find_package(... REQUIRED)`), and the PR must state why Autotest is + insufficient. See "Integration Test Drivers" in the governance document. - Use LF line endings for text files. Only `.bat` and `.cmd` files may use CRLF. - Keep source file additions deterministic: update the relevant `CMakeLists.txt` or explain why the file is generated or included indirectly. diff --git a/docs/developers_guide/agent_governance.md b/docs/developers_guide/agent_governance.md index 53e911eafe0..9919ff45b6d 100644 --- a/docs/developers_guide/agent_governance.md +++ b/docs/developers_guide/agent_governance.md @@ -97,6 +97,7 @@ decisions. | Heterogeneous test evidence | CUDA/ROCM/kernel change has test evidence or reason | phase-one mechanical warning + AI review | CI + AI review | high | warn | changed paths and PR body | Sufficiency is human-reviewed | | Test existence | Source change has test evidence or reason | phase-one mechanical warning + AI review | CI + AI review | high | warn | PR body and changed paths | Sufficiency is human-reviewed | | Test sufficiency | Tests cover important behavior | AI review + human confirmation | AI + human review | medium | human confirmation | semantic review | Not mechanically blocked | +| Integration test driver | Integration test uses an Autotest case, not a standalone `add_test()` script | AI review + human confirmation | AI + human review | medium | human confirmation | `tests/**/CMakeLists.txt` and new scripts under `tests/` | Standalone drivers need a stated reason Autotest cannot express the check; see Integration Test Drivers | | INPUT behavior linkage | Parameter metadata/default/type/parser behavior updates YAML and docs | phase-one mechanical + AI review | CI + AI review | high | warn | behavior-field diff plus docs/PR body | Comment-only parameter-file changes are not blocked | | Documentation sync | Behavior/interface docs updated | phase-one mechanical warning + AI review | CI + AI review | medium | warn | changed paths and PR body | Major behavior changes escalate to reviewers | | PR metadata completeness | Issue, tests, behavior, INPUT, core impact, exceptions | phase-one mechanical | CI or GitHub bot | medium | warn | PR template fields | Not run by local hook | @@ -179,6 +180,51 @@ project regressions. Do not relax existing integration tests or reference files just to make a failure pass. Update references only when the intended behavior changed and the PR explains why the new reference is correct. +## Integration Test Drivers + +Integration tests use the Autotest framework. A test is a case directory under +a suite such as `tests/01_PW/`, with its own `INPUT`, `STRU`, `KPT` (when +needed), and a `result.ref`. The case is listed in the suite's `CASES_*.txt`, +and `Autotest.sh` runs it through the suite's existing CTest entry. A reviewer +can then find every case, its inputs, and its reference values in one place, +and can rerun any case the same way. + +Helper scripts that Autotest calls, such as `CompareFile.py` or `cube_tool.py` +under `tests/integrate/validation_tools/`, are part of this framework and are +not affected by this section. + +Do not bypass the framework with a standalone driver. A standalone driver is a +script registered directly with `add_test()` that launches ABACUS, builds or +rewrites inputs at run time, and decides pass or fail with its own assertions. +Such drivers spread test inputs across scripts, are invisible to the +`CASES_*.txt` lists, and copy launcher, environment, and fixture handling from +PR to PR. + +A standalone driver is acceptable only when Autotest cannot express the check. +Examples are an expected nonzero exit status, a message that must appear on +the stderr of a specific MPI rank, or a property of the process rather than of +the output files. When a driver is justified: + +- Put the script in `tests/integrate/validation_tools/` and name it after the + check it performs (for example `check_.py`). Reserve the `test_*.py` + prefix for unit tests of the validation tools themselves. +- Keep inputs in a dedicated fixture directory. Do not rewrite another case's + `INPUT` by string replacement at run time; that couples the driver to the + formatting of a fixture it does not own. +- Treat the interpreter as optional: use `find_program(... python3)` with an + `if()` guard, never `find_package(Python3 ... REQUIRED)`, so configuring the + tests never fails on a machine without Python. +- For MPI drivers, pass `MPIEXEC_NUMPROC_FLAG`, `MPIEXEC_PREFLAGS`, and + `MPIEXEC_POSTFLAGS` through, and set the CTest `PROCESSORS` property to the + number of ranks launched. +- Set `OMP_NUM_THREADS=1`, write scratch files to a temporary directory, and + set a CTest `TIMEOUT`. +- State in the PR why an Autotest case cannot express the check. + +Reviewers should ask for an Autotest case first. If part of the check fits +Autotest, that part belongs in a case directory, and the driver covers only +the remainder. + ## CLI Verification When a usable ABACUS executable is present, INPUT and command-line changes From 1e563b42fe5a2e95ef180e61942ebc675f4ec695 Mon Sep 17 00:00:00 2001 From: Xiaoyang Zhang Date: Thu, 5 Nov 2026 17:53:53 +0800 Subject: [PATCH 2/3] docs: shorten integration test driver guidance --- AGENTS.md | 20 +++---- docs/developers_guide/agent_governance.md | 64 ++++++++--------------- 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 48a86ee03c4..5c989d9a08f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,20 +79,12 @@ rules. Read the complete governance document before making or reviewing changes: guards and `#ifdef`/`#ifndef` usages of existing macros are exempt. 21. Do not initialize member variables from `PARAM` in `.h`/`.hpp` files; pass the value through the constructor or a setter instead. - 22. Write integration tests as Autotest cases: a case directory with - `INPUT`/`STRU`/`KPT` and `result.ref`, listed in the suite's - `CASES_*.txt` and run by `Autotest.sh`. Do not register a standalone - script (Python, shell, or otherwise) with `add_test()` that launches - ABACUS, rewrites a fixture at run time, and asserts on the output - itself. Python stays available as a comparison helper called from - `tests/integrate/validation_tools/` (e.g., `CompareFile.py`). A - standalone driver is allowed only for a check that Autotest cannot - express, such as an expected nonzero exit or per-rank stderr. Such a - driver must live in `tests/integrate/validation_tools/`, keep its - inputs in a dedicated fixture directory, treat the interpreter as - optional (`find_program` plus `if()`, never - `find_package(... REQUIRED)`), and the PR must state why Autotest is - insufficient. See "Integration Test Drivers" in the governance document. + 22. Use Autotest cases (`INPUT`/`STRU`/`KPT`, `result.ref`, `CASES_*.txt`, + `Autotest.sh`) for integration tests. Standalone `add_test()` drivers + are allowed only for checks Autotest cannot express; explain the need + in the PR. Keep drivers in `tests/integrate/validation_tools/` with + dedicated fixtures and an optional interpreter. Follow "Integration + Test Drivers" in the governance document; Autotest helpers are unaffected. - Use LF line endings for text files. Only `.bat` and `.cmd` files may use CRLF. - Keep source file additions deterministic: update the relevant `CMakeLists.txt` or explain why the file is generated or included indirectly. diff --git a/docs/developers_guide/agent_governance.md b/docs/developers_guide/agent_governance.md index 9919ff45b6d..d6284925de5 100644 --- a/docs/developers_guide/agent_governance.md +++ b/docs/developers_guide/agent_governance.md @@ -182,48 +182,28 @@ changed and the PR explains why the new reference is correct. ## Integration Test Drivers -Integration tests use the Autotest framework. A test is a case directory under -a suite such as `tests/01_PW/`, with its own `INPUT`, `STRU`, `KPT` (when -needed), and a `result.ref`. The case is listed in the suite's `CASES_*.txt`, -and `Autotest.sh` runs it through the suite's existing CTest entry. A reviewer -can then find every case, its inputs, and its reference values in one place, -and can rerun any case the same way. - -Helper scripts that Autotest calls, such as `CompareFile.py` or `cube_tool.py` -under `tests/integrate/validation_tools/`, are part of this framework and are -not affected by this section. - -Do not bypass the framework with a standalone driver. A standalone driver is a -script registered directly with `add_test()` that launches ABACUS, builds or -rewrites inputs at run time, and decides pass or fail with its own assertions. -Such drivers spread test inputs across scripts, are invisible to the -`CASES_*.txt` lists, and copy launcher, environment, and fixture handling from -PR to PR. - -A standalone driver is acceptable only when Autotest cannot express the check. -Examples are an expected nonzero exit status, a message that must appear on -the stderr of a specific MPI rank, or a property of the process rather than of -the output files. When a driver is justified: - -- Put the script in `tests/integrate/validation_tools/` and name it after the - check it performs (for example `check_.py`). Reserve the `test_*.py` - prefix for unit tests of the validation tools themselves. -- Keep inputs in a dedicated fixture directory. Do not rewrite another case's - `INPUT` by string replacement at run time; that couples the driver to the - formatting of a fixture it does not own. -- Treat the interpreter as optional: use `find_program(... python3)` with an - `if()` guard, never `find_package(Python3 ... REQUIRED)`, so configuring the - tests never fails on a machine without Python. -- For MPI drivers, pass `MPIEXEC_NUMPROC_FLAG`, `MPIEXEC_PREFLAGS`, and - `MPIEXEC_POSTFLAGS` through, and set the CTest `PROCESSORS` property to the - number of ranks launched. -- Set `OMP_NUM_THREADS=1`, write scratch files to a temporary directory, and - set a CTest `TIMEOUT`. -- State in the PR why an Autotest case cannot express the check. - -Reviewers should ask for an Autotest case first. If part of the check fits -Autotest, that part belongs in a case directory, and the driver covers only -the remainder. +Integration tests use Autotest cases under a suite such as `tests/01_PW/`: +dedicated `INPUT`, `STRU`, `KPT` (when needed), and `result.ref`, listed in the +suite's `CASES_*.txt` and run by `Autotest.sh` through its existing CTest entry. +This keeps inputs and references together and gives reviewers one way to find +and rerun cases. Helpers called by Autotest, such as `CompareFile.py`, are unaffected. + +Standalone `add_test()` drivers that launch ABACUS are allowed only when +Autotest cannot express the check (e.g., expected nonzero exits or per-rank +stderr). Keep Autotest-compatible checks in cases and explain the remaining +need in the PR. For a justified driver: + +- Put scripts in `tests/integrate/validation_tools/` with descriptive names + (e.g., `check_.py`); reserve `test_*.py` for tool unit tests. +- Use dedicated fixtures; do not rewrite another case's `INPUT` by string + replacement at run time. +- Make the interpreter optional with `find_program(... python3)` and `if()`; + do not use `find_package(Python3 ... REQUIRED)`, so test configuration still + succeeds without Python. +- For MPI, forward `MPIEXEC_NUMPROC_FLAG`, `MPIEXEC_PREFLAGS`, and + `MPIEXEC_POSTFLAGS`; set CTest `PROCESSORS` to the launched rank count. +- Set `OMP_NUM_THREADS=1`, use a temporary scratch directory, and set CTest + `TIMEOUT`. ## CLI Verification From 96a1b6e4d92b61c45d782d66138b1c113a17e5f3 Mon Sep 17 00:00:00 2001 From: Xiaoyang Zhang Date: Thu, 5 Nov 2026 17:57:50 +0800 Subject: [PATCH 3/3] docs: further condense integration test guidance --- AGENTS.md | 10 +++---- docs/developers_guide/agent_governance.md | 33 +++++++++-------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5c989d9a08f..5b7bccfb5d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,12 +79,10 @@ rules. Read the complete governance document before making or reviewing changes: guards and `#ifdef`/`#ifndef` usages of existing macros are exempt. 21. Do not initialize member variables from `PARAM` in `.h`/`.hpp` files; pass the value through the constructor or a setter instead. - 22. Use Autotest cases (`INPUT`/`STRU`/`KPT`, `result.ref`, `CASES_*.txt`, - `Autotest.sh`) for integration tests. Standalone `add_test()` drivers - are allowed only for checks Autotest cannot express; explain the need - in the PR. Keep drivers in `tests/integrate/validation_tools/` with - dedicated fixtures and an optional interpreter. Follow "Integration - Test Drivers" in the governance document; Autotest helpers are unaffected. + 22. Use Autotest cases for integration tests. Standalone `add_test()` drivers + require a check Autotest cannot express, a PR rationale, and compliance + with "Integration Test Drivers" in the governance document. Autotest + helpers are unaffected. - Use LF line endings for text files. Only `.bat` and `.cmd` files may use CRLF. - Keep source file additions deterministic: update the relevant `CMakeLists.txt` or explain why the file is generated or included indirectly. diff --git a/docs/developers_guide/agent_governance.md b/docs/developers_guide/agent_governance.md index d6284925de5..731be2d8889 100644 --- a/docs/developers_guide/agent_governance.md +++ b/docs/developers_guide/agent_governance.md @@ -182,28 +182,21 @@ changed and the PR explains why the new reference is correct. ## Integration Test Drivers -Integration tests use Autotest cases under a suite such as `tests/01_PW/`: -dedicated `INPUT`, `STRU`, `KPT` (when needed), and `result.ref`, listed in the -suite's `CASES_*.txt` and run by `Autotest.sh` through its existing CTest entry. -This keeps inputs and references together and gives reviewers one way to find -and rerun cases. Helpers called by Autotest, such as `CompareFile.py`, are unaffected. - -Standalone `add_test()` drivers that launch ABACUS are allowed only when -Autotest cannot express the check (e.g., expected nonzero exits or per-rank -stderr). Keep Autotest-compatible checks in cases and explain the remaining -need in the PR. For a justified driver: - -- Put scripts in `tests/integrate/validation_tools/` with descriptive names +Use Autotest cases with `INPUT`, `STRU`, `KPT` (when needed) and `result.ref`, +listed in `CASES_*.txt` and run by `Autotest.sh` through the suite's CTest entry. +Autotest helpers are unaffected. + +Standalone `add_test()` drivers that launch ABACUS require a check Autotest +cannot express (e.g., nonzero exits or per-rank stderr) and a PR rationale. +Keep Autotest-compatible checks in cases. Drivers must: + +- Live in `tests/integrate/validation_tools/` with descriptive names (e.g., `check_.py`); reserve `test_*.py` for tool unit tests. -- Use dedicated fixtures; do not rewrite another case's `INPUT` by string - replacement at run time. -- Make the interpreter optional with `find_program(... python3)` and `if()`; - do not use `find_package(Python3 ... REQUIRED)`, so test configuration still - succeeds without Python. -- For MPI, forward `MPIEXEC_NUMPROC_FLAG`, `MPIEXEC_PREFLAGS`, and +- Use dedicated fixtures; never rewrite another case's `INPUT` at run time. +- Use `find_program` plus `if()` for optional interpreters; never `REQUIRED`. +- For MPI, forward `MPIEXEC_NUMPROC_FLAG`, `MPIEXEC_PREFLAGS` and `MPIEXEC_POSTFLAGS`; set CTest `PROCESSORS` to the launched rank count. -- Set `OMP_NUM_THREADS=1`, use a temporary scratch directory, and set CTest - `TIMEOUT`. +- Set `OMP_NUM_THREADS=1`, use temporary scratch space and a CTest `TIMEOUT`. ## CLI Verification