[hzhou-muse]

Pull Request Description

test/units can test internal routines without building MPICH, but its runner always exited 0, nothing ran it, and a unit could only test functions copied out of a source file along with hand-written copies of their dependencies.

This PR makes it a usable framework:

• make unit-testing, or python3 test/units/run_unit.py before autogen/configure.
• TAP output; the run fails on a failed test, a failed build, or a test that hangs (--timeout).
• --junit FILE also writes JUnit XML for CI result publishers.
• sources: compiles real source files unchanged against small header shims in test/units/shim/ (mpl, core), so units need no generated headers.
• Units build with ASan/UBSan when available; a sanitizer report fails the test.
• Units: the rankmap units ported to sources mode, with malformed-input and round-trip cases; new circ_graph schedule tests.
• Wiki page doc/wiki/testing/Unit_Tests.md.

Found by the new tests:

• MPL_rankmap_array_to_str wrote past an array when the smallest node id is not 0; fixed here.
• circ_graph schedules are wrong for some p >= 33, and a real 33-process circ_graph bcast or allgather returns wrong data. The test checks p <= 32 for now; the fix will come in a separate PR.

Author Checklist

• Provide Description
  Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
• Commits Follow Good Practice
  Commits are self-contained and do not do two things at once.
  Commit message is of the form: module: short description
  Commit message explains what's in the commit.
• Passes All Tests
  Whitespace checker. Warnings test. Additional tests via comments.
• Contribution Agreement
  For non-Argonne authors, check contribution agreement.
  If necessary, request an explicit comment from your companies PR approval manager.

🤖 Generated with Claude Code

[hzhou-muse]

Plan

The plan this PR was developed from, including the design decisions and what the new tests found.

plan.md

Plan: unit testing framework for MPICH internals (test/units)

Goal

Make test/units a dependable way to test internal C code (parsers,
string/map converters, small algorithms) without building MPICH: failures
fail the run, make unit-testing runs it, and the wiki documents it.

Starting point (main)

test/units/run_unit.py (2022) builds a t.c from a .unit file's
template: block, pulls function bodies out of sources between
/* START UNIT - name */ and /* END UNIT */, and runs cmd:/expect: pairs.
Three units: mpl_rankmap_str_to_array, mpl_rankmap_array_to_str,
slurm_list_to_nodes.

Gaps:

• The run always exits 0. run_tests()'s error count is ignored, a nonzero
  test exit code prints "not ok" but isn't counted, and a unit that fails to
  build is skipped silently.
• Nothing runs it: no make target, Jenkins step, or workflow.
• Marker extraction makes each unit copy the function's static helpers and
  types by hand (e.g. slurm_list_to_nodes.unit reimplements
  HYDU_add_to_node_list).
• test/units/out/ is not in .gitignore.
• No sanitizers; no documentation.

