[RFC PATCH v2 0/7] Introduce clar testing framework

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 18197 bytes --]

Hi,

this is the second version of my RFC patch series that introduces the
clar testing framework into our unit tests. The intent is to not have to
hand-craft all features of a proper unit testing framework, while still
not painting us into a corner. As such, the clar itself is small and
extensible while still bringing some nice features to the table.

Changes compared to v1:

  - Convert the ctypes unit tests to use clar, as I had it lying around
    anyway and René was asking for it.

  - Adopt the AWK-based script by René, replacing the Python-based one.
    This gets rid of the mandatory Python dependency and at the same
    time also allows us to be more flexible going forward.

  - Wire up the third party sources in our Makefiles such that they do
    not get linted via hdr-check or Coccinelle.

  - Fix t/Makefile to not pass GIT_TEST_OPTS to our unit tests. They
    don't know how to handle those options, and clar-based tests error
    out when they see unknown options.

  - Adapt Documentation/technical/unit-tests.txt to mention the clar.

  - Rebase the series onto the latest master at 406f326d27 (The second
    batch, 2024-08-01) to avoid some conflicts.

  - Cherry-pick a whitespace fix that otherwise makes git-apply(1)
    unhappy. The CI job is still broken because the first patch that
    imports clar continues to be broken. I've created an upstream PR.

  - Cherry-pick a fix by Randall to make clar work on HP NonStop. The
    fix has been proposed upstream in a PR.

With these changes, the CI jobs at GitLab all pass, except for the
whitespace and clang-format checks.

I've also Cc'd Ed, one of the maintainers of clar. I also noticed that
I'm a maintainer of that project back from my libgit2 times, so I think
it should be relatively easy to land changes upstream.

Thanks!

Patrick

Patrick Steinhardt (7):
  t: do not pass GIT_TEST_OPTS to unit tests with prove
  t: import the clar unit testing framework
  t/clar: fix whitespace errors
  t/clar: fix compatibility with NonStop
  Makefile: wire up the clar unit testing framework
  t/unit-tests: convert strvec tests to use clar
  t/unit-tests: convert ctype tests to use clar

 .gitignore                                 |   1 +
 Documentation/technical/unit-tests.txt     |   2 +
 Makefile                                   |  42 +-
 t/Makefile                                 |   4 +-
 t/run-test.sh                              |   2 +-
 t/unit-tests/.gitignore                    |   2 +
 t/unit-tests/clar-generate.awk             |  50 ++
 t/unit-tests/clar/.github/workflows/ci.yml |  23 +
 t/unit-tests/clar/COPYING                  |  15 +
 t/unit-tests/clar/README.md                | 329 ++++++++
 t/unit-tests/clar/clar.c                   | 842 +++++++++++++++++++++
 t/unit-tests/clar/clar.h                   | 173 +++++
 t/unit-tests/clar/clar/fixtures.h          |  50 ++
 t/unit-tests/clar/clar/fs.h                | 522 +++++++++++++
 t/unit-tests/clar/clar/print.h             | 211 ++++++
 t/unit-tests/clar/clar/sandbox.h           | 159 ++++
 t/unit-tests/clar/clar/summary.h           | 143 ++++
 t/unit-tests/clar/generate.py              | 266 +++++++
 t/unit-tests/clar/test/.gitignore          |   4 +
 t/unit-tests/clar/test/Makefile            |  39 +
 t/unit-tests/clar/test/clar_test.h         |  16 +
 t/unit-tests/clar/test/main.c              |  40 +
 t/unit-tests/clar/test/main.c.sample       |  27 +
 t/unit-tests/clar/test/resources/test/file |   1 +
 t/unit-tests/clar/test/sample.c            |  84 ++
 t/unit-tests/{t-ctype.c => ctype.c}        |  71 +-
 t/unit-tests/{t-strvec.c => strvec.c}      | 119 ++-
 t/unit-tests/unit-test.c                   |  17 +
 t/unit-tests/unit-test.h                   |   3 +
 29 files changed, 3159 insertions(+), 98 deletions(-)
 create mode 100644 t/unit-tests/clar-generate.awk
 create mode 100644 t/unit-tests/clar/.github/workflows/ci.yml
 create mode 100644 t/unit-tests/clar/COPYING
 create mode 100644 t/unit-tests/clar/README.md
 create mode 100644 t/unit-tests/clar/clar.c
 create mode 100644 t/unit-tests/clar/clar.h
 create mode 100644 t/unit-tests/clar/clar/fixtures.h
 create mode 100644 t/unit-tests/clar/clar/fs.h
 create mode 100644 t/unit-tests/clar/clar/print.h
 create mode 100644 t/unit-tests/clar/clar/sandbox.h
 create mode 100644 t/unit-tests/clar/clar/summary.h
 create mode 100755 t/unit-tests/clar/generate.py
 create mode 100644 t/unit-tests/clar/test/.gitignore
 create mode 100644 t/unit-tests/clar/test/Makefile
 create mode 100644 t/unit-tests/clar/test/clar_test.h
 create mode 100644 t/unit-tests/clar/test/main.c
 create mode 100644 t/unit-tests/clar/test/main.c.sample
 create mode 100644 t/unit-tests/clar/test/resources/test/file
 create mode 100644 t/unit-tests/clar/test/sample.c
 rename t/unit-tests/{t-ctype.c => ctype.c} (71%)
 rename t/unit-tests/{t-strvec.c => strvec.c} (54%)
 create mode 100644 t/unit-tests/unit-test.c
 create mode 100644 t/unit-tests/unit-test.h

