[v2,0/6] unit-tests: add and use for_test to simplify tests

Message

René Scharfe July 21, 2024, 6:12 a.m. UTC

Changes since v1:
- replace "if (TEST_RUN(...))" with "for_test (...)", a macro based on the
  keyword "for" that doesn't require changes to existing functions,
- clarify the commit messages of patches 3 and 5,
- convert the strbuf initialization tests as well.

  t0080: move expected output to a file
  unit-tests: add for_test
  t-ctype: use for_test
  t-reftable-basics: use for_test
  t-strvec: use for_test
  t-strbuf: use for_test

 .clang-format                    |   2 +
 t/helper/test-example-tap.c      |  33 +++
 t/t0080-unit-test-output.sh      |  48 +----
 t/t0080/expect                   |  76 +++++++
 t/unit-tests/t-ctype.c           |   4 +-
 t/unit-tests/t-reftable-basics.c | 228 +++++++++-----------
 t/unit-tests/t-strbuf.c          | 122 ++++++-----
 t/unit-tests/t-strvec.c          | 356 ++++++++++++++-----------------
 t/unit-tests/test-lib.h          |  19 ++
 9 files changed, 454 insertions(+), 434 deletions(-)
 create mode 100644 t/t0080/expect

Range-Diff gegen v1:
1:  6efe5a37f0 = 1:  5faabaea54 t0080: move expected output to a file
2:  8297c2b121 < -:  ---------- unit-tests: add TEST_RUN
-:  ---------- > 2:  d4f9fa0938 unit-tests: add for_test
3:  ec5599906d ! 3:  a7cd5a2a3a t-ctype: use TEST_RUN
    @@ Metadata
     Author: René Scharfe <[e-mail]>

      ## Commit message ##
    -    t-ctype: use TEST_RUN
    +    t-ctype: use for_test

    -    Use the macro TEST_RUN instead of the internal functions
    -    test__run_begin() and test__run_end().
    +    Use the documented macro for_test instead of the internal functions
    +    test__run_begin() and test__run_end(), which are supposed to be private
    +    to the unit test framework.

      ## t/unit-tests/t-ctype.c ##
     @@
    @@ t/unit-tests/t-ctype.c
      		BUILD_ASSERT_OR_ZERO(sizeof(string[0]) == sizeof(char)); \
     -	int skip = test__run_begin(); \
     -	if (!skip) { \
    -+	if (TEST_RUN(#class " works")) { \
    ++	for_test (#class " works") { \
      		for (int i = 0; i < 256; i++) { \
      			if (!check_int(class(i), ==, !!memchr(string, i, len)))\
      				test_msg("      i: 0x%02x", i); \
4:  e589468f98 ! 4:  cc07910f88 t-reftable-basics: use TEST_RUN
    @@ Metadata
     Author: René Scharfe <[e-mail]>

      ## Commit message ##
    -    t-reftable-basics: use TEST_RUN
    +    t-reftable-basics: use for_test

         The macro TEST takes a single expression.  If a test requires multiple
         statements then they need to be placed in a function that's called in
         the TEST expression.

         Remove the overhead of defining and calling single-use functions by
    -    using TEST_RUN instead.
    +    using for_test instead.

         Run the tests in the order of definition.  We can reorder them like that
         because they are independent.  Technically this changes the output, but
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -		struct integer_needle_lesseq_args args = {
     -			.haystack = haystack,
     -			.needle = testcases[i].needle,
    -+	if (TEST_RUN("binary search with binsearch works")) {
    ++	for_test ("binary search with binsearch works") {
     +		int haystack[] = { 2, 4, 6, 8, 10 };
     +		struct {
     +			int needle;
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -	const char *a[] = { "a", "b", "c", NULL };
     -	const char *b[] = { "a", "b", "d", NULL };
     -	const char *c[] = { "a", "b", NULL };
    -+	if (TEST_RUN("names_length retuns size of a NULL-terminated string array")) {
    ++	for_test ("names_length retuns size of a NULL-terminated string array") {
     +		const char *a[] = { "a", "b", NULL };
     +		check_int(names_length(a), ==, 2);
     +	}
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -	check(!names_equal(a, b));
     -	check(!names_equal(a, c));
     -}
    -+	if (TEST_RUN("names_equal compares NULL-terminated string arrays")) {
    ++	for_test ("names_equal compares NULL-terminated string arrays") {
     +		const char *a[] = { "a", "b", "c", NULL };
     +		const char *b[] = { "a", "b", "d", NULL };
     +		const char *c[] = { "a", "b", NULL };
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -	check(!out[2]);
     -	free_names(out);
     -}
    -+	if (TEST_RUN("parse_names works for basic input")) {
    ++	for_test ("parse_names works for basic input") {
     +		char in1[] = "line\n";
     +		char in2[] = "a\nb\nc";
     +		char **out = NULL;
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -		check_int(common_prefix_size(&a, &b), ==, cases[i].want);
     -		strbuf_reset(&a);
     -		strbuf_reset(&b);
    -+	if (TEST_RUN("parse_names drops empty string")) {
    ++	for_test ("parse_names drops empty string") {
     +		char in[] = "a\n\nb\n";
     +		char **out = NULL;
     +		parse_names(in, strlen(in), &out);
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -	out = get_be24(dest);
     -	check_int(in, ==, out);
     -}
    -+	if (TEST_RUN("common_prefix_size works")) {
    ++	for_test ("common_prefix_size works") {
     +		struct strbuf a = STRBUF_INIT;
     +		struct strbuf b = STRBUF_INIT;
     +		struct {
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     -	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
     -	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
     -	TEST(test_u16_roundtrip(), "put_be16 and get_be16 work");
    -+	if (TEST_RUN("put_be24 and get_be24 work")) {
    ++	for_test ("put_be24 and get_be24 work") {
     +		uint32_t in = 0x112233;
     +		uint8_t dest[3];
     +		uint32_t out;
    @@ t/unit-tests/t-reftable-basics.c: static int integer_needle_lesseq(size_t i, voi
     +		check_int(in, ==, out);
     +	}
     +
    -+	if (TEST_RUN("put_be16 and get_be16 work")) {
    ++	for_test ("put_be16 and get_be16 work") {
     +		uint32_t in = 0xfef1;
     +		uint8_t dest[3];
     +		uint32_t out;
5:  5805a9cbd7 ! 5:  11c1675a13 t-strvec: use TEST_RUN
    @@ Metadata
     Author: René Scharfe <[e-mail]>

      ## Commit message ##
    -    t-strvec: use TEST_RUN
    +    t-strvec: use for_test

         The macro TEST takes a single expression.  If a test requires multiple
         statements then they need to be placed in a function that's called in
         the TEST expression.

    -    Remove the overhead of defining and calling single-use functions by
    -    using TEST_RUN instead.
    +    Remove the cognitive overhead of defining and calling single-use
    +    functions by using for_test instead.

      ## t/unit-tests/t-strvec.c ##
     @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct strvec *vec, ...)
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_uint(vec.nr, ==, 0);
     -	check_uint(vec.alloc, ==, 0);
     -}
    -+	if (TEST_RUN("static initialization")) {
    ++	for_test ("static initialization") {
     +		struct strvec vec = STRVEC_INIT;
     +		check_pointer_eq(vec.v, empty_strvec);
     +		check_uint(vec.nr, ==, 0);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_uint(vec.nr, ==, 0);
     -	check_uint(vec.alloc, ==, 0);
     -}
    -+	if (TEST_RUN("dynamic initialization")) {
    ++	for_test ("dynamic initialization") {
     +		struct strvec vec;
     +		strvec_init(&vec);
     +		check_pointer_eq(vec.v, empty_strvec);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_uint(vec.nr, ==, 0);
     -	check_uint(vec.alloc, ==, 0);
     -}
    -+	if (TEST_RUN("clear")) {
    ++	for_test ("clear") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_push(&vec, "foo");
     +		strvec_clear(&vec);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -static void t_push(void)
     -{
     -	struct strvec vec = STRVEC_INIT;
    -+	if (TEST_RUN("push")) {
    ++	for_test ("push") {
     +		struct strvec vec = STRVEC_INIT;

     -	strvec_push(&vec, "foo");
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo: 1", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("pushf")) {
    ++	for_test ("pushf") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushf(&vec, "foo: %d", 1);
     +		check_strvec(&vec, "foo: 1", NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("pushl")) {
    ++	for_test ("pushl") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		check_strvec(&vec, "foo", "bar", "baz", NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -		"foo", "bar", "baz", NULL,
     -	};
     -	struct strvec vec = STRVEC_INIT;
    -+	if (TEST_RUN("pushv")) {
    ++	for_test ("pushv") {
     +		const char *strings[] = {
     +			"foo", "bar", "baz", NULL,
     +		};
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "replaced", "bar", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("replace at head")) {
    ++	for_test ("replace at head") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_replace(&vec, 0, "replaced");
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", "replaced", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("replace at tail")) {
    ++	for_test ("replace at tail") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_replace(&vec, 2, "replaced");
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "replaced", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("replace in between")) {
    ++	for_test ("replace in between") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_replace(&vec, 1, "replaced");
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "oo", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("replace with substring")) {
    ++	for_test ("replace with substring") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", NULL);
     +		strvec_replace(&vec, 0, vec.v[0] + 1);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "bar", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("remove at head")) {
    ++	for_test ("remove at head") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_remove(&vec, 0);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("remove at tail")) {
    ++	for_test ("remove at tail") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_remove(&vec, 2);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("remove in between")) {
    ++	for_test ("remove in between") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_remove(&vec, 1);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("pop with empty array")) {
    ++	for_test ("pop with empty array") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pop(&vec);
     +		check_strvec(&vec, NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("pop with non-empty array")) {
    ++	for_test ("pop with non-empty array") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
     +		strvec_pop(&vec);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("split empty string")) {
    ++	for_test ("split empty string") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_split(&vec, "");
     +		check_strvec(&vec, NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("split single item")) {
    ++	for_test ("split single item") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_split(&vec, "foo");
     +		check_strvec(&vec, "foo", NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", "baz", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("split multiple items")) {
    ++	for_test ("split multiple items") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_split(&vec, "foo bar baz");
     +		check_strvec(&vec, "foo", "bar", "baz", NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("split whitespace only")) {
    ++	for_test ("split whitespace only") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_split(&vec, " \t\n");
     +		check_strvec(&vec, NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -	check_strvec(&vec, "foo", "bar", NULL);
     -	strvec_clear(&vec);
     -}
    -+	if (TEST_RUN("split multiple consecutive whitespaces")) {
    ++	for_test ("split multiple consecutive whitespaces") {
     +		struct strvec vec = STRVEC_INIT;
     +		strvec_split(&vec, "foo\n\t bar");
     +		check_strvec(&vec, "foo", "bar", NULL);
    @@ t/unit-tests/t-strvec.c: static void check_strvec_loc(const char *loc, struct st
     -{
     -	struct strvec vec = STRVEC_INIT;
     -	const char **detached;
    -+	if (TEST_RUN("detach")) {
    ++	for_test ("detach") {
     +		struct strvec vec = STRVEC_INIT;
     +		const char **detached;

6:  188b31884b ! 6:  cd79132f95 t-strbuf: use TEST_RUN
    @@ Metadata
     Author: René Scharfe <[e-mail]>

      ## Commit message ##
    -    t-strbuf: use TEST_RUN
    +    t-strbuf: use for_test

         The macro TEST takes a single expression.  If a test requires multiple
         statements then they need to be placed in a function that's called in
    @@ Commit message
         are used for that purpose and take another function as an argument,
         making the control flow hard to follow.

    -    Remove the overhead of these functions by using TEST_RUN instead.  Move
    +    Remove the overhead of these functions by using for_test instead.  Move
         their duplicate post-condition checks into a new helper, t_release(),
         and let t_addch() and t_addstr() accept properly typed input parameters
         instead of void pointers.
    @@ t/unit-tests/t-strbuf.c
      static int assert_sane_strbuf(struct strbuf *buf)
      {
      	/* Initialized strbufs should always have a non-NULL buffer */
    -@@ t/unit-tests/t-strbuf.c: static void t_dynamic_init(void)
    - 	strbuf_release(&buf);
    +@@ t/unit-tests/t-strbuf.c: static int assert_sane_strbuf(struct strbuf *buf)
    + 	return check_uint(buf->len, <, buf->alloc);
      }

    --static void t_addch(struct strbuf *buf, const void *data)
    +-static void t_static_init(void)
     +static void t_addch(struct strbuf *buf, int ch)
      {
    +-	struct strbuf buf = STRBUF_INIT;
    +-
    +-	check_uint(buf.len, ==, 0);
    +-	check_uint(buf.alloc, ==, 0);
    +-	check_char(buf.buf[0], ==, '\0');
    +-}
    +-
    +-static void t_dynamic_init(void)
    +-{
    +-	struct strbuf buf;
    +-
    +-	strbuf_init(&buf, 1024);
    +-	check(assert_sane_strbuf(&buf));
    +-	check_uint(buf.len, ==, 0);
    +-	check_uint(buf.alloc, >=, 1024);
    +-	check_char(buf.buf[0], ==, '\0');
    +-	strbuf_release(&buf);
    +-}
    +-
    +-static void t_addch(struct strbuf *buf, const void *data)
    +-{
     -	const char *p_ch = data;
     -	const char ch = *p_ch;
      	size_t orig_alloc = buf->alloc;
    @@ t/unit-tests/t-strbuf.c: static void t_addstr(struct strbuf *buf, const void *da
     +
      int cmd_main(int argc, const char **argv)
      {
    - 	if (!TEST(t_static_init(), "static initialization works"))
    - 		test_skip_all("STRBUF_INIT is broken");
    - 	TEST(t_dynamic_init(), "dynamic initialization works");
    +-	if (!TEST(t_static_init(), "static initialization works"))
    +-		test_skip_all("STRBUF_INIT is broken");
    +-	TEST(t_dynamic_init(), "dynamic initialization works");
     -	TEST(setup(t_addch, "a"), "strbuf_addch adds char");
     -	TEST(setup(t_addch, ""), "strbuf_addch adds NUL char");
     -	TEST(setup_populated(t_addch, "initial value", "a"),
    @@ t/unit-tests/t-strbuf.c: static void t_addstr(struct strbuf *buf, const void *da
     -	TEST(setup(t_addstr, "hello there"), "strbuf_addstr adds string");
     -	TEST(setup_populated(t_addstr, "initial value", "hello there"),
     -	     "strbuf_addstr appends string to initial value");
    ++	for_test ("static initialization works") {
    ++		struct strbuf buf = STRBUF_INIT;
    ++
    ++		if (!check_uint(buf.len, ==, 0) ||
    ++		    !check_uint(buf.alloc, ==, 0) ||
    ++		    !check_char(buf.buf[0], ==, '\0'))
    ++			test_skip_all("STRBUF_INIT is broken");
    ++	}
    ++
    ++	for_test ("dynamic initialization works") {
    ++		struct strbuf buf;
    ++
    ++		strbuf_init(&buf, 1024);
    ++		check(assert_sane_strbuf(&buf));
    ++		check_uint(buf.len, ==, 0);
    ++		check_uint(buf.alloc, >=, 1024);
    ++		check_char(buf.buf[0], ==, '\0');
    ++		strbuf_release(&buf);
    ++	}
     +
    -+	if (TEST_RUN("strbuf_addch adds char")) {
    ++	for_test ("strbuf_addch adds char") {
     +		struct strbuf sb = STRBUF_INIT;
     +		t_addch(&sb, 'a');
     +		t_release(&sb);
     +	}
     +
    -+	if (TEST_RUN("strbuf_addch adds NUL char")) {
    ++	for_test ("strbuf_addch adds NUL char") {
     +		struct strbuf sb = STRBUF_INIT;
     +		t_addch(&sb, '\0');
     +		t_release(&sb);
     +	}
     +
    -+	if (TEST_RUN("strbuf_addch appends to initial value")) {
    ++	for_test ("strbuf_addch appends to initial value") {
     +		struct strbuf sb = STRBUF_INIT;
     +		t_addstr(&sb, "initial value");
     +		t_addch(&sb, 'a');
     +		t_release(&sb);
     +	}
     +
    -+	if (TEST_RUN("strbuf_addstr adds string")) {
    ++	for_test ("strbuf_addstr adds string") {
     +		struct strbuf sb = STRBUF_INIT;
     +		t_addstr(&sb, "hello there");
     +		t_release(&sb);
     +	}
     +
    -+	if (TEST_RUN("strbuf_addstr appends string to initial value")) {
    ++	for_test ("strbuf_addstr appends string to initial value") {
     +		struct strbuf sb = STRBUF_INIT;
     +		t_addstr(&sb, "initial value");
     +		t_addstr(&sb, "hello there");
--
2.45.2