Sequencing with the JSON parser PR (#8034)

• PR A (this plan): branch unit-tests from origin/main, independent of
  coll/csel: replace json-c with a built-in parser #8034.
• If PR A merges first: rebase coll/csel: replace json-c with a built-in parser #8034 on main and add one commit,
  test/units: add csel_json unit tests.
• If coll/csel: replace json-c with a built-in parser #8034 merges first: open PR C from main with the same commit.
• Either way the csel_json tests use only what PR A provides (sources mode,
  shim/core, expect_err, sanitizers), so PR A must ship shim/core
  even though its own units only need shim/mpl.

PR A design

Unit file format (backward compatible)

Keep the current indentation-based format and parser (no PyYAML dependency).
Existing template: and TESTS: blocks keep working. New optional blocks:

sources:
    src/mpl/src/mpl_rankmap.c          # compiled as-is, linked with the driver
include:
    src/mpl/include                    # extra -I, after the shim dirs
shim:
    mpl                                # test/units/shim/mpl first on -I
template:
    ...driver main() only...
TESTS:
    cmd: ./t 4 '(vector,1,2,3,0)'
    expect: 1 2 3 0

    cmd: ./t 4 'bogus'
    expect_err: err = 0x               # substring of stdout, exit code nonzero

• cmd gets $top_srcdir substituted, so a test can read repo files
  (e.g. ./t -f $top_srcdir/src/mpi/coll/coll_selection.json).
• expect = exact stdout and exit 0 (as now); expect_err = nonzero exit
  and the substring in stdout. A test with neither just needs exit 0.

Build independence

Units must build and run from a fresh git checkout, with no autogen or
configure step:

• A unit may not include generated or configured headers (mplconfig.h,
  mpichconf.h, coll_autogen.h, ...). The shims stand in for the headers
  that would pull them in; a unit whose source needs generated code is out
  of scope (or waits for the real-build mode).
• The runner needs only python3 and a C compiler.
• make unit-testing is a convenience in a configured tree (it supplies the
  configured $(CC) and runs from the build tree); it adds no dependency.
• The wiki states this rule, and the runner can be checked against it by
  running it in a clean clone.

Shims (test/units/shim/<name>/)

Small headers named like the real ones, put first on the include path, so
real sources compile unchanged:

• shim/mpl/mpl.h: libc includes, MPL_malloc/calloc/free/strdup,
  MPL_SUCCESS, MPL_ERR_* that the MPL units need.
• shim/core/mpiimpl.h: MPI_SUCCESS, MPI_ERR_*, MPL allocation,
  MPIR_Assert (aborts), MPIR_ERR_SETANDJUMPn / MPIR_ERR_CHKANDJUMPn /
  MPIR_ERR_CHECK that format the generic message into a global
  unit_last_error and jump to fn_fail, so drivers can print it.

Shims only define what a unit needs and grow as units are added. Marker
extraction stays for code with heavy dependencies (hydra slurm).

Runner (run_unit.py)

• Exit status nonzero if any unit fails to build or any test fails
  (including nonzero exit codes).
• TAP output: ok N - unit: cmd / not ok N - ... with the diff, plus a
  summary line. (Dropped -v: failures already show their output.)
• Build flags: $CC (default gcc), -g -Wall, plus
  -fsanitize=address,undefined -fno-sanitize-recover=all when a probe
  program builds and runs; --no-sanitize and UNIT_CFLAGS to override.
  Warnings are shown under the build line but are not fatal (-Werror
  would make results depend on the compiler); UNIT_CFLAGS=-Werror opts in.
  A sanitizer report fails the test, even one that expects an error.
• --timeout SEC (default 60): each test runs in its own process group,
  which is killed on timeout. Needed because a malformed rankmap string can
  hang MPL_rankmap_str_to_array (see Findings).
• Multi-line expect:: lines indented deeper than expect: continue it;
  trailing whitespace per line is ignored.
• Build in test/units/out/<unit>/ (per unit, so failures keep artifacts);
  add test/units/out/ to .gitignore.
• Keep python3 test/units/run_unit.py [unit ...]; runs before autogen.
• --junit FILE also writes the results as JUnit XML (added after review
  of the Jenkins setup: the JUnit publisher does not read TAP).

Port the existing units

• Merge the two mpl_rankmap_* units into mpl_rankmap.unit with
  sources: + shim: mpl (the shim includes the real mpl_err.h), and
  drop the START/END UNIT markers from mpl_rankmap.c.
• slurm_list_to_nodes stays on markers. -Wall showed its
  HYDU_ERR_POP stand-in returned instead of jumping to fn_fail like
  the real macro; fixed.
• Add expect_err cases for malformed mapping strings, and roundtrip
  cases (array2str output must parse back to the same map).

First core-shim unit: circ_graph

src/mpi/coll/algorithms/circ_graph/circ_graph.c builds the per-round
send/receive schedules of the circulant-graph collectives. Compile the whole
file with shim: core (plus an opaque-type stand-in for
mpidu_genq_private_pool.h and the types circ_graph.h only mentions). The
driver simulates the schedule for every p up to a bound and checks that
every rank receives each block exactly once and only sends blocks it
already has. This exercises shim/core in PR A, before the csel_json
tests land.

Result: show 17 matches the example in the source comment, but the check
fails for some p >= 33 (see Findings). Decision: PR A checks p <= 32 only,
with a comment; the fixes and the full range go to a separate circ_graph
PR.

make unit-testing

Add a unit-testing target to the top-level Makefile.am, next to testing:

unit-testing:
	CC="$(CC)" python3 $(top_srcdir)/test/units/run_unit.py

• Uses the configured compiler, so make unit-testing exercises the same
  toolchain as the build.
• run_unit.py builds each unit in test/units/out/<unit>/ under the
  current directory, created as needed (or in test/units/out when started
  from test/units). So a VPATH build keeps build products in the build
  tree without an option. (An earlier --outdir option was dropped in
  favor of this.)
• No PYTHON variable exists in configure; call python3 directly
  (autogen already requires it; document that a tarball build needs it for
  this target only).
• Running the script directly still works before autogen/configure.
• CI: Jenkins (or any job) can run make unit-testing after configure;
  wiring it into jenkins-scripts is a separate local commit there.

Wiki

doc/wiki/testing/Unit_Tests.md, linked from doc/wiki/Index.md and
doc/wiki/testing/Writing_New_Tests.md:

• When to write a unit test vs. a test/mpi test.
• Running: make unit-testing in a configured tree, or the script directly
  (all units, one unit, --no-sanitize, --timeout, --junit).
• The .unit format with a full example; sources vs. marker extraction.
• Shims: what they are, how to add a definition, keeping them minimal.
• Writing a driver: canonical stdout dump, error output for expect_err.

Commits (as pushed)

1. test/units: use TAP output and fail on errors
2. test/units: add sources and shim support
3. test/units: build with sanitizers when available
4. mpl: fix node_markers index in MPL_rankmap_array_to_str
5. test/units: time out hung tests
6. test/units: port the mpl rankmap units to sources mode
7. test/units: allow multi-line expect
8. test/units: add circ_graph schedule tests
9. build: add a make unit-testing target
10. doc/wiki: add unit test documentation
11. test/units: add --junit output

Verified: every commit passes its own units; make unit-testing in a
configured tree; run_unit.py in a fresh clone without submodules,
autogen, or configure; whitespace checker clean.

Findings from the new tests

• MPL_rankmap_array_to_str wrote past node_markers when the smallest
  node id is not 0 (two places indexed by the raw node id). Fixed in
  commit 4; caught by ASan on a roundtrip case.
• MPL_rankmap_str_to_array, not fixed (what malformed or legacy input
  should do is a design call): (vector,(0,0,1)) loops forever;
  (vector,1) with sz=4 and (vector,(1,0,1)) return success with
  entries unset; (vector,[1]x0) repeats until the map is full.
• circ_graph, on main:
  • p = 1 (allgather and reduce don't guard comm_size < 2):
    gen_rsched reads stack[-1]; works by luck in normal builds.
  • gen_ssched breaks S(r)[k] == R(r+Skip[k])[k] for some p >= 33
    (33, 65-81, 129-146, ...).
  • gen_rsched schedules a block to be forwarded in the round it is
    received for p just above powers of two (33, 65-73, 129-153,
    257-299, ...), against condition 3 in the source comment.
  • Real runs (MPIR_CVAR_*_INTRA_ALGORITHM=circ_graph, 4 KB chunks,
    1-40 chunks): 32 processes clean; 33 processes give wrong data for
    bcast (503,808 values) and allgather (3.9M values). The gen_ssched
    fix alone reduces bcast to 232,448 and leaves allgather unchanged.
  • Local branch circ-graph-fix holds the p = 1 guard and the
    gen_ssched fix; the gen_rsched fix is still to do.

Follow-up (not in PR A)

• csel_json unit tests (see sequencing).
• Fuzz mode (libFuzzer entry point, or a built-in mutation loop seeded from
  the unit's cases) for parser units; short in CI, long nightly.
• dims_create unit (needs PVAR/CHKLMEM/CVAR stubs in shim/core).
• circ_graph PR: p = 1 guard, gen_ssched fix, gen_rsched fix, and the
  unit's check widened past p = 32.
• MPL_rankmap_str_to_array malformed-input behavior.
• Real-build mode: compile the same units against a configured tree's
  headers and libmpl in Jenkins, to catch shims drifting from the real code.

Decisions

• make unit-testing target (matches make testing) instead of a GitHub
  workflow or a check-* name, which would read as part of make check.
• TAP output.
• No autogen/configure dependency for the units themselves (see
  "Build independence").

[hzhou]

test:mpich/unit-testing