Range-diff against v1:
-:  ---------- > 1:  78a9cc1162 t: do not pass GIT_TEST_OPTS to unit tests with prove
1:  4e3862991a ! 2:  6a88cf22a5 t: import the clar unit testing framework
    @@ Commit message
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    + ## Documentation/technical/unit-tests.txt ##
    +@@ Documentation/technical/unit-tests.txt: GitHub / GitLab stars to estimate this.
    + :criterion: https://github.com/Snaipe/Criterion[Criterion]
    + :c-tap: https://github.com/rra/c-tap-harness/[C TAP]
    + :check: https://libcheck.github.io/check/[Check]
    ++:clar: https://github.com/clar-test/clar[Clar]
    + 
    + [format="csv",options="header",width="33%",subs="specialcharacters,attributes,quotes,macros"]
    + |=====
    +@@ Documentation/technical/unit-tests.txt: Framework,"<<license,License>>","<<vendorable-or-ubiquitous,Vendorable or ubiqui
    + {criterion},{mit},{false},{partial},{true},{true},{true},{true},{true},{false},{true},19,1800
    + {c-tap},{expat},{true},{partial},{partial},{true},{false},{true},{false},{false},{false},4,33
    + {check},{lgpl},{false},{partial},{true},{true},{true},{false},{false},{false},{true},17,973
    ++{clar},{lgpl},{false},{partial},{true},{true},{true},{true},{false},{false},{true},1,192
    + |=====
    + 
    + === Additional framework candidates
    +
    + ## Makefile ##
    +@@ Makefile: THIRD_PARTY_SOURCES += compat/poll/%
    + THIRD_PARTY_SOURCES += compat/regex/%
    + THIRD_PARTY_SOURCES += sha1collisiondetection/%
    + THIRD_PARTY_SOURCES += sha1dc/%
    ++THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
    ++THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
    + 
    + UNIT_TEST_PROGRAMS += t-ctype
    + UNIT_TEST_PROGRAMS += t-example-decorate
    +@@ Makefile: $(SP_OBJ): %.sp: %.c %.o
    + .PHONY: sparse
    + sparse: $(SP_OBJ)
    + 
    +-EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%
    ++EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% $(UNIT_TEST_DIR)/clar/% $(UNIT_TEST_DIR)/clar/clar/%
    + ifndef OPENSSL_SHA1
    + 	EXCEPT_HDRS += sha1/openssl.h
    + endif
    +
      ## t/unit-tests/clar/.github/workflows/ci.yml (new) ##
     @@
     +name: CI
