Skip to content

Governance: require Autotest cases for integration tests - #8274

Open
Critsium-xy wants to merge 2 commits into
deepmodeling:developfrom
Critsium-xy:governance/integration-test-drivers
Open

Critsium-xy wants to merge 2 commits into
deepmodeling:developfrom
Critsium-xy:governance/integration-test-drivers

Conversation

@Critsium-xy

Copy link
Copy Markdown
Collaborator

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

Linked Issue

No linked issue. This is a documentation-only governance change, prompted by a pattern seen in recent PRs (see below).

Unit Tests and/or Case Tests for my changes

  • Commands run:
    • git diff --cached --check
    • python tools/03_code_analysis/agent_governance_check.py --staged
  • Result summary: no whitespace errors; governance check reported no findings. Both files have LF line endings (git ls-files --eol).
  • Checks not run, with reason: no build or CTest run. The PR changes only Markdown and adds no mechanical check.

What's changed?

Integration tests in ABACUS are Autotest cases: a case directory with INPUT/STRU/KPT and result.ref, listed in CASES_*.txt and run by Autotest.sh. Several recent PRs (#8053, #8069, and the open #8235 and #8242) instead register standalone Python scripts with add_test(). These scripts launch ABACUS, build or rewrite inputs at run time, and assert on the output themselves. Each PR has invented its own launcher, environment, and fixture handling. Examples:

  • One driver rewrites another suite's INPUT by exact-whitespace string replacement.
  • One uses find_package(Python3 ... REQUIRED), which makes Python a hard configure-time dependency for MPI test builds.
  • One drops MPIEXEC_PREFLAGS/POSTFLAGS and the PROCESSORS property.

The written rules said nothing about this, so reviewers had no basis to push back.

This PR:

  • Adds AGENTS.md rule 22:
    • Integration tests are Autotest cases by default.
    • A standalone add_test() driver is allowed only for checks Autotest cannot express, such as an expected nonzero exit or per-rank stderr.
    • Python helpers called from Autotest (e.g., CompareFile.py) are unaffected.
  • Adds an "Integration Test Drivers" section to agent_governance.md with the requirements for a justified driver:
    • It lives in tests/integrate/validation_tools/ and is named check_*.py.
    • Its inputs come from a dedicated fixture directory, with no run-time rewriting of another case's INPUT.
    • The interpreter is optional: find_program plus if(), never REQUIRED.
    • MPI drivers pass through the launcher flags and set PROCESSORS.
    • It sets OMP_NUM_THREADS=1, uses a temporary scratch directory, and sets a CTest TIMEOUT.
    • The PR states why Autotest is insufficient.
  • Adds a matching rule-grading matrix row, set to AI review + human confirmation. No mechanical blocker is introduced, and existing drivers are not affected retroactively.

Governance Notes

  • INPUT/docs changes: developer documentation only (AGENTS.md, docs/developers_guide/agent_governance.md). No INPUT or user-facing behavior change.
  • Core module impact: none; no source/ changes.
  • Exceptions requested: none.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@mohanchen mohanchen added the Standards Coding standards and governance rules label Nov 4, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Standards Coding standards and governance rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants