From eebb3787d1820ec4ef389304771460cbb908953c Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:30:15 +0000 Subject: [PATCH 01/12] test/units: use TAP output and fail on errors Report every unit build and test as a TAP line with failure details, and exit nonzero when any of them fails. Ignore the test/units/out build directory. Previously run_unit.py always exited 0: a nonzero test exit code was not counted as a failure and a unit that failed to build was skipped. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 3 ++ test/units/run_unit.py | 62 ++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 0287b2f8c48..c0432d518b5 100644 --- a/.gitignore +++ b/.gitignore @@ -513,6 +513,9 @@ Makefile.am-stamp # /test/mpid/ch3/ /test/mpid/ch3/reorder +# /test/units/ +/test/units/out/ + # /test/util/info/ /test/util/info/infotest /test/util/info/infomany diff --git a/test/units/run_unit.py b/test/units/run_unit.py index ee82ac0b6a0..f69335ce4be 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -7,6 +7,8 @@ class G: unit_name = None template_lines = [] tests = [] + num_tests = 0 + num_failed = 0 class RE: m = None @@ -36,10 +38,18 @@ def main(): run_tests() os.chdir("..") + # TAP plan and summary + print("1..%d" % G.num_tests) + if G.num_failed: + print("# FAILED %d of %d" % (G.num_failed, G.num_tests)) + sys.exit(1) + else: + print("# all %d passed" % G.num_tests) + def load_test_units(): files = os.listdir() units = [] - for f in files: + for f in sorted(files): if RE.match(r'(\w+)\.unit', f): units.append(RE.m.group(1)) return units @@ -90,30 +100,40 @@ def build_unit(): else: cc = 'gcc' - os.system(cc + " -o t t.c") - - return os.path.exists("t") - + ret = subprocess.run(cc + " -o t t.c", shell=True, encoding='ascii', errors='replace', + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + ok = ret.returncode == 0 and os.path.exists("t") + report(ok, "%s: build" % G.unit_name, ret.stdout) + return ok def run_tests(): - errs = 0 - print("Test - %s" % G.unit_name) - for i, test in enumerate(G.tests): - print(" %d:" % (i + 1)) - print(" %s" % test['cmd']) - ret = subprocess.run(test['cmd'], shell=True, encoding='ascii', capture_output=True) + for test in G.tests: + ret = subprocess.run(test['cmd'], shell=True, encoding='ascii', errors='replace', + capture_output=True) + output = ret.stdout.strip() + diag = "" if ret.returncode: - print(" not ok, exit code = %d" % ret.returncode) - print(ret.stdout) + ok = False + diag = "exit code = %d\n" % ret.returncode + elif test['expect'] is not None and test['expect'] != output: + ok = False + diag = "expect: %s\n" % test['expect'] else: - output = ret.stdout.strip() - print(" %s" % output) - if test['expect'] and test['expect'] != output: - print(" not ok, expect: %s" % test['expect']) - errs += 1 - else: - print(" ok"); - return errs + ok = True + if not ok: + diag += "got: %s\n%s" % (output, ret.stderr) + report(ok, "%s: %s" % (G.unit_name, test['cmd']), diag) + +def report(ok, desc, diag=""): + """Print one TAP test line, with diagnostics as '#' lines when it fails""" + G.num_tests += 1 + if ok: + print("ok %d - %s" % (G.num_tests, desc)) + else: + G.num_failed += 1 + print("not ok %d - %s" % (G.num_tests, desc)) + for line in diag.rstrip().split("\n"): + print("# %s" % line) #---------------------------------------- if __name__ == "__main__": From 794e1d208ed0dc5df1ecd3ca576d4eb215650866 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:31:04 +0000 Subject: [PATCH 02/12] test/units: add sources and shim support A unit can now list real source files to compile unchanged under "sources:", extra include directories under "include:", and shim directories under "shim:". Shim directories in test/units/shim/ hold small stand-ins for headers such as mpl.h and are searched first, so the sources build without autogen or configure. A test can use "expect_err:" to expect a failing exit with a message in its output, and "$top_srcdir" in its command to refer to files in the tree. Previously a unit could only test functions copied out of a source file between START UNIT and END UNIT markers, along with hand-written copies of whatever they depend on. Co-Authored-By: Claude Opus 5.5 --- test/units/run_unit.py | 61 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/test/units/run_unit.py b/test/units/run_unit.py index f69335ce4be..06cb78c3048 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -2,11 +2,18 @@ import sys import re import subprocess +import shlex class G: unit_name = None template_lines = [] tests = [] + # optional blocks: real sources to compile, extra include dirs, and shim dirs + sources = [] + includes = [] + shims = [] + unit_dir = None + top_srcdir = None num_tests = 0 num_failed = 0 @@ -17,7 +24,9 @@ def match(pat, str, flags=0): return RE.m def main(): - os.chdir(sys.path[0]) + G.unit_dir = os.path.abspath(sys.path[0]) + G.top_srcdir = os.path.dirname(os.path.dirname(G.unit_dir)) + os.chdir(G.unit_dir) if not os.path.exists("out"): os.mkdir("out") @@ -31,6 +40,9 @@ def main(): G.unit_name = a G.template_lines = [] G.tests = [] + G.sources = [] + G.includes = [] + G.shims = [] load_unit(a + '.unit') os.chdir("out") @@ -70,15 +82,26 @@ def load_unit(file_spec): G.template_lines.append(re.sub('^ ', '', line)) elif cur_block == 'TESTS': if RE.match(r'\s+cmd:\s*(.+)', line): - cur_test = {'cmd': RE.m.group(1), 'expect': None} + cmd = RE.m.group(1).replace('$top_srcdir', shlex.quote(G.top_srcdir)) + cur_test = {'cmd': cmd, 'expect': None, 'expect_err': None} G.tests.append(cur_test) elif RE.match(r'\s+expect:\s*(.*)', line): cur_test['expect'] = RE.m.group(1).rstrip() + elif RE.match(r'\s+expect_err:\s*(.*)', line): + cur_test['expect_err'] = RE.m.group(1).rstrip() + elif cur_block in ('sources', 'include', 'shim'): + item = re.sub(r'#.*', '', line).strip() + if item: + if cur_block == 'sources': + G.sources.append(item) + elif cur_block == 'include': + G.includes.append(item) + else: + G.shims.append(item) def load_source(file_src, name): capture = False - # assume cwd is test/units - with open("../../" + file_src, "r") as In: + with open(os.path.join(G.top_srcdir, file_src), "r") as In: for line in In: if RE.match(r'\s*\/\*\s*START UNIT\s*-\s*(\w+)', line) and RE.m.group(1) == name: capture = True @@ -100,19 +123,41 @@ def build_unit(): else: cc = 'gcc' - ret = subprocess.run(cc + " -o t t.c", shell=True, encoding='ascii', errors='replace', + # shim dirs go first so that their headers replace the real ones; any .c + # file in a shim dir is compiled in as well + cmd = shlex.split(cc) + srcs = ["t.c"] + for a in G.shims: + shim_dir = os.path.join(G.unit_dir, "shim", a) + if not os.path.isdir(shim_dir): + report(False, "%s: build" % G.unit_name, "shim %s not found" % a) + return False + cmd.append("-I" + shim_dir) + srcs += sorted(os.path.join(shim_dir, f) for f in os.listdir(shim_dir) if f.endswith(".c")) + for a in G.includes: + cmd.append("-I" + os.path.join(G.top_srcdir, a)) + for a in G.sources: + srcs.append(os.path.join(G.top_srcdir, a)) + cmd += ["-o", "t"] + srcs + + ret = subprocess.run(cmd, encoding='utf-8', errors='replace', stdout=subprocess.PIPE, stderr=subprocess.STDOUT) ok = ret.returncode == 0 and os.path.exists("t") - report(ok, "%s: build" % G.unit_name, ret.stdout) + report(ok, "%s: build" % G.unit_name, ' '.join(cmd) + "\n" + ret.stdout) return ok def run_tests(): for test in G.tests: - ret = subprocess.run(test['cmd'], shell=True, encoding='ascii', errors='replace', + ret = subprocess.run(test['cmd'], shell=True, encoding='utf-8', errors='replace', capture_output=True) output = ret.stdout.strip() diag = "" - if ret.returncode: + if test['expect_err'] is not None: + # expect a failure exit and the error message in stdout + ok = ret.returncode > 0 and test['expect_err'] in output + if not ok: + diag = "exit code = %d, expect_err: %s\n" % (ret.returncode, test['expect_err']) + elif ret.returncode: ok = False diag = "exit code = %d\n" % ret.returncode elif test['expect'] is not None and test['expect'] != output: From dca4448761ae288832363982524e27635e57c14a Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:32:14 +0000 Subject: [PATCH 03/12] test/units: build with sanitizers when available Build units with -g -Wall, plus -fsanitize=address,undefined when a probe program builds and runs with it. A sanitizer report fails the test. --no-sanitize turns sanitizers off and UNIT_CFLAGS adds extra flags. Build warnings are shown under the build line. Fix what this exposes: the rankmap driver leaked the returned string, and the slurm unit's HYDU_ERR_POP returned instead of jumping to fn_fail like the real macro. Co-Authored-By: Claude Opus 5.5 --- test/units/mpl_rankmap_array_to_str.unit | 1 + test/units/run_unit.py | 58 +++++++++++++++++++----- test/units/slurm_list_to_nodes.unit | 2 +- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/test/units/mpl_rankmap_array_to_str.unit b/test/units/mpl_rankmap_array_to_str.unit index 0f7e665fc6e..7889b81bac4 100644 --- a/test/units/mpl_rankmap_array_to_str.unit +++ b/test/units/mpl_rankmap_array_to_str.unit @@ -41,6 +41,7 @@ template: } printf("%s\n", str); + MPL_free(str); return 0; } diff --git a/test/units/run_unit.py b/test/units/run_unit.py index 06cb78c3048..2fdaa59c980 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -3,6 +3,7 @@ import re import subprocess import shlex +import argparse class G: unit_name = None @@ -14,6 +15,8 @@ class G: shims = [] unit_dir = None top_srcdir = None + cc = [] + cflags = [] num_tests = 0 num_failed = 0 @@ -28,12 +31,26 @@ def main(): G.top_srcdir = os.path.dirname(os.path.dirname(G.unit_dir)) os.chdir(G.unit_dir) + parser = argparse.ArgumentParser(description="Build and run the unit tests in test/units") + parser.add_argument("units", nargs="*", help="units to run (default: all *.unit files)") + parser.add_argument("--no-sanitize", action="store_true", + help="do not build with -fsanitize=address,undefined") + opts = parser.parse_args() + if not os.path.exists("out"): os.mkdir("out") - argc = len(sys.argv) - if argc > 1: - args = sys.argv[1:] + G.cc = shlex.split(os.environ.get('CC', 'gcc')) + G.cflags = ["-g", "-Wall"] + if not opts.no_sanitize: + os.chdir("out") + G.cflags += probe_sanitizers() + os.chdir("..") + G.cflags += shlex.split(os.environ.get('UNIT_CFLAGS', '')) + print("# %s" % ' '.join(G.cc + G.cflags)) + + if opts.units: + args = opts.units else: args = load_test_units() for a in args: @@ -118,14 +135,9 @@ def build_unit(): if os.path.exists("t"): os.remove("t") - if 'CC' in os.environ: - cc = os.environ['CC'] - else: - cc = 'gcc' - # shim dirs go first so that their headers replace the real ones; any .c # file in a shim dir is compiled in as well - cmd = shlex.split(cc) + cmd = G.cc + G.cflags srcs = ["t.c"] for a in G.shims: shim_dir = os.path.join(G.unit_dir, "shim", a) @@ -143,16 +155,37 @@ def build_unit(): ret = subprocess.run(cmd, encoding='utf-8', errors='replace', stdout=subprocess.PIPE, stderr=subprocess.STDOUT) ok = ret.returncode == 0 and os.path.exists("t") - report(ok, "%s: build" % G.unit_name, ' '.join(cmd) + "\n" + ret.stdout) + if ok: + # show warnings, if any + report(ok, "%s: build" % G.unit_name, ret.stdout) + else: + report(ok, "%s: build" % G.unit_name, ' '.join(cmd) + "\n" + ret.stdout) return ok +def probe_sanitizers(): + """Return the sanitizer flags if a test program builds and runs with them""" + flags = ["-fsanitize=address,undefined", "-fno-sanitize-recover=all"] + with open("probe.c", "w") as Out: + Out.write("int main(void) { return 0; }\n") + ret = subprocess.run(G.cc + flags + ["-o", "probe", "probe.c"], capture_output=True) + if ret.returncode == 0: + ret = subprocess.run(["./probe"], capture_output=True) + if ret.returncode == 0: + return flags + else: + print("# sanitizers are not available, building without them") + return [] + def run_tests(): for test in G.tests: ret = subprocess.run(test['cmd'], shell=True, encoding='utf-8', errors='replace', capture_output=True) output = ret.stdout.strip() diag = "" - if test['expect_err'] is not None: + if re.search(r'ERROR: \w+Sanitizer|runtime error:', ret.stderr): + # a sanitizer report fails the test even if a failure is expected + ok = False + elif test['expect_err'] is not None: # expect a failure exit and the error message in stdout ok = ret.returncode > 0 and test['expect_err'] in output if not ok: @@ -170,13 +203,14 @@ def run_tests(): report(ok, "%s: %s" % (G.unit_name, test['cmd']), diag) def report(ok, desc, diag=""): - """Print one TAP test line, with diagnostics as '#' lines when it fails""" + """Print one TAP test line, followed by diagnostics as '#' lines""" G.num_tests += 1 if ok: print("ok %d - %s" % (G.num_tests, desc)) else: G.num_failed += 1 print("not ok %d - %s" % (G.num_tests, desc)) + if diag.strip(): for line in diag.rstrip().split("\n"): print("# %s" % line) diff --git a/test/units/slurm_list_to_nodes.unit b/test/units/slurm_list_to_nodes.unit index efd7ac54a3e..13d7725bd88 100644 --- a/test/units/slurm_list_to_nodes.unit +++ b/test/units/slurm_list_to_nodes.unit @@ -12,7 +12,7 @@ template: #define MPL_strdup strdup #define MPL_snprintf_nowarn snprintf #define MAX_HOSTNAME_LEN 256 - #define HYDU_ERR_POP(status,msg) if (status) return 1 + #define HYDU_ERR_POP(status,msg) do { if (status) goto fn_fail; } while (0) struct HYD_node { char *hostname; From 83f02cec04c765d0b125c6fcd9cecebc24021d36 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:34:01 +0000 Subject: [PATCH 04/12] mpl: fix node_markers index in MPL_rankmap_array_to_str node_markers has one entry per node id from min_id to max_id, so index it by node id minus min_id everywhere. Previously two places indexed it by the raw node id, writing past the end of the array when the smallest node id in the map was not 0. Co-Authored-By: Claude Opus 5.5 --- src/mpl/src/mpl_rankmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mpl/src/mpl_rankmap.c b/src/mpl/src/mpl_rankmap.c index 840075e42fc..8c08798872c 100644 --- a/src/mpl/src/mpl_rankmap.c +++ b/src/mpl/src/mpl_rankmap.c @@ -273,7 +273,7 @@ int MPL_rankmap_array_to_str(int *rankmap, int sz, char **out_mapping_str) struct node_block *p1 = &node_blocks[i_blk - 1]; if (p0->num_ranks == p1->num_ranks && p0->start_id + p0->num_nodes == p1->start_id) { p0->num_nodes++; - node_markers[p1->start_id] = -1; + node_markers[p1->start_id - min_id] = -1; i_blk--; } } @@ -312,7 +312,7 @@ int MPL_rankmap_array_to_str(int *rankmap, int sz, char **out_mapping_str) node_blocks[i_blk].num_nodes = 1; node_blocks[i_blk].num_ranks = 1; node_blocks[i_blk].start_repeat = false; - node_markers[node_id] = i_blk; + node_markers[node_id - min_id] = i_blk; i_blk++; } } From 796b61ee53471e3de5bc852a97bd168bc33abea6 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:34:25 +0000 Subject: [PATCH 05/12] test/units: time out hung tests Run each test command in its own process group and kill the group if it runs longer than --timeout seconds (default 60), reporting the test as failed. Previously a test that never finished hung the whole run. Co-Authored-By: Claude Opus 5.5 --- test/units/run_unit.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/test/units/run_unit.py b/test/units/run_unit.py index 2fdaa59c980..bfd46d3ea39 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -4,6 +4,7 @@ import subprocess import shlex import argparse +import signal class G: unit_name = None @@ -17,6 +18,7 @@ class G: top_srcdir = None cc = [] cflags = [] + timeout = 60 num_tests = 0 num_failed = 0 @@ -35,7 +37,10 @@ def main(): parser.add_argument("units", nargs="*", help="units to run (default: all *.unit files)") parser.add_argument("--no-sanitize", action="store_true", help="do not build with -fsanitize=address,undefined") + parser.add_argument("--timeout", type=int, default=60, + help="seconds before a test is killed (default: 60)") opts = parser.parse_args() + G.timeout = opts.timeout if not os.path.exists("out"): os.mkdir("out") @@ -178,11 +183,13 @@ def probe_sanitizers(): def run_tests(): for test in G.tests: - ret = subprocess.run(test['cmd'], shell=True, encoding='utf-8', errors='replace', - capture_output=True) + ret = run_cmd(test['cmd']) output = ret.stdout.strip() diag = "" - if re.search(r'ERROR: \w+Sanitizer|runtime error:', ret.stderr): + if ret.returncode is None: + ok = False + diag = "timed out after %d seconds\n" % G.timeout + elif re.search(r'ERROR: \w+Sanitizer|runtime error:', ret.stderr): # a sanitizer report fails the test even if a failure is expected ok = False elif test['expect_err'] is not None: @@ -202,6 +209,19 @@ def run_tests(): diag += "got: %s\n%s" % (output, ret.stderr) report(ok, "%s: %s" % (G.unit_name, test['cmd']), diag) +def run_cmd(cmd): + """Run a test command in its own process group; returncode is None if it timed out""" + proc = subprocess.Popen(cmd, shell=True, encoding='utf-8', errors='replace', + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + start_new_session=True) + try: + stdout, stderr = proc.communicate(timeout=G.timeout) + return subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr) + except subprocess.TimeoutExpired: + os.killpg(proc.pid, signal.SIGKILL) + stdout, stderr = proc.communicate() + return subprocess.CompletedProcess(cmd, None, stdout, stderr) + def report(ok, desc, diag=""): """Print one TAP test line, followed by diagnostics as '#' lines""" G.num_tests += 1 From 5cb3549673269964b4de8dd6a2926262726b051e Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:34:25 +0000 Subject: [PATCH 06/12] test/units: port the mpl rankmap units to sources mode Merge the two rankmap units into mpl_rankmap.unit, which compiles src/mpl/src/mpl_rankmap.c as-is against a stand-in mpl.h in test/units/shim/mpl, and drop the now unused START UNIT/END UNIT markers from that file. Add cases for malformed mapping strings and round-trip checks that array2str output parses back to the same map. Previously the units copied the functions out between the markers along with hand-written definitions of MPL_malloc and the error codes. Co-Authored-By: Claude Opus 5.5 --- src/mpl/src/mpl_rankmap.c | 7 - test/units/mpl_rankmap.unit | 158 +++++++++++++++++++++++ test/units/mpl_rankmap_array_to_str.unit | 63 --------- test/units/mpl_rankmap_str_to_array.unit | 60 --------- test/units/shim/mpl/mpl.h | 35 +++++ 5 files changed, 193 insertions(+), 130 deletions(-) create mode 100644 test/units/mpl_rankmap.unit delete mode 100644 test/units/mpl_rankmap_array_to_str.unit delete mode 100644 test/units/mpl_rankmap_str_to_array.unit create mode 100644 test/units/shim/mpl/mpl.h diff --git a/src/mpl/src/mpl_rankmap.c b/src/mpl/src/mpl_rankmap.c index 8c08798872c..351e3e27e2c 100644 --- a/src/mpl/src/mpl_rankmap.c +++ b/src/mpl/src/mpl_rankmap.c @@ -30,8 +30,6 @@ * A block_repeat repeats the bracketed block list by num of times. */ -/* START UNIT - MPL_rankmap_str_to_array */ - #define RANKMAP_PARSE_ERROR() do { goto fn_fail; } while (0) /* advance _c until we find a non whitespace character */ @@ -172,9 +170,6 @@ int MPL_rankmap_str_to_array(char *mapping, int sz, int *out_rankmap) return MPL_ERR_FAIL; } -/* END UNIT - MPL_rankmap_str_to_array */ - -/* START UNIT - MPL_rankmap_array_to_str */ /* Generate a string from the rankmap array */ #define MAPSTR_GROW(n) do { \ @@ -347,5 +342,3 @@ int MPL_rankmap_array_to_str(int *rankmap, int sz, char **out_mapping_str) return MPL_SUCCESS; } - -/* END UNIT - MPL_rankmap_array_to_str */ diff --git a/test/units/mpl_rankmap.unit b/test/units/mpl_rankmap.unit new file mode 100644 index 00000000000..a749b3bbd45 --- /dev/null +++ b/test/units/mpl_rankmap.unit @@ -0,0 +1,158 @@ +sources: + src/mpl/src/mpl_rankmap.c + +shim: + mpl + +template: + #include "mpl.h" + + #define MAX_SZ 100 + int map[MAX_SZ]; + int map2[MAX_SZ]; + + /* ./t str2array sz mapping - parse a rankmap string and print the map + * ./t array2str id id ... - print the rankmap string of a map + * ./t roundtrip id id ... - array2str followed by str2array, which must give back the map + */ + int main(int argc, char **argv) + { + int err; + + if (argc < 2) { + printf("usage: t str2array|array2str|roundtrip ...\n"); + return 1; + } + + if (strcmp(argv[1], "str2array") == 0) { + int sz = atoi(argv[2]); + if (sz > MAX_SZ) { + printf("sz exceeds limit (%d > %d)\n", sz, MAX_SZ); + return 1; + } + + err = MPL_rankmap_str_to_array(argv[3], sz, map); + if (err) { + printf("err = 0x%x\n", err); + return 1; + } + + for (int i = 0; i < sz; i++) { + printf("%d ", map[i]); + } + printf("\n"); + return 0; + } + + int sz = argc - 2; + if (sz > MAX_SZ) { + printf("sz exceeds limit (%d > %d)\n", sz, MAX_SZ); + return 1; + } + for (int i = 0; i < sz; i++) { + map[i] = atoi(argv[2 + i]); + } + + char *str; + err = MPL_rankmap_array_to_str(map, sz, &str); + if (err) { + printf("err = 0x%x\n", err); + return 1; + } + if (str == NULL) { + printf("str is NULL\n"); + return 1; + } + printf("%s\n", str); + + if (strcmp(argv[1], "roundtrip") == 0) { + err = MPL_rankmap_str_to_array(str, sz, map2); + if (err) { + printf("str2array err = 0x%x\n", err); + MPL_free(str); + return 1; + } + for (int i = 0; i < sz; i++) { + if (map2[i] != map[i]) { + printf("mismatch at rank %d: %d != %d\n", i, map2[i], map[i]); + MPL_free(str); + return 1; + } + } + } + + MPL_free(str); + return 0; + } + +TESTS: + cmd: ./t str2array 4 '(vector,1,2,3,0)' + expect: 1 2 3 0 + + cmd: ./t str2array 12 '(vector,(0,3,4))' + expect: 0 0 0 0 1 1 1 1 2 2 2 2 + + cmd: ./t str2array 12 '(vector,[(0,3,1)]x4)' + expect: 0 1 2 0 1 2 0 1 2 0 1 2 + + cmd: ./t str2array 12 '(vector,[(0,3,1)]x3,(0,3,1))' + expect: 0 1 2 0 1 2 0 1 2 0 1 2 + + cmd: ./t str2array 12 '(vector,[(0,2,1),2]x3,(0,3,1))' + expect: 0 1 2 0 1 2 0 1 2 0 1 2 + + cmd: ./t str2array 4 ' ( vector , 1 , 2 , 3 , 0 ) ' + expect: 1 2 3 0 + + # legacy with default repeats from node 0 + + cmd: ./t str2array 12 '(vector,(1,3,1),(0,4,1))' + expect: 1 2 3 0 1 2 3 0 1 2 3 0 + + # malformed strings + + cmd: ./t str2array 4 '' + expect_err: err = 0x1 + + cmd: ./t str2array 4 'vector,1,2,3,0' + expect_err: err = 0x1 + + cmd: ./t str2array 4 '(vectors,1,2,3,0)' + expect_err: err = 0x1 + + cmd: ./t str2array 4 '(vector,1;2,3,0)' + expect_err: err = 0x1 + + cmd: ./t str2array 4 '(vector,(0,2))' + expect_err: err = 0x1 + + cmd: ./t str2array 4 '(vector,1]x2)' + expect_err: err = 0x1 + + cmd: ./t array2str 3 2 1 0 + expect: (vector,3,2,1,0) + + cmd: ./t array2str 1 2 3 0 + expect: (vector,(1,3,1),0) + + cmd: ./t array2str 0 0 0 0 1 1 1 1 2 2 2 2 + expect: (vector,(0,3,4)) + + cmd: ./t array2str 0 1 2 0 1 2 0 1 2 0 1 2 + expect: (vector,[(0,3,1)]x4) + + cmd: ./t array2str 1 2 3 0 1 2 3 0 1 2 3 0 + expect: (vector,[(1,3,1),0]x3) + + # array2str output must parse back to the same map + + cmd: ./t roundtrip 0 + cmd: ./t roundtrip 0 0 0 0 0 0 0 0 + cmd: ./t roundtrip 0 1 2 3 4 5 6 7 + cmd: ./t roundtrip 7 6 5 4 3 2 1 0 + cmd: ./t roundtrip 0 0 1 1 2 2 0 0 1 1 2 2 + cmd: ./t roundtrip 0 1 0 1 0 1 2 2 3 3 + cmd: ./t roundtrip 3 1 4 1 5 9 2 6 5 3 5 8 9 7 9 + cmd: ./t roundtrip 5 6 7 5 6 7 5 6 7 + cmd: ./t roundtrip 4 5 4 5 6 7 6 7 + cmd: ./t roundtrip 0 0 0 1 1 1 2 2 2 0 0 0 1 1 1 2 2 2 3 diff --git a/test/units/mpl_rankmap_array_to_str.unit b/test/units/mpl_rankmap_array_to_str.unit deleted file mode 100644 index 7889b81bac4..00000000000 --- a/test/units/mpl_rankmap_array_to_str.unit +++ /dev/null @@ -1,63 +0,0 @@ -template: - #include - #include - #include - #include - #include - #include - - #define MAX_SZ 100 - int map[MAX_SZ]; - - #define MPL_SUCCESS 0 - #define MPL_ERR_FAIL 1 - #define MPL_malloc(sz,t) malloc(sz) - #define MPL_realloc(p,sz,t) realloc(p,sz) - #define MPL_free free - - [src/mpl/src/mpl_rankmap.c:MPL_rankmap_array_to_str] - - int main(int argc, char **argv) - { - int sz = argc - 1; - if (sz > MAX_SZ) { - printf("sz exceeds limit (%d > %d)\n", sz, MAX_SZ); - return 1; - } - for (int i = 0; i < sz; i++) { - map[i] = atoi(argv[1 + i]); - } - - char *str; - int err = MPL_rankmap_array_to_str(map, sz, &str); - - if (err) { - printf("err = 0x%x\n", err); - return 1; - } - if (str == NULL) { - printf("str is NULL\n"); - return 1; - } - - printf("%s\n", str); - MPL_free(str); - - return 0; - } - -TESTS: - cmd: ./t 3 2 1 0 - expect: (vector,3,2,1,0) - - cmd: ./t 1 2 3 0 - expect: (vector,(1,3,1),0) - - cmd: ./t 0 0 0 0 1 1 1 1 2 2 2 2 - expect: (vector,(0,3,4)) - - cmd: ./t 0 1 2 0 1 2 0 1 2 0 1 2 - expect: (vector,[(0,3,1)]x4) - - cmd: ./t 1 2 3 0 1 2 3 0 1 2 3 0 - expect: (vector,[(1,3,1),0]x3) diff --git a/test/units/mpl_rankmap_str_to_array.unit b/test/units/mpl_rankmap_str_to_array.unit deleted file mode 100644 index ed0f3f4dcc8..00000000000 --- a/test/units/mpl_rankmap_str_to_array.unit +++ /dev/null @@ -1,60 +0,0 @@ -template: - #include - #include - #include - #include - #include - - #define MAX_SZ 100 - int map[MAX_SZ]; - - #define MPL_SUCCESS 0 - #define MPL_ERR_FAIL 1 - - [src/mpl/src/mpl_rankmap.c:MPL_rankmap_str_to_array] - - int main(int argc, char **argv) - { - int sz = atoi(argv[1]); - char *mapping = argv[2]; - - if (sz > MAX_SZ) { - printf("sz exceeds limit (%d > %d)\n", sz, MAX_SZ); - return 1; - } - - int err = MPL_rankmap_str_to_array(mapping, sz, map); - - if (err) { - printf("err = 0x%x\n", err); - return 1; - } - - for (int i = 0; i < sz; i++) { - printf("%d ", map[i]); - } - printf("\n"); - - return 0; - } - -TESTS: - cmd: ./t 4 '(vector,1,2,3,0)' - expect: 1 2 3 0 - - cmd: ./t 12 '(vector,(0,3,4))' - expect: 0 0 0 0 1 1 1 1 2 2 2 2 - - cmd: ./t 12 '(vector,[(0,3,1)]x4)' - expect: 0 1 2 0 1 2 0 1 2 0 1 2 - - cmd: ./t 12 '(vector,[(0,3,1)]x3,(0,3,1))' - expect: 0 1 2 0 1 2 0 1 2 0 1 2 - - cmd: ./t 12 '(vector,[(0,2,1),2]x3,(0,3,1))' - expect: 0 1 2 0 1 2 0 1 2 0 1 2 - - # legacy with default repeats from node 0 - - cmd: ./t 12 '(vector,(1,3,1),(0,4,1))' - expect: 1 2 3 0 1 2 3 0 1 2 3 0 diff --git a/test/units/shim/mpl/mpl.h b/test/units/shim/mpl/mpl.h new file mode 100644 index 00000000000..ee853c104e6 --- /dev/null +++ b/test/units/shim/mpl/mpl.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) by Argonne National Laboratory + * See COPYRIGHT in top-level directory + */ + +/* Unit test stand-in for src/mpl/include/mpl.h. The real header includes the + * configure-generated mplconfig.h; this one defines only what the units use. + * Add to it as needed, keeping the real signatures. */ + +#ifndef MPL_H_INCLUDED +#define MPL_H_INCLUDED + +#include +#include +#include +#include +#include +#include +#include + +/* MPL_SUCCESS, MPL_ERR_* */ +#include "../../../../src/mpl/include/mpl_err.h" + +/* memory allocation, ignoring the memory class */ +#define MPL_malloc(size, class) malloc(size) +#define MPL_calloc(n, size, class) calloc(n, size) +#define MPL_realloc(p, size, class) realloc(p, size) +#define MPL_strdup(s) strdup(s) +#define MPL_free(p) free(p) + +/* src/mpl/src/mpl_rankmap.c */ +int MPL_rankmap_str_to_array(char *mapping, int sz, int *out_nodemap); +int MPL_rankmap_array_to_str(int *nodemap, int sz, char **out_mapping_str); + +#endif /* MPL_H_INCLUDED */ From 8d93fece1e4d3227a56f6f87be4c45e8571c1b05 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 00:38:27 +0000 Subject: [PATCH 07/12] test/units: allow multi-line expect Lines indented deeper than "expect:" continue the expected output, so a test can check output of several lines. Trailing whitespace on each line is ignored when comparing. Co-Authored-By: Claude Opus 5.5 --- test/units/run_unit.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/units/run_unit.py b/test/units/run_unit.py index bfd46d3ea39..4a0aedefeb7 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -91,6 +91,7 @@ def load_test_units(): def load_unit(file_spec): cur_block = None cur_test = None + in_expect = False with open(file_spec, "r") as In: for line in In: @@ -103,12 +104,21 @@ def load_unit(file_spec): # trim the leading indentation G.template_lines.append(re.sub('^ ', '', line)) elif cur_block == 'TESTS': + if in_expect and RE.match(r' \s*(\S.*)', line): + # a line indented deeper than "expect:" continues it + if cur_test['expect']: + cur_test['expect'] += "\n" + RE.m.group(1).rstrip() + else: + cur_test['expect'] = RE.m.group(1).rstrip() + continue + in_expect = False if RE.match(r'\s+cmd:\s*(.+)', line): cmd = RE.m.group(1).replace('$top_srcdir', shlex.quote(G.top_srcdir)) cur_test = {'cmd': cmd, 'expect': None, 'expect_err': None} G.tests.append(cur_test) elif RE.match(r'\s+expect:\s*(.*)', line): cur_test['expect'] = RE.m.group(1).rstrip() + in_expect = True elif RE.match(r'\s+expect_err:\s*(.*)', line): cur_test['expect_err'] = RE.m.group(1).rstrip() elif cur_block in ('sources', 'include', 'shim'): @@ -184,7 +194,8 @@ def probe_sanitizers(): def run_tests(): for test in G.tests: ret = run_cmd(test['cmd']) - output = ret.stdout.strip() + # ignore trailing whitespace on each line + output = '\n'.join(l.rstrip() for l in ret.stdout.strip().split('\n')) diag = "" if ret.returncode is None: ok = False From 6423a9a335b99611d4737a0bc16f88eb7e854d8a Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 01:03:33 +0000 Subject: [PATCH 08/12] test/units: add circ_graph schedule tests Add a shim for mpiimpl.h in test/units/shim/core, with the MPIR_ERR_ macros recording their message in unit_last_error, and use it to test src/mpi/coll/algorithms/circ_graph/circ_graph.c. The test prints the p = 17 schedules to compare with the example in the source comment, and simulates a bcast for every p up to 32 with up to 40 blocks, checking that sends match receives, that a process only sends blocks it already has, and that every process ends up with every block. It also checks that MPII_circ_graph_create_all agrees with MPII_circ_graph_create. Co-Authored-By: Claude Opus 5.5 --- test/units/circ_graph.unit | 189 +++++++++++++++++++++++++++++++++ test/units/shim/core/mpiimpl.h | 71 +++++++++++++ test/units/shim/core/shim.c | 17 +++ test/units/shim/mpl/mpl.h | 6 ++ 4 files changed, 283 insertions(+) create mode 100644 test/units/circ_graph.unit create mode 100644 test/units/shim/core/mpiimpl.h create mode 100644 test/units/shim/core/shim.c diff --git a/test/units/circ_graph.unit b/test/units/circ_graph.unit new file mode 100644 index 00000000000..6ed422421eb --- /dev/null +++ b/test/units/circ_graph.unit @@ -0,0 +1,189 @@ +sources: + src/mpi/coll/algorithms/circ_graph/circ_graph.c + +include: + src/mpi/coll/algorithms/circ_graph + src/mpid/common/genq + +shim: + core + +template: + #include "mpiimpl.h" + #include "circ_graph.h" + + /* ./t show p - print the Skip, R and S schedules for p processes + * ./t check pmax nmax - simulate bcast for every 2 <= p <= pmax and n <= nmax blocks + */ + + static void show(int p) + { + MPII_circ_graph all; + MPII_circ_graph_create_all(&all, p); + int q = all.q; + + printf("Skip:"); + for (int k = 0; k <= q; k++) { + printf(" %d", all.Skip[k]); + } + printf("\n"); + for (int k = 0; k < q; k++) { + printf("R[%d]:", k); + for (int r = 0; r < p; r++) { + printf(" %d", all.R[r * q + k]); + } + printf("\n"); + } + for (int k = 0; k < q; k++) { + printf("S[%d]:", k); + for (int r = 0; r < p; r++) { + printf(" %d", all.S[r * q + k]); + } + printf("\n"); + } + MPII_circ_graph_free(&all); + } + + /* Run the bcast schedule the way MPIR_Bcast_intra_circ_graph does, with + * root 0, and check that every send matches the peer's receive, that a + * process only sends blocks it already has, that no block is received + * twice except the clamped last block, and that all processes end up + * with all n blocks. */ + static int check_bcast(MPII_circ_graph * cga, int p, int n, bool * has) + { + int q = cga[0].q; + int x = (q - ((n - 1) % q)) % q; + int offset = -x; + + memset(has, 0, p * n * sizeof(bool)); + for (int b = 0; b < n; b++) { + has[b] = true; + } + + for (int i = x; i < n - 1 + q + x; i++) { + int k = i % q; + int skip = cga[0].Skip[k]; + for (int r = 0; r < p; r++) { + int peer = (r + skip) % p; + int send_block = cga[r].S[k] + offset; + int recv_block = cga[peer].R[k] + offset; + bool do_send = (send_block >= 0 && peer != 0); + bool do_recv = (recv_block >= 0 && peer != 0); + if (do_send != do_recv || (do_send && send_block != recv_block)) { + printf("p=%d n=%d round %d: %d sends %d but %d receives %d\n", + p, n, i, r, do_send ? send_block : -1, peer, do_recv ? recv_block : -1); + return 1; + } + if (!do_send) { + continue; + } + int b = send_block >= n ? n - 1 : send_block; + if (!has[r * n + b]) { + printf("p=%d n=%d round %d: %d sends block %d it does not have\n", + p, n, i, r, b); + return 1; + } + if (has[peer * n + b] && send_block < n) { + printf("p=%d n=%d round %d: %d receives block %d twice\n", p, n, i, peer, b); + return 1; + } + } + /* receives complete at the end of the round */ + for (int r = 0; r < p; r++) { + int peer = (r + skip) % p; + int send_block = cga[r].S[k] + offset; + if (send_block >= 0 && peer != 0) { + has[peer * n + (send_block >= n ? n - 1 : send_block)] = true; + } + } + if (k == q - 1) { + offset += q; + } + } + + for (int r = 0; r < p; r++) { + for (int b = 0; b < n; b++) { + if (!has[r * n + b]) { + printf("p=%d n=%d: %d is missing block %d\n", p, n, r, b); + return 1; + } + } + } + return 0; + } + + static int check(int pmax, int nmax) + { + MPII_circ_graph *cga = malloc(pmax * sizeof(MPII_circ_graph)); + bool *has = malloc(pmax * nmax * sizeof(bool)); + int err = 0; + + for (int p = 2; p <= pmax && !err; p++) { + for (int r = 0; r < p; r++) { + MPII_circ_graph_create(&cga[r], p, r); + } + + /* create_all must agree with create for every rank */ + MPII_circ_graph all; + MPII_circ_graph_create_all(&all, p); + for (int r = 0; r < p && !err; r++) { + for (int k = 0; k < all.q; k++) { + if (all.R[r * all.q + k] != cga[r].R[k] || all.S[r * all.q + k] != cga[r].S[k]) { + printf("p=%d: create_all differs from create at r=%d k=%d\n", p, r, k); + err = 1; + break; + } + } + } + MPII_circ_graph_free(&all); + + for (int n = 1; n <= nmax && !err; n++) { + err = check_bcast(cga, p, n, has); + } + + for (int r = 0; r < p; r++) { + MPII_circ_graph_free(&cga[r]); + } + } + + free(cga); + free(has); + if (!err) { + printf("ok\n"); + } + return err; + } + + int main(int argc, char **argv) + { + if (argc == 3 && strcmp(argv[1], "show") == 0) { + show(atoi(argv[2])); + return 0; + } else if (argc == 4 && strcmp(argv[1], "check") == 0) { + return check(atoi(argv[2]), atoi(argv[3])); + } + printf("usage: t show p | t check pmax nmax\n"); + return 1; + } + +TESTS: + # the p=17 example in the comment of circ_graph.c, with [b] written as b + cmd: ./t show 17 + expect: + Skip: 1 2 3 5 9 17 + R[0]: -4 0 -5 -4 -3 -5 -2 -5 -4 -3 -1 -5 -4 -3 -5 -2 -5 + R[1]: -5 -4 1 -5 -4 -3 -3 -2 -5 -4 -3 -1 -5 -4 -3 -3 -2 + R[2]: -2 -2 -2 2 0 -4 -4 -3 -2 -2 -4 -3 -1 -1 -4 -4 -3 + R[3]: -1 -3 -3 -2 -2 3 0 1 2 -5 -2 -2 -2 -2 -1 -1 -1 + R[4]: -3 -1 -1 -1 -1 -1 -1 -1 -1 4 0 1 2 0 3 0 1 + S[0]: 0 -5 -4 -3 -5 -2 -5 -4 -3 -1 -5 -4 -3 -5 -2 -5 -4 + S[1]: 1 -5 -4 -3 -3 -2 -5 -4 -3 -1 -5 -4 -3 -3 -2 -5 -4 + S[2]: 2 0 -4 -4 -3 -2 -2 -4 -3 -1 -1 -4 -4 -3 -2 -2 -2 + S[3]: 3 0 1 2 -5 -2 -2 -2 -2 -1 -1 -1 -1 -3 -3 -2 -2 + S[4]: 4 0 1 2 0 3 0 1 -3 -1 -1 -1 -1 -1 -1 -1 -1 + + # every p up to 32 with up to 40 blocks. Larger p is not checked yet: for + # some p >= 33 (33, 65-73, 129-153, ...) the schedules break the + # correctness conditions in circ_graph.c, which is being fixed separately. + cmd: ./t check 32 40 + expect: ok diff --git a/test/units/shim/core/mpiimpl.h b/test/units/shim/core/mpiimpl.h new file mode 100644 index 00000000000..ed08cd40014 --- /dev/null +++ b/test/units/shim/core/mpiimpl.h @@ -0,0 +1,71 @@ +/* + * Copyright (C) by Argonne National Laboratory + * See COPYRIGHT in top-level directory + */ + +/* Unit test stand-in for src/include/mpiimpl.h. The real header pulls in + * generated and configured headers; this one defines only what the units use. + * Add to it as needed, keeping the real names and argument order. */ + +#ifndef MPIIMPL_H_INCLUDED +#define MPIIMPL_H_INCLUDED + +#include "../mpl/mpl.h" + +#define MPI_SUCCESS 0 +#define MPI_ERR_OTHER 15 +#define MPI_ERR_INTERN 16 + +typedef intptr_t MPI_Aint; +typedef int MPI_Datatype; +typedef int MPI_Op; + +/* Types that headers under test only mention. Pointers to them are fine as + * incomplete types; the ones used by value are placeholders whose layout is + * NOT the real one, so units must not depend on their contents. */ +typedef struct MPIR_Comm MPIR_Comm; +typedef struct MPIR_Request MPIR_Request; +typedef struct { + int placeholder; +} MPIR_gpu_req; + +/* Errors: instead of creating an error code, the MPIR_ERR_ macros format the + * generic message, or the specific message with its arguments, into + * unit_last_error, so a driver can print it. */ +extern char unit_last_error[1024]; +void unit_set_error(const char *fmt, ...); + +#define MPIR_ERR_SETANDJUMP(err_, class_, gmsg_) \ + do { unit_set_error("%s", gmsg_); err_ = class_; goto fn_fail; } while (0) +#define MPIR_ERR_SETANDJUMP_N(err_, class_, gmsg_, smsg_, ...) \ + do { unit_set_error(smsg_, __VA_ARGS__); err_ = class_; goto fn_fail; } while (0) +#define MPIR_ERR_SETANDJUMP1 MPIR_ERR_SETANDJUMP_N +#define MPIR_ERR_SETANDJUMP2 MPIR_ERR_SETANDJUMP_N +#define MPIR_ERR_SETANDJUMP3 MPIR_ERR_SETANDJUMP_N +#define MPIR_ERR_SETANDJUMP4 MPIR_ERR_SETANDJUMP_N + +#define MPIR_ERR_CHKANDJUMP(cond_, err_, class_, gmsg_) \ + do { if (cond_) MPIR_ERR_SETANDJUMP(err_, class_, gmsg_); } while (0) +#define MPIR_ERR_CHKANDJUMP_N(cond_, err_, class_, gmsg_, smsg_, ...) \ + do { if (cond_) MPIR_ERR_SETANDJUMP_N(err_, class_, gmsg_, smsg_, __VA_ARGS__); } while (0) +#define MPIR_ERR_CHKANDJUMP1 MPIR_ERR_CHKANDJUMP_N +#define MPIR_ERR_CHKANDJUMP2 MPIR_ERR_CHKANDJUMP_N +#define MPIR_ERR_CHKANDJUMP3 MPIR_ERR_CHKANDJUMP_N +#define MPIR_ERR_CHKANDJUMP4 MPIR_ERR_CHKANDJUMP_N + +#define MPIR_ERR_CHECK(err_) do { if (err_) goto fn_fail; } while (0) +#define MPIR_ERR_POP(err_) do { goto fn_fail; } while (0) + +/* Assertions are always on and abort, so a failed one fails the test even if + * it expects an error. */ +#define MPIR_Assert(a_) \ + do { \ + if (!(a_)) { \ + fprintf(stderr, "Assertion failed in file %s at line %d: %s\n", \ + __FILE__, __LINE__, #a_); \ + abort(); \ + } \ + } while (0) +#define MPIR_Assertp(a_) MPIR_Assert(a_) + +#endif /* MPIIMPL_H_INCLUDED */ diff --git a/test/units/shim/core/shim.c b/test/units/shim/core/shim.c new file mode 100644 index 00000000000..d34f3997c1a --- /dev/null +++ b/test/units/shim/core/shim.c @@ -0,0 +1,17 @@ +/* + * Copyright (C) by Argonne National Laboratory + * See COPYRIGHT in top-level directory + */ + +#include "mpiimpl.h" +#include + +char unit_last_error[1024]; + +void unit_set_error(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + vsnprintf(unit_last_error, sizeof(unit_last_error), fmt, ap); + va_end(ap); +} diff --git a/test/units/shim/mpl/mpl.h b/test/units/shim/mpl/mpl.h index ee853c104e6..b4c01c53d81 100644 --- a/test/units/shim/mpl/mpl.h +++ b/test/units/shim/mpl/mpl.h @@ -28,6 +28,12 @@ #define MPL_strdup(s) strdup(s) #define MPL_free(p) free(p) +/* Types that headers under test only mention. The layout is a placeholder, + * NOT the real one, so units must not depend on their contents. */ +typedef struct { + int placeholder; +} MPL_pointer_attr_t; + /* src/mpl/src/mpl_rankmap.c */ int MPL_rankmap_str_to_array(char *mapping, int sz, int *out_nodemap); int MPL_rankmap_array_to_str(int *nodemap, int sz, char **out_mapping_str); From 97c0d452eacdb9088460e448ae593f0a34358282 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 02:05:09 +0000 Subject: [PATCH 09/12] build: add a make unit-testing target "make unit-testing" runs test/units/run_unit.py with the configured compiler. run_unit.py now builds each unit in its own subdirectory of test/units/out under the current directory, creating it as needed, so an out-of-tree build keeps the build products in the build tree. When started from test/units itself, it builds in test/units/out as before. Co-Authored-By: Claude Opus 5.5 --- Makefile.am | 4 ++++ test/units/run_unit.py | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4a4b5529f2e..c803efa1021 100644 --- a/Makefile.am +++ b/Makefile.am @@ -197,6 +197,10 @@ testconfig: test-clean: ( cd test && $(MAKE) $(AM_MAKEFLAGS) clean ) +# unit tests of internal routines, see doc/wiki/testing/Unit_Tests.md +unit-testing: + CC="$(CC)" python3 $(top_srcdir)/test/units/run_unit.py + ## FIXME: this should live in src/env/Makefile.mk. Moving it there ## will not cause a problem, but it will break the build if another ## Makefile.mk also decides to create a install-exec-hook target. A diff --git a/test/units/run_unit.py b/test/units/run_unit.py index 4a0aedefeb7..d756a38501b 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -16,6 +16,7 @@ class G: shims = [] unit_dir = None top_srcdir = None + outdir = None cc = [] cflags = [] timeout = 60 @@ -31,6 +32,13 @@ def match(pat, str, flags=0): def main(): G.unit_dir = os.path.abspath(sys.path[0]) G.top_srcdir = os.path.dirname(os.path.dirname(G.unit_dir)) + # build under test/units/out of the current directory, e.g. the build tree + # for "make unit-testing", or under out/ when started from test/units + cwd = os.path.realpath(os.getcwd()) + if cwd == os.path.realpath(G.unit_dir): + G.outdir = os.path.join(G.unit_dir, "out") + else: + G.outdir = os.path.join(cwd, "test", "units", "out") os.chdir(G.unit_dir) parser = argparse.ArgumentParser(description="Build and run the unit tests in test/units") @@ -42,15 +50,14 @@ def main(): opts = parser.parse_args() G.timeout = opts.timeout - if not os.path.exists("out"): - os.mkdir("out") + os.makedirs(G.outdir, exist_ok=True) G.cc = shlex.split(os.environ.get('CC', 'gcc')) G.cflags = ["-g", "-Wall"] if not opts.no_sanitize: - os.chdir("out") + os.chdir(G.outdir) G.cflags += probe_sanitizers() - os.chdir("..") + os.chdir(G.unit_dir) G.cflags += shlex.split(os.environ.get('UNIT_CFLAGS', '')) print("# %s" % ' '.join(G.cc + G.cflags)) @@ -59,6 +66,7 @@ def main(): else: args = load_test_units() for a in args: + a = re.sub(r'\.unit$', '', os.path.basename(a)) G.unit_name = a G.template_lines = [] G.tests = [] @@ -67,10 +75,13 @@ def main(): G.shims = [] load_unit(a + '.unit') - os.chdir("out") + # build and run each unit in its own directory + unit_out = os.path.join(G.outdir, a) + os.makedirs(unit_out, exist_ok=True) + os.chdir(unit_out) if build_unit(): run_tests() - os.chdir("..") + os.chdir(G.unit_dir) # TAP plan and summary print("1..%d" % G.num_tests) From ad150cc0cd9162d7f61804a1a5967f564ed934fa Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 01:03:41 +0000 Subject: [PATCH 10/12] doc/wiki: add unit test documentation Describe test/units in doc/wiki/testing/Unit_Tests.md: when to write a unit test, how to run it with make unit-testing or run_unit.py, the .unit format, shims, and writing a driver. Link it from Index.md and Writing_New_Tests.md. Co-Authored-By: Claude Opus 5.5 --- doc/wiki/Index.md | 1 + doc/wiki/testing/Unit_Tests.md | 171 ++++++++++++++++++++++++++ doc/wiki/testing/Writing_New_Tests.md | 4 + 3 files changed, 176 insertions(+) create mode 100644 doc/wiki/testing/Unit_Tests.md diff --git a/doc/wiki/Index.md b/doc/wiki/Index.md index 09782d87d6f..63d086e4cee 100644 --- a/doc/wiki/Index.md +++ b/doc/wiki/Index.md @@ -38,6 +38,7 @@ - [Nightly Tests](testing/Nightly_Tests.md) (Out Of Date) - [Jenkins](testing/Jenkins.md) - [Writing New Tests](testing/Writing_New_Tests.md) + - [Unit Tests](testing/Unit_Tests.md) - [Testing Crons](testing/Testing_Crons.md) - [Code Coverage Reports](testing/Code_Coverage_Reports.md) - [MPICH ABI Tests](testing/MPICH_ABI_Tests.md) diff --git a/doc/wiki/testing/Unit_Tests.md b/doc/wiki/testing/Unit_Tests.md new file mode 100644 index 00000000000..25096fc9a65 --- /dev/null +++ b/doc/wiki/testing/Unit_Tests.md @@ -0,0 +1,171 @@ +# Unit Tests + +The unit tests in `test/units/` exercise internal routines directly, without +building MPICH or starting MPI processes. They suit code that can be tested +from its inputs and outputs alone: string parsers and printers, map and +schedule generators, small algorithms. + +Use a [test/mpi test](Writing_New_Tests.md) instead when the behavior needs a +running MPI library, such as communication, progress, or anything that +depends on the device. + +## Running + +From a configured build tree: + +``` +make unit-testing +``` + +The script also runs directly, from a fresh git checkout before autogen or +configure: + +``` +python3 test/units/run_unit.py # all units +python3 test/units/run_unit.py mpl_rankmap # one unit +python3 test/units/run_unit.py --no-sanitize # build without sanitizers +``` + +| Option or variable | Meaning | +|---|---| +| `--timeout SEC` | Kill a test that runs longer than this (default 60). | +| `--no-sanitize` | Don't build with AddressSanitizer and UndefinedBehaviorSanitizer. | +| `CC` | Compiler (default `gcc`). `make unit-testing` passes the configured `$(CC)`. | +| `UNIT_CFLAGS` | Extra compiler flags, e.g. `-Werror`. | + +Each unit builds in its own subdirectory of `test/units/out` under the +current directory, which is created as needed: the source tree when run from +its top (the directory is in `.gitignore`), the build tree for +`make unit-testing`. Started from `test/units` itself, it uses +`test/units/out` too. + +`make unit-testing` needs `python3`, which autogen already requires. A build +from a release tarball does not otherwise need Python. + +Units build with `-g -Wall`, plus `-fsanitize=address,undefined` when a probe +program builds and runs with it. A sanitizer report fails the test. + +The output is [TAP](https://testanything.org/): one `ok` or `not ok` line per +unit build and per test, details as `#` lines, and the plan line `1..N` at +the end. The script exits nonzero if anything failed. + +``` +# gcc -g -Wall -fsanitize=address,undefined -fno-sanitize-recover=all +ok 1 - mpl_rankmap: build +ok 2 - mpl_rankmap: ./t str2array 4 '(vector,1,2,3,0)' +not ok 3 - mpl_rankmap: ./t str2array 12 '(vector,(0,3,4))' +# expect: 0 0 0 0 1 1 1 1 2 2 2 2 +# got: 0 0 0 0 1 1 1 1 2 2 2 3 +... +1..29 +# FAILED 1 of 29 +``` + +## Build independence + +Units must build and run from a fresh git checkout, with only `python3` and a +C compiler. A unit may not include headers generated by autogen or configure +(`mplconfig.h`, `mpichconf.h`, `coll_autogen.h`, ...). Shims (below) stand in +for the headers that would pull them in. Code that needs generated headers +is out of scope for unit tests. + +## Writing a unit + +A unit is a `test/units/.unit` file. The script builds a test program +`t` from it and runs the listed commands: + +``` +sources: + src/mpl/src/mpl_rankmap.c + +shim: + mpl + +template: + #include "mpl.h" + + int main(int argc, char **argv) + { + ...call the routine with the arguments, print the result... + } + +TESTS: + cmd: ./t str2array 4 '(vector,1,2,3,0)' + expect: 1 2 3 0 + + cmd: ./t str2array 4 'vector,1,2,3,0' + expect_err: err = 0x1 +``` + +Blocks start with a name and a colon in the first column; their content is +indented. + +| Block | Content | +|---|---| +| `sources:` | Source files, relative to the top of the tree, compiled unchanged and linked with the driver. | +| `include:` | Extra include directories, relative to the top of the tree, searched after the shims. | +| `shim:` | Shim directories under `test/units/shim/`, searched first. | +| `template:` | The driver, written to `t.c` with the leading four spaces removed. A line `[path:name]` pastes in the part of `path` between `/* START UNIT - name */` and `/* END UNIT */`. | +| `TESTS:` | The test cases. | + +Each test case starts with `cmd:`, a shell command run in the unit's build +directory. `$top_srcdir` in it is replaced with the top of the tree, so a +test can read files there, such as `./t -f $top_srcdir/path/to/input`. Then +at most one of: + +- `expect: TEXT`: the command must exit 0 and print exactly `TEXT`. For + output of several lines, leave the value empty and indent the lines deeper + than `expect:`. Whitespace at the start and end of the output, and at the + end of each line, is ignored. +- `expect_err: TEXT`: the command must exit with a positive code and print + `TEXT` somewhere in its standard output. A crash or a failed assertion + does not count. + +With neither, the command just has to exit 0. Lines starting with `#` are +comments. + +### Sources or markers + +Prefer `sources:`. It compiles the file as it is, so the test covers the real +code, including its static helpers, and nothing has to be copied by hand. +The whole file must compile against the shims, though. When a function lives +in a file that depends on too much else, mark it with `START UNIT`/`END UNIT` +comments and paste it into the template instead, along with whatever it +needs (see `slurm_list_to_nodes.unit`). + +### Shims + +A shim is a small header with the same name as a real one, such as `mpl.h` +or `mpiimpl.h`, that defines only what the units need. Because shim +directories come first on the include path, `#include "mpiimpl.h"` in the +source under test finds the shim. Any `.c` file in a shim directory is +compiled into the test program as well. + +| Shim | Stands in for | Provides | +|---|---|---| +| `mpl` | `src/mpl/include/mpl.h` | libc headers, the real `mpl_err.h` error codes, `MPL_malloc` and friends (ignoring the memory class), prototypes of the tested MPL routines | +| `core` | `src/include/mpiimpl.h` | everything in `mpl`, `MPI_SUCCESS` and some `MPI_ERR_` classes, `MPI_Aint`, opaque `MPIR_Comm`/`MPIR_Request`, the `MPIR_ERR_` macros, `MPIR_Assert` | + +The `core` versions of `MPIR_ERR_SETANDJUMP`, `MPIR_ERR_CHKANDJUMP` and their +numbered forms jump to `fn_fail` like the real ones, but instead of creating +an error code they format the message into `unit_last_error`, so a driver can +print it for `expect_err`. `MPIR_Assert` is always on and aborts. + +Add to a shim as a unit needs it. Keep the real names, signatures, and +values, and include real headers when they are self-contained (the `mpl` +shim uses the real `mpl_err.h`). A type that a header under test only +mentions may get a placeholder definition; mark it as such so no unit +relies on its layout. + +### Writing the driver + +- Take the input from the command line and print the result in a short, + canonical form, so `expect:` stays readable. +- On an error, print a line with the error (the return code, or + `unit_last_error` with the `core` shim) and return 1. +- Free what the routine returns. LeakSanitizer reports leaks in the driver + too. +- For properties that are hard to list case by case, let the driver check + them itself and print `ok`, e.g. that a printed map parses back to the + same map, or that a generated schedule is consistent for every process + count up to a bound. diff --git a/doc/wiki/testing/Writing_New_Tests.md b/doc/wiki/testing/Writing_New_Tests.md index e3952a6ec12..acc93808786 100644 --- a/doc/wiki/testing/Writing_New_Tests.md +++ b/doc/wiki/testing/Writing_New_Tests.md @@ -27,6 +27,10 @@ within this process, it must do the following: In addition, to make it easier to diagnose any failures, a good test will write detailed diagnostic information about any failure. +Internal routines that can be tested without MPI, such as parsers and +schedule generators, can instead get a unit test in `test/units/`; see +[Unit Tests](Unit_Tests.md). + To aid in writing tests, there are a set of utility routines in `test/util/mtest.c` (with corresponding versions for Fortran and C++). These routines include ones to print the required output, check the From 85aab0c03c4f38fc2660468af69e497abcdc0e85 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 02:05:48 +0000 Subject: [PATCH 11/12] test/units: add --junit output With --junit FILE, run_unit.py also writes the results as JUnit XML, a test suite per unit and a test case per TAP line, with the failure details and build warnings. CI can publish it like the test/mpi results. Co-Authored-By: Claude Opus 5.5 --- doc/wiki/testing/Unit_Tests.md | 1 + test/units/run_unit.py | 52 +++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/doc/wiki/testing/Unit_Tests.md b/doc/wiki/testing/Unit_Tests.md index 25096fc9a65..9cb84876f93 100644 --- a/doc/wiki/testing/Unit_Tests.md +++ b/doc/wiki/testing/Unit_Tests.md @@ -29,6 +29,7 @@ python3 test/units/run_unit.py --no-sanitize # build without sanitizers | Option or variable | Meaning | |---|---| | `--timeout SEC` | Kill a test that runs longer than this (default 60). | +| `--junit FILE` | Also write the results as JUnit XML, for CI result publishers: a test suite per unit, a test case per TAP line. | | `--no-sanitize` | Don't build with AddressSanitizer and UndefinedBehaviorSanitizer. | | `CC` | Compiler (default `gcc`). `make unit-testing` passes the configured `$(CC)`. | | `UNIT_CFLAGS` | Extra compiler flags, e.g. `-Werror`. | diff --git a/test/units/run_unit.py b/test/units/run_unit.py index d756a38501b..fdecfbba82f 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -5,6 +5,8 @@ import shlex import argparse import signal +import time +import xml.etree.ElementTree as ET class G: unit_name = None @@ -22,6 +24,8 @@ class G: timeout = 60 num_tests = 0 num_failed = 0 + # (unit, name, ok, diag, seconds) of every reported test, for --junit + results = [] class RE: m = None @@ -39,7 +43,6 @@ def main(): G.outdir = os.path.join(G.unit_dir, "out") else: G.outdir = os.path.join(cwd, "test", "units", "out") - os.chdir(G.unit_dir) parser = argparse.ArgumentParser(description="Build and run the unit tests in test/units") parser.add_argument("units", nargs="*", help="units to run (default: all *.unit files)") @@ -47,10 +50,13 @@ def main(): help="do not build with -fsanitize=address,undefined") parser.add_argument("--timeout", type=int, default=60, help="seconds before a test is killed (default: 60)") + parser.add_argument("--junit", metavar="FILE", help="also write the results as JUnit XML to FILE") opts = parser.parse_args() G.timeout = opts.timeout - + # resolve the --junit path before leaving the current directory + junit_file = os.path.abspath(opts.junit) if opts.junit else None os.makedirs(G.outdir, exist_ok=True) + os.chdir(G.unit_dir) G.cc = shlex.split(os.environ.get('CC', 'gcc')) G.cflags = ["-g", "-Wall"] @@ -83,6 +89,9 @@ def main(): run_tests() os.chdir(G.unit_dir) + if junit_file: + write_junit(junit_file) + # TAP plan and summary print("1..%d" % G.num_tests) if G.num_failed: @@ -168,7 +177,7 @@ def build_unit(): for a in G.shims: shim_dir = os.path.join(G.unit_dir, "shim", a) if not os.path.isdir(shim_dir): - report(False, "%s: build" % G.unit_name, "shim %s not found" % a) + report(False, "build", "shim %s not found" % a) return False cmd.append("-I" + shim_dir) srcs += sorted(os.path.join(shim_dir, f) for f in os.listdir(shim_dir) if f.endswith(".c")) @@ -178,14 +187,15 @@ def build_unit(): srcs.append(os.path.join(G.top_srcdir, a)) cmd += ["-o", "t"] + srcs + t0 = time.time() ret = subprocess.run(cmd, encoding='utf-8', errors='replace', stdout=subprocess.PIPE, stderr=subprocess.STDOUT) ok = ret.returncode == 0 and os.path.exists("t") if ok: # show warnings, if any - report(ok, "%s: build" % G.unit_name, ret.stdout) + report(ok, "build", ret.stdout, time.time() - t0) else: - report(ok, "%s: build" % G.unit_name, ' '.join(cmd) + "\n" + ret.stdout) + report(ok, "build", ' '.join(cmd) + "\n" + ret.stdout, time.time() - t0) return ok def probe_sanitizers(): @@ -204,7 +214,9 @@ def probe_sanitizers(): def run_tests(): for test in G.tests: + t0 = time.time() ret = run_cmd(test['cmd']) + seconds = time.time() - t0 # ignore trailing whitespace on each line output = '\n'.join(l.rstrip() for l in ret.stdout.strip().split('\n')) diag = "" @@ -229,7 +241,7 @@ def run_tests(): ok = True if not ok: diag += "got: %s\n%s" % (output, ret.stderr) - report(ok, "%s: %s" % (G.unit_name, test['cmd']), diag) + report(ok, test['cmd'], diag, seconds) def run_cmd(cmd): """Run a test command in its own process group; returncode is None if it timed out""" @@ -244,9 +256,11 @@ def run_cmd(cmd): stdout, stderr = proc.communicate() return subprocess.CompletedProcess(cmd, None, stdout, stderr) -def report(ok, desc, diag=""): +def report(ok, name, diag="", seconds=0): """Print one TAP test line, followed by diagnostics as '#' lines""" G.num_tests += 1 + G.results.append((G.unit_name, name, ok, diag, seconds)) + desc = "%s: %s" % (G.unit_name, name) if ok: print("ok %d - %s" % (G.num_tests, desc)) else: @@ -256,6 +270,30 @@ def report(ok, desc, diag=""): for line in diag.rstrip().split("\n"): print("# %s" % line) +def write_junit(file): + """Write the results as JUnit XML: a testsuite per unit, a testcase per TAP line""" + def xml_text(s): + # drop characters that XML 1.0 does not allow + return re.sub(r'[^\x09\x0a\x0d\x20-\ud7ff\ue000-\ufffd]', '?', s) + + root = ET.Element("testsuites", name="test/units", tests=str(G.num_tests), + failures=str(G.num_failed)) + suites = {} + for unit, name, ok, diag, seconds in G.results: + if unit not in suites: + suites[unit] = ET.SubElement(root, "testsuite", name=unit, tests="0", failures="0") + suite = suites[unit] + suite.set("tests", str(int(suite.get("tests")) + 1)) + case = ET.SubElement(suite, "testcase", classname="units." + unit, + name=xml_text(name), time="%.3f" % seconds) + if not ok: + suite.set("failures", str(int(suite.get("failures")) + 1)) + failure = ET.SubElement(case, "failure", message=xml_text(diag.strip().split("\n")[0])) + failure.text = xml_text(diag) + elif diag.strip(): + ET.SubElement(case, "system-out").text = xml_text(diag) + ET.ElementTree(root).write(file, encoding="utf-8", xml_declaration=True) + #---------------------------------------- if __name__ == "__main__": main() From 0061a60a2fdfbd0441a445e52fd5d6ad4dadbc63 Mon Sep 17 00:00:00 2001 From: hzhou-muse Date: Sat, 17 Oct 2026 03:33:06 +0000 Subject: [PATCH 12/12] test/units: run with Python 3.6 and gcc 4.8 The runner captures probe output with stdout/stderr pipes, which Python 3.6 supports, and adds -std=gnu99 when the compiler does not accept C99 code by default. Previously it failed on CentOS 7, whose Python 3.6 lacks capture_output and whose gcc 4.8 defaults to C89. Co-Authored-By: Claude Opus 5.5 --- test/units/run_unit.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/test/units/run_unit.py b/test/units/run_unit.py index fdecfbba82f..106a9fc6d7e 100644 --- a/test/units/run_unit.py +++ b/test/units/run_unit.py @@ -60,10 +60,14 @@ def main(): G.cc = shlex.split(os.environ.get('CC', 'gcc')) G.cflags = ["-g", "-Wall"] + os.chdir(G.outdir) + # the units are C99, which older compilers such as gcc 4.8 do not default to + c99 = "int main(void) { for (int i = 0; i < 1; i++) {} return 0; }\n" + if not probe([], c99) and probe(["-std=gnu99"], c99): + G.cflags.append("-std=gnu99") if not opts.no_sanitize: - os.chdir(G.outdir) G.cflags += probe_sanitizers() - os.chdir(G.unit_dir) + os.chdir(G.unit_dir) G.cflags += shlex.split(os.environ.get('UNIT_CFLAGS', '')) print("# %s" % ' '.join(G.cc + G.cflags)) @@ -198,15 +202,20 @@ def build_unit(): report(ok, "build", ' '.join(cmd) + "\n" + ret.stdout, time.time() - t0) return ok +def probe(flags, src="int main(void) { return 0; }\n"): + """Return True if a test program builds and runs with the extra flags""" + with open("probe.c", "w") as Out: + Out.write(src) + ret = subprocess.run(G.cc + G.cflags + flags + ["-o", "probe", "probe.c"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + if ret.returncode == 0: + ret = subprocess.run(["./probe"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + return ret.returncode == 0 + def probe_sanitizers(): """Return the sanitizer flags if a test program builds and runs with them""" flags = ["-fsanitize=address,undefined", "-fno-sanitize-recover=all"] - with open("probe.c", "w") as Out: - Out.write("int main(void) { return 0; }\n") - ret = subprocess.run(G.cc + flags + ["-o", "probe", "probe.c"], capture_output=True) - if ret.returncode == 0: - ret = subprocess.run(["./probe"], capture_output=True) - if ret.returncode == 0: + if probe(flags): return flags else: print("# sanitizers are not available, building without them")