-:  ---------- > 3:  a52ee59bf4 t/clar: fix whitespace errors
-:  ---------- > 4:  02fb86dfbc t/clar: fix compatibility with NonStop
2:  7a5dfd5065 ! 5:  848dc673c4 Makefile: wire up the clar unit testing framework
    @@ .gitignore
      /bin-wrappers/
     
      ## Makefile ##
    -@@ Makefile: THIRD_PARTY_SOURCES += compat/regex/%
    - THIRD_PARTY_SOURCES += sha1collisiondetection/%
    - THIRD_PARTY_SOURCES += sha1dc/%
    +@@ Makefile: THIRD_PARTY_SOURCES += sha1dc/%
    + THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
    + THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
      
     +UNIT_TESTS_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
     +UNIT_TESTS_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TESTS_SUITES))
    @@ Makefile: endif
      
      bin-wrappers/%: wrap-for-bin.sh
      	$(call mkdir_p_parent_template)
    +@@ Makefile: $(SP_OBJ): %.sp: %.c %.o
    + .PHONY: sparse
    + sparse: $(SP_OBJ)
    + 
    +-EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% $(UNIT_TEST_DIR)/clar/% $(UNIT_TEST_DIR)/clar/clar/%
    ++EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% $(UNIT_TEST_DIR)/unit-test.h $(UNIT_TEST_DIR)/clar/% $(UNIT_TEST_DIR)/clar/clar/%
    + ifndef OPENSSL_SHA1
    + 	EXCEPT_HDRS += sha1/openssl.h
    + endif
     @@ Makefile: endif
      
      artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
    @@ Makefile: cocciclean:
      
      clean: profile-clean coverage-clean cocciclean
      	$(RM) -r .build $(UNIT_TEST_BIN)
    -+	$(RM) $(UNIT_TEST_DIR)/clar.suite $(UNIT_TEST_DIR)/.clarcache
    ++	$(RM) GIT-TEST-SUITES $(UNIT_TEST_DIR)/clar.suite $(UNIT_TEST_DIR)/clar-decls.h
      	$(RM) po/git.pot po/git-core.pot
      	$(RM) git.res
      	$(RM) $(OBJECTS)
    @@ Makefile: $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o \
     +	    if test x"$$FLAGS" != x"`cat GIT-TEST-SUITES 2>/dev/null`" ; then \
     +		echo >&2 "    * new test suites"; \
     +		echo "$$FLAGS" >GIT-TEST-SUITES; \
    -+		rm -f $(UNIT_TESTS_DIR)/.clarcache; \
     +            fi
     +
    -+$(UNIT_TEST_DIR)/clar.suite: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(UNIT_TESTS_SUITES)) GIT-TEST-SUITES
    -+	$(QUIET_GEN)$(UNIT_TEST_DIR)/clar/generate.py $(UNIT_TEST_DIR) >/dev/null
    -+	@touch $@
    -+$(UNIT_TEST_DIR)/clar-decls.h: $(UNIT_TEST_DIR)/clar.suite
    -+	$(QUIET_GEN)grep '^extern void' $^ >$@
    ++$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(UNIT_TESTS_SUITES)) GIT-TEST-SUITES
    ++	$(QUIET_GEN)for suite in $(UNIT_TESTS_SUITES); do \
    ++		sed -ne "s/^\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\)/extern \1;/p" $(UNIT_TEST_DIR)/$$suite.c; \
    ++	done >$@
    ++$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
    ++	$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
     +$(UNIT_TESTS_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
     +$(UNIT_TESTS_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR) -I$(UNIT_TEST_DIR)/clar
     +$(UNIT_TESTS_PROG): $(UNIT_TEST_DIR)/clar.suite $(UNIT_TESTS_OBJS) $(GITLIBS) GIT-LDFLAGS
    @@ t/Makefile: CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard cha
      ## t/unit-tests/.gitignore ##
     @@
      /bin
    -+/.clarcache
     +/clar.suite
     +/clar-decls.h
     
    + ## t/unit-tests/clar-generate.awk (new) ##
    +@@
    ++function add_suite(suite, initialize, cleanup, count) {
    ++       if (!suite) return
    ++       suite_count++
    ++       callback_count += count
    ++       suites = suites "    {\n"
    ++       suites = suites "        \"" suite "\",\n"
    ++       suites = suites "        " initialize ",\n"
    ++       suites = suites "        " cleanup ",\n"
    ++       suites = suites "        _clar_cb_" suite ", " count ", 1\n"
    ++       suites = suites "    },\n"
    ++}
    ++
    ++BEGIN {
    ++       suites = "static struct clar_suite _clar_suites[] = {\n"
    ++}
    ++
    ++{
    ++       print
    ++       name = $3; sub(/\(.*$/, "", name)
    ++       suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
    ++       short_name = name; sub(/^.*__/, "", short_name)
    ++       cb = "{ \"" short_name "\", &" name " }"
    ++       if (suite != prev_suite) {
    ++               add_suite(prev_suite, initialize, cleanup, count)
    ++               if (callbacks) callbacks = callbacks "};\n"
    ++               callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
    ++               initialize = "{ NULL, NULL }"
    ++               cleanup = "{ NULL, NULL }"
    ++               count = 0
    ++               prev_suite = suite
    ++       }
    ++       if (short_name == "initialize") {
    ++               initialize = cb
    ++       } else if (short_name == "cleanup") {
    ++               cleanup = cb
    ++       } else {
    ++               callbacks = callbacks "    " cb ",\n"
    ++               count++
    ++       }
    ++}
    ++
    ++END {
    ++       add_suite(suite, initialize, cleanup, count)
    ++       suites = suites "};"
    ++       if (callbacks) callbacks = callbacks "};"
    ++       print callbacks
    ++       print suites
    ++       print "static const size_t _clar_suite_count = " suite_count ";"
    ++       print "static const size_t _clar_callback_count = " callback_count ";"
    ++}
    +
      ## t/unit-tests/unit-test.c (new) ##
     @@
     +#include "unit-test.h"
     +
     +int cmd_main(int argc, const char **argv)
     +{
    -+	const char **args;
    ++	const char **argv_copy;
     +	int ret;
     +
     +	/* Append the "-t" flag such that the tests generate TAP output. */
    -+	DUP_ARRAY(args, argv, argc + 1);
    -+	args[argc++] = "-t";
    ++	ALLOC_ARRAY(argv_copy, argc + 2);
    ++	COPY_ARRAY(argv_copy, argv, argc);
    ++	argv_copy[argc++] = "-t";
    ++	argv_copy[argc] = NULL;
     +
    -+	ret = clar_test(argc, (char **) args);
    ++	ret = clar_test(argc, (char **) argv_copy);
     +
    -+	free(args);
    ++	free(argv_copy);
     +	return ret;
     +}
     
3:  1c2a510547 ! 6:  578e657269 t/unit-tests: convert strvec tests to use clar
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## Makefile ##
    -@@ Makefile: THIRD_PARTY_SOURCES += compat/regex/%
    - THIRD_PARTY_SOURCES += sha1collisiondetection/%
    - THIRD_PARTY_SOURCES += sha1dc/%
    +@@ Makefile: THIRD_PARTY_SOURCES += sha1dc/%
    + THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
    + THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
      
     +UNIT_TESTS_SUITES += strvec
      UNIT_TESTS_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
      UNIT_TESTS_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TESTS_SUITES))
      UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
    -@@ Makefile: UNIT_TEST_PROGRAMS += t-reftable-basics
    +@@ Makefile: UNIT_TEST_PROGRAMS += t-reftable-merged
      UNIT_TEST_PROGRAMS += t-reftable-record
      UNIT_TEST_PROGRAMS += t-strbuf
      UNIT_TEST_PROGRAMS += t-strcmp-offset
    @@ t/unit-tests/t-strvec.c => t/unit-tests/strvec.c
      #include "strvec.h"
      
      #define check_strvec(vec, ...) \
    --	check_strvec_loc(TEST_LOCATION(), vec, __VA_ARGS__)
    -+	check_strvec_loc(__FILE__, __func__, __LINE__, vec, __VA_ARGS__)
    - LAST_ARG_MUST_BE_NULL
    --static void check_strvec_loc(const char *loc, struct strvec *vec, ...)
    -+static void check_strvec_loc(const char *file, const char *func, size_t line, struct strvec *vec, ...)
    - {
    - 	va_list ap;
    - 	size_t nr = 0;
    -@@ t/unit-tests/strvec.c: static void check_strvec_loc(const char *loc, struct strvec *vec, ...)
    - 		if (!str)
    - 			break;
    - 
    --		if (!check_uint(vec->nr, >, nr) ||
    --		    !check_uint(vec->alloc, >, nr) ||
    --		    !check_str(vec->v[nr], str)) {
    --			struct strbuf msg = STRBUF_INIT;
    --			strbuf_addf(&msg, "strvec index %"PRIuMAX, (uintmax_t) nr);
    --			test_assert(loc, msg.buf, 0);
    --			strbuf_release(&msg);
    --			va_end(ap);
    --			return;
    --		}
    -+		clar__assert(vec->nr > nr, file, func, line, "Vector size too small", NULL, 1);
    -+		clar__assert(vec->alloc > nr, file, func, line, "Vector allocation too small", NULL, 1);
    -+		cl_assert_equal_s(vec->v[nr], str);
    - 
    - 		nr++;
    - 	}
    - 	va_end(ap);
    - 
    --	check_uint(vec->nr, ==, nr);
    --	check_uint(vec->alloc, >=, nr);
    --	check_pointer_eq(vec->v[nr], NULL);
    -+	cl_assert(vec->nr == nr);
    -+	cl_assert(vec->alloc >= nr);
    -+	cl_assert_equal_p(vec->v[nr], NULL);
    - }
    + 	do { \
    + 		const char *expect[] = { __VA_ARGS__ }; \
    +-		if (check_uint(ARRAY_SIZE(expect), >, 0) && \
    +-		    check_pointer_eq(expect[ARRAY_SIZE(expect) - 1], NULL) && \
    +-		    check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \
    +-		    check_uint((vec)->nr, <=, (vec)->alloc)) { \
    +-			for (size_t i = 0; i < ARRAY_SIZE(expect); i++) { \
    +-				if (!check_str((vec)->v[i], expect[i])) { \
    +-					test_msg("      i: %"PRIuMAX, \
    +-						 (uintmax_t)i); \
    +-					break; \
    +-				} \
    +-			} \
    +-		} \
    ++		cl_assert(ARRAY_SIZE(expect) > 0); \
    ++		cl_assert_equal_p(expect[ARRAY_SIZE(expect) - 1], NULL); \
    ++		cl_assert_equal_i((vec)->nr, ARRAY_SIZE(expect) - 1); \
    ++		cl_assert((vec)->nr <= (vec)->alloc); \
    ++		for (size_t i = 0; i < ARRAY_SIZE(expect); i++) \
    ++			cl_assert_equal_s((vec)->v[i], expect[i]); \
    + 	} while (0)
      
     -static void t_static_init(void)
     +void test_strvec__init(void)
    @@ t/unit-tests/strvec.c: static void t_push(void)
      }
      
     -static void t_pushf(void)
    -+void test_strvec__pushft_pushf(void)
    ++void test_strvec__pushf(void)
      {
      	struct strvec vec = STRVEC_INIT;
      	strvec_pushf(&vec, "foo: %d", 1);
    @@ t/unit-tests/strvec.c: static void t_detach(void)
     -	TEST(t_detach(), "detach");
     -	return test_done();
     -}
    +
    + ## t/unit-tests/unit-test.c ##
    +@@ t/unit-tests/unit-test.c: int cmd_main(int argc, const char **argv)
    + 	int ret;
    + 
    + 	/* Append the "-t" flag such that the tests generate TAP output. */
    +-	ALLOC_ARRAY(argv_copy, argc + 2);
    ++	ALLOC_ARRAY(argv_copy, argc + 1);
    + 	COPY_ARRAY(argv_copy, argv, argc);
    + 	argv_copy[argc++] = "-t";
    +-	argv_copy[argc] = NULL;
    + 
    + 	ret = clar_test(argc, (char **) argv_copy);
    + 
-:  ---------- > 7:  238de33b93 t/unit-tests: convert ctype tests to use clar
-- 
2.46.0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]