Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the regression testing setup for MACE simulations by splitting tests into per-module scripts/macros (SimMACE/SimMMS/SimTTC), adding shared “rc” helper scripts, and expanding CI workflows to run regression tests across compilers (Clang/GCC) and MPI flavors (MPICH/OpenMPI).
Changes:
- Added modular regression test runners and data-generation scripts under
src/test/simulation/MACE/<Module>/, plus reusable helpers undersrc/test/rc/. - Added/updated ROOT macros for generating and validating regression histograms for multiple data tuples.
- Added new GitHub Actions workflows for SimMACE and SimTTC (and refactored SimMMS workflows) to run regression tests under Clang/GCC with MPICH/OpenMPI.
Reviewed changes
Copilot reviewed 28 out of 38 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/test/simulation/TestSomeDataTuple.cxx.template |
New template for writing tuple-level ROOT regression tests. |
src/test/simulation/ReadSomeDataTuple.cxx.template |
New template for generating golden histograms from a tuple. |
src/test/simulation/MACE/SimTTC/regression_test.bash |
New SimTTC regression runner using shared rc helpers. |
src/test/simulation/MACE/SimTTC/generate_regression_data.bash |
New SimTTC golden-data generator script. |
src/test/simulation/MACE/SimTTC/TestTTCSimHit.cxx |
Renames/aligns test macro to TTCSimHit. |
src/test/simulation/MACE/SimTTC/ReadTTCSimHit.cxx |
Updates TTCSimHit golden histogram generation (currently contains merge-conflict markers). |
src/test/simulation/MACE/SimMMS/regression_test.bash |
New SimMMS regression runner using shared rc helpers. |
src/test/simulation/MACE/SimMMS/generate_regression_data.bash |
New SimMMS golden-data generator script. |
src/test/simulation/MACE/SimMMS/TestMMSSimTrack.cxx |
Updates tuple name/function naming and column definitions for MMSSimTrack test. |
src/test/simulation/MACE/SimMMS/TestCDCSimHit.cxx |
Updates tuple name/function naming for CDCSimHit test. |
src/test/simulation/MACE/SimMMS/ReadMMSSimTrack.cxx |
New golden histogram generator for MMSSimTrack. |
src/test/simulation/MACE/SimMMS/ReadCDCSimHit.cxx |
Updates golden histogram generator for CDCSimHit. |
src/test/simulation/MACE/SimMACE/regression_test.bash |
New SimMACE regression runner using shared rc helpers. |
src/test/simulation/MACE/SimMACE/generate_regression_data.bash |
New SimMACE golden-data generator script. |
src/test/simulation/MACE/SimMACE/TestTTCSimHit.cxx |
New SimMACE test macro for TTCSimHit. |
src/test/simulation/MACE/SimMACE/TestMMSSimTrack.cxx |
New SimMACE test macro for MMSSimTrack. |
src/test/simulation/MACE/SimMACE/TestMCPSimHit.cxx |
New SimMACE test macro for MCPSimHit. |
src/test/simulation/MACE/SimMACE/TestCDCSimHit.cxx |
New SimMACE test macro for CDCSimHit. |
src/test/simulation/MACE/SimMACE/ReadTTCSimHit.cxx |
New SimMACE golden histogram generator for TTCSimHit. |
src/test/simulation/MACE/SimMACE/ReadMMSSimTrack.cxx |
New SimMACE golden histogram generator for MMSSimTrack. |
src/test/simulation/MACE/SimMACE/ReadMCPSimHit.cxx |
New SimMACE golden histogram generator for MCPSimHit. |
src/test/simulation/MACE/SimMACE/ReadCDCSimHit.cxx |
New SimMACE golden histogram generator for CDCSimHit. |
src/test/simulation/MACE/SimECAL/generate_regression_data.bash |
Placeholder for future SimECAL golden-data generation. |
src/test/scripts/ReadMRPCSimHit.cxx |
Removes legacy MRPC golden-hist macro from the old scripts layout. |
src/test/scripts/ReadECALSimHit.cxx |
Removes legacy ECAL golden-hist macro from the old scripts layout. |
src/test/regression_test_all.bash |
Updates “run all” script to use per-module directories and new layout. |
src/test/rc/summarize-rc.bash |
New shared end-of-run summary/exit helper. |
src/test/rc/run_command-rc.bash |
New shared command runner/logger with failure handling. |
src/test/rc/parexec-rc.bash |
New shared MPI parallel execution helper (hwthreads/physical cores). |
src/test/rc/init-rc.bash |
New shared environment + per-test working directory initializer. |
src/test/generate_regression_data_dr.bash |
Refactors golden-data generation driver into modular per-module sourcing. |
src/test/CMakeLists.txt |
Switches to copying simulation/ and rc/ trees into build/test. |
.github/workflows/regression-test-with-gcc-simttc.yml |
New CI workflow for SimTTC regression under GCC (MPICH/OpenMPI). |
.github/workflows/regression-test-with-gcc-simmms.yml |
Refactors SimMMS GCC regression workflow to new script locations/naming. |
.github/workflows/regression-test-with-gcc-simmace.yml |
New CI workflow for SimMACE regression under GCC (MPICH/OpenMPI). |
.github/workflows/regression-test-with-clang-simttc.yml |
New CI workflow for SimTTC regression under Clang (MPICH/OpenMPI). |
.github/workflows/regression-test-with-clang-simmms.yml |
Refactors SimMMS Clang regression workflow to new script locations/naming. |
.github/workflows/regression-test-with-clang-simmace.yml |
New CI workflow for SimMACE regression under Clang (MPICH/OpenMPI). |
Comments suppressed due to low confidence (4)
src/test/generate_regression_data_dr.bash:59
- This script uses
bcto computen_proc(echo "$(nproc) / $threads_per_core" | bc).bcmay not be present in minimal CI containers and can be avoided by using bash arithmetic expansion instead.
src/test/generate_regression_data_dr.bash:19 build_dirandregression_data_dirare computed fromtest_script_dir, but that variable is never defined (you likely meanttest_src_dirordriver_script_dir). As written, this script will cd into an empty path and later fail to source the offline data script.
src/test/generate_regression_data_dr.bash:66$script_diris used in themvdestination and log message, but this script no longer definesscript_dir(it was renamed todriver_script_dir). This will move the golden file to the wrong place (or fail) and print an incorrect path; use the actual variable holding the test source dir.
src/test/simulation/MACE/SimTTC/ReadTTCSimHit.cxx:54- Unresolved git merge-conflict markers (<<<<<<< / ======= / >>>>>>>) are present inside histParameterList. This will not compile and also makes it ambiguous which histogram configuration should be used; resolve the conflict and keep only the intended entries.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…actor/modulize-test
|
@zhao-shihan The PR is ready to review. |
|
@zhao-shihan The PR is ready to review. |
There was a problem hiding this comment.
Pull request overview
Refactors MACESW’s regression testing to be module-scoped (SimMACE/SimMMS/SimTTC), adds modular CI workflows for GCC/Clang across MPICH/OpenMPI, and reorganizes test drivers/scripts to improve reuse and maintainability.
Changes:
- Added per-module regression test and golden-data generation scripts, plus shared bash helpers (
init,parexec,run_command,summarize). - Introduced/updated GitHub Actions workflows for regression testing SimMACE/SimMMS/SimTTC under GCC and Clang (MPICH + OpenMPI matrices).
- Updated documentation and test CMake wiring to copy the new modular test tree into the build directory.
Reviewed changes
Copilot reviewed 34 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/gen-regression-workflows.sh | Adds a workflow YAML generator for per-module GCC/Clang regression CI. |
| src/test/source/summarize-source.bash | New shared summary/exit reporting helper for test drivers. |
| src/test/source/run_command-source.bash | New shared command wrapper for logging and failure handling. |
| src/test/source/parexec-source.bash | New shared MPI parallel-exec helper supporting hwthreads/physical cores. |
| src/test/source/init-source.bash | New shared init helper (dirs, env sourcing, golden file checks). |
| src/test/simulation/TestSomeDataTuple.cxx.template | Adds a template for writing new Test*.cxx ROOT macros. |
| src/test/simulation/ReadSomeDataTuple.cxx.template | Adds a template for writing new Read*.cxx ROOT macros. |
| src/test/simulation/MACE/SimTTC/TestTTCSimHit.cxx | Renames/aligns SimTTC test macro naming and verdict helper. |
| src/test/simulation/MACE/SimTTC/regression_test.bash | Adds SimTTC per-module regression test runner. |
| src/test/simulation/MACE/SimTTC/ReadTTCSimHit.cxx | Updates SimTTC golden histogram macro (currently contains conflict markers). |
| src/test/simulation/MACE/SimTTC/generate_regression_data.bash | Adds SimTTC per-module golden data generator (sourced by driver). |
| src/test/simulation/MACE/SimMMS/TestMMSSimTrack.cxx | Adds a new MMS track regression test macro. |
| src/test/simulation/MACE/SimMMS/TestCDCSimHit.cxx | Renames verdict helper to align with other modules. |
| src/test/simulation/MACE/SimMMS/regression_test.bash | Adds SimMMS per-module regression test runner. |
| src/test/simulation/MACE/SimMMS/ReadMMSSimTrack.cxx | Adds MMS track golden histogram macro. |
| src/test/simulation/MACE/SimMMS/ReadCDCSimHit.cxx | Repurposes/updates CDC hit golden histogram macro for SimMMS. |
| src/test/simulation/MACE/SimMMS/generate_regression_data.bash | Adds SimMMS per-module golden data generator (sourced by driver). |
| src/test/simulation/MACE/SimMACE/TestTTCSimHit.cxx | Renames verdict helper to align with other modules. |
| src/test/simulation/MACE/SimMACE/TestMMSSimTrack.cxx | Renames verdict helper to align with other modules. |
| src/test/simulation/MACE/SimMACE/TestMCPSimHit.cxx | Renames verdict helper to align with other modules. |
| src/test/simulation/MACE/SimMACE/TestCDCSimHit.cxx | Fixes tuple name and aligns verdict helper; adjusts dataframe definitions. |
| src/test/simulation/MACE/SimMACE/regression_test.bash | Adds SimMACE per-module regression test runner. |
| src/test/simulation/MACE/SimMACE/ReadTTCSimHit.cxx | Adds SimMACE TTCSimHit golden histogram macro. |
| src/test/simulation/MACE/SimMACE/ReadMMSSimTrack.cxx | Adds SimMACE MMSSimTrack golden histogram macro. |
| src/test/simulation/MACE/SimMACE/ReadMCPSimHit.cxx | Adds SimMACE MCPSimHit golden histogram macro. |
| src/test/simulation/MACE/SimMACE/ReadCDCSimHit.cxx | Adds SimMACE CDCSimHit golden histogram macro. |
| src/test/simulation/MACE/SimMACE/generate_regression_data.bash | Adds SimMACE per-module golden data generator (sourced by driver). |
| src/test/scripts/TestMRPCSimHit.cxx | Removes legacy monolithic MRPC regression test macro. |
| src/test/scripts/ReadMRPCSimHit.cxx | Removes legacy monolithic MRPC golden macro. |
| src/test/scripts/ReadECALSimHit.cxx | Removes legacy monolithic ECAL golden macro. |
| src/test/regression_test_all.bash | Updates all-in-one test driver to call module-scoped macros/scripts. |
| src/test/generate_regression_data_dr.bash | Refactors golden-data driver to source per-module generators (has path/var issues). |
| src/test/CMakeLists.txt | Switches to copying modular test tree into build dir (has file(COPY ...) arg issues). |
| docs/regression-test-guide.md | Updates terminology and documents modular per-module test layout + workflow generator. |
| .github/workflows/regression-test-with-gcc-simttc.yml | Adds SimTTC regression CI workflow for GCC (MPICH/OpenMPI). |
| .github/workflows/regression-test-with-gcc-simmms.yml | Renames/refactors SimMMS GCC regression workflow to use per-module script. |
| .github/workflows/regression-test-with-gcc-simmace.yml | Adds SimMACE regression CI workflow for GCC (MPICH/OpenMPI). |
| .github/workflows/regression-test-with-clang-simttc.yml | Adds SimTTC regression CI workflow for Clang (MPICH/OpenMPI). |
| .github/workflows/regression-test-with-clang-simmms.yml | Renames/refactors SimMMS Clang regression workflow to use per-module script. |
| .github/workflows/regression-test-with-clang-simmace.yml | Adds SimMACE regression CI workflow for Clang (MPICH/OpenMPI). |
Comments suppressed due to low confidence (3)
src/test/simulation/MACE/SimTTC/ReadTTCSimHit.cxx:55
- This file still contains unresolved Git merge conflict markers (e.g.
<<<<<<<,=======,>>>>>>>) insidehistParameterList, which will not compile and will break regression data generation. Resolve the conflict by selecting the intended histogram parameter list and removing all conflict marker lines.
src/test/generate_regression_data_dr.bash:20 build_dirandregression_data_dirare derived fromtest_script_dir, but that variable is not defined in this script (likely meanttest_src_dir). As written, the driver can source the offline data from the wrong path and create the working directory in an unexpected location.
src/test/generate_regression_data_dr.bash:66- This block uses
$script_dirwhen moving/printing the archivedmacesw_regression_data.root, butscript_diris not defined anywhere in this script (the new variable isdriver_script_dir/test_src_dir). This will move the file to the wrong place (or fail) and the log message will be misleading.
This pull request significantly refactors and expands the project's regression testing infrastructure, particularly for the SimMACE, SimMMS, and SimTTC modules. It introduces new, more modular GitHub Actions workflows for regression testing with both Clang and GCC compilers, reorganizes test scripts and their installation, and improves test script robustness and maintainability. The changes also include new utility scripts to standardize environment setup and parallel execution for tests.
The most important changes are:
1. CI/CD Workflow Expansion and Modularization
regression-test-with-clang-simmace.yml,regression-test-with-gcc-simmace.yml,regression-test-with-clang-simttc.yml,regression-test-with-gcc-simttc.yml). Each workflow builds and tests the relevant simulation with both MPICH and OpenMPI containers, and uploads test reports as artifacts. [1] [2] [3] [4]2. Test Script and Directory Organization
src/test/CMakeLists.txtto copy the entiresimulationandrcdirectories, as well as key test scripts, to the build directory, using more robust and flexible CMake commands. This replaces the previous approach of copying only individual scripts.3. Test Script Refactoring and Robustness
generate_regression_data_dr.bash, previouslygenerate_regression_data.bash) to improve path handling, modularize simulation runs, and delegate simulation-specific data generation to scripts in their respective directories. [1] [2] [3]4. New Utility Scripts for Testing
src/test/rc/init-rc.bashfor standardized test environment setup, including directory creation and environment variable sourcing.src/test/rc/parexec-rc.bashto encapsulate logic for parallel execution of tests, supporting both hardware thread and physical core usage, and handling differences between MPI implementations.These changes improve the maintainability, modularity, and scalability of the project's regression testing infrastructure, making it easier to add or modify tests for different simulation modules and compiler configurations.
Type of change: Refactor
Checklist before requesting review
mainbranch.