test/units: make the unit tests a usable framework

.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

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

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)

doc/wiki/testing/Unit_Tests.md

@@ -0,0 +1,172 @@
+# 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). |
+| `--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`. |
+
+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/<name>.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.

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

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 { \
@@ -273,7 +268,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 +307,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++;
         }
     }
@@ -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 */