Merge branch 'rs/unit-tests-test-run'

Unit-test framework has learned a simple control structure to allow
embedding test statements in-line instead of having to create a new
function to contain them.

* rs/unit-tests-test-run:
  t-strvec: use if_test
  t-reftable-basics: use if_test
  t-ctype: use if_test
  unit-tests: add if_test
  unit-tests: show location of checks outside of tests
  t0080: use here-doc test body

.clang-format

@@ -196,6 +196,11 @@ ForEachMacros:
   - 'strmap_for_each_entry'
   - 'strset_for_each_entry'
 
+# A list of macros that should be interpreted as conditionals instead of as
+# function calls.
+IfMacros:
+  - 'if_test'
+
 # The maximum number of consecutive empty lines to keep.
 MaxEmptyLinesToKeep: 1
 

t/helper/test-example-tap.c

@@ -72,6 +72,8 @@ static void t_empty(void)
 
 int cmd__example_tap(int argc, const char **argv)
 {
+	check(1);
+
 	test_res = TEST(check_res = check_int(1, ==, 1), "passing test");
 	TEST(t_res(1), "passing test and assertion return 1");
 	test_res = TEST(check_res = check_int(1, ==, 2), "failing test");
@@ -92,5 +94,38 @@ int cmd__example_tap(int argc, const char **argv)
 	test_res = TEST(t_empty(), "test with no checks");
 	TEST(check_int(test_res, ==, 0), "test with no checks returns 0");
 
+	if_test ("if_test passing test")
+		check_int(1, ==, 1);
+	if_test ("if_test failing test")
+		check_int(1, ==, 2);
+	if_test ("if_test passing TEST_TODO()")
+		TEST_TODO(check(0));
+	if_test ("if_test failing TEST_TODO()")
+		TEST_TODO(check(1));
+	if_test ("if_test test_skip()") {
+		check(0);
+		test_skip("missing prerequisite");
+		check(1);
+	}
+	if_test ("if_test test_skip() inside TEST_TODO()")
+		TEST_TODO((test_skip("missing prerequisite"), 1));
+	if_test ("if_test TEST_TODO() after failing check") {
+		check(0);
+		TEST_TODO(check(0));
+	}
+	if_test ("if_test failing check after TEST_TODO()") {
+		check(1);
+		TEST_TODO(check(0));
+		check(0);
+	}
+	if_test ("if_test messages from failing string and char comparison") {
+		check_str("\thello\\", "there\"\n");
+		check_str("NULL", NULL);
+		check_char('a', ==, '\n');
+		check_char('\\', ==, '\'');
+	}
+	if_test ("if_test test with no checks")
+		; /* nothing */
+
 	return test_done();
 }

t/t0080-unit-test-output.sh

@@ -5,23 +5,24 @@ test_description='Test the output of the unit test framework'
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success 'TAP output from unit tests' '
+test_expect_success 'TAP output from unit tests' - <<\EOT
 	cat >expect <<-EOF &&
+	# BUG: check outside of test at t/helper/test-example-tap.c:75
 	ok 1 - passing test
 	ok 2 - passing test and assertion return 1
-	# check "1 == 2" failed at t/helper/test-example-tap.c:77
+	# check "1 == 2" failed at t/helper/test-example-tap.c:79
 	#    left: 1
 	#   right: 2
 	not ok 3 - failing test
 	ok 4 - failing test and assertion return 0
 	not ok 5 - passing TEST_TODO() # TODO
 	ok 6 - passing TEST_TODO() returns 1
-	# todo check ${SQ}check(x)${SQ} succeeded at t/helper/test-example-tap.c:26
+	# todo check 'check(x)' succeeded at t/helper/test-example-tap.c:26
 	not ok 7 - failing TEST_TODO()
 	ok 8 - failing TEST_TODO() returns 0
 	# check "0" failed at t/helper/test-example-tap.c:31
 	# skipping test - missing prerequisite
-	# skipping check ${SQ}1${SQ} at t/helper/test-example-tap.c:33
+	# skipping check '1' at t/helper/test-example-tap.c:33
 	ok 9 - test_skip() # SKIP
 	ok 10 - skipped test returns 1
 	# skipping test - missing prerequisite
@@ -39,21 +40,54 @@ test_expect_success 'TAP output from unit tests' '
 	# check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:63
 	#    left: "NULL"
 	#   right: NULL
-	# check "${SQ}a${SQ} == ${SQ}\n${SQ}" failed at t/helper/test-example-tap.c:64
-	#    left: ${SQ}a${SQ}
-	#   right: ${SQ}\012${SQ}
-	# check "${SQ}\\\\${SQ} == ${SQ}\\${SQ}${SQ}" failed at t/helper/test-example-tap.c:65
-	#    left: ${SQ}\\\\${SQ}
-	#   right: ${SQ}\\${SQ}${SQ}
+	# check "'a' == '\n'" failed at t/helper/test-example-tap.c:64
+	#    left: 'a'
+	#   right: '\012'
+	# check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:65
+	#    left: '\\\\'
+	#   right: '\\''
 	not ok 17 - messages from failing string and char comparison
-	# BUG: test has no checks at t/helper/test-example-tap.c:92
+	# BUG: test has no checks at t/helper/test-example-tap.c:94
 	not ok 18 - test with no checks
 	ok 19 - test with no checks returns 0
-	1..19
+	ok 20 - if_test passing test
+	# check "1 == 2" failed at t/helper/test-example-tap.c:100
+	#    left: 1
+	#   right: 2
+	not ok 21 - if_test failing test
+	not ok 22 - if_test passing TEST_TODO() # TODO
+	# todo check 'check(1)' succeeded at t/helper/test-example-tap.c:104
+	not ok 23 - if_test failing TEST_TODO()
+	# check "0" failed at t/helper/test-example-tap.c:106
+	# skipping test - missing prerequisite
+	# skipping check '1' at t/helper/test-example-tap.c:108
+	ok 24 - if_test test_skip() # SKIP
+	# skipping test - missing prerequisite
+	ok 25 - if_test test_skip() inside TEST_TODO() # SKIP
+	# check "0" failed at t/helper/test-example-tap.c:113
+	not ok 26 - if_test TEST_TODO() after failing check
+	# check "0" failed at t/helper/test-example-tap.c:119
+	not ok 27 - if_test failing check after TEST_TODO()
+	# check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:122
+	#    left: "\011hello\\\\"
+	#   right: "there\"\012"
+	# check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:123
+	#    left: "NULL"
+	#   right: NULL
+	# check "'a' == '\n'" failed at t/helper/test-example-tap.c:124
+	#    left: 'a'
+	#   right: '\012'
+	# check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:125
+	#    left: '\\\\'
+	#   right: '\\''
+	not ok 28 - if_test messages from failing string and char comparison
+	# BUG: test has no checks at t/helper/test-example-tap.c:127
+	not ok 29 - if_test test with no checks
+	1..29
 	EOF
 
 	! test-tool example-tap >actual &&
 	test_cmp expect actual
-'
+EOT
 
 test_done

t/unit-tests/t-ctype.c

@@ -4,15 +4,13 @@
 	size_t len = ARRAY_SIZE(string) - 1 + \
 		BUILD_ASSERT_OR_ZERO(ARRAY_SIZE(string) > 0) + \
 		BUILD_ASSERT_OR_ZERO(sizeof(string[0]) == sizeof(char)); \
-	int skip = test__run_begin(); \
-	if (!skip) { \
+	if_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); \
 		} \
 		check(!class(EOF)); \
 	} \
-	test__run_end(!skip, TEST_LOCATION(), #class " works"); \
 } while (0)
 
 #define DIGIT "0123456789"

t/unit-tests/t-reftable-basics.c

@@ -20,8 +20,9 @@ static int integer_needle_lesseq(size_t i, void *_args)
 	return args->needle <= args->haystack[i];
 }
 
-static void test_binsearch(void)
+int cmd_main(int argc, const char *argv[])
 {
+	if_test ("binary search with binsearch works") {
 		int haystack[] = { 2, 4, 6, 8, 10 };
 		struct {
 			int needle;
@@ -47,19 +48,18 @@ static void test_binsearch(void)
 			};
 			size_t idx;
 
-		idx = binsearch(ARRAY_SIZE(haystack), &integer_needle_lesseq, &args);
+			idx = binsearch(ARRAY_SIZE(haystack),
+					&integer_needle_lesseq, &args);
 			check_int(idx, ==, testcases[i].expected_idx);
 		}
-}
+	}
 
-static void test_names_length(void)
-{
+	if_test ("names_length retuns size of a NULL-terminated string array") {
 		const char *a[] = { "a", "b", NULL };
 		check_int(names_length(a), ==, 2);
-}
+	}
 
-static void test_names_equal(void)
-{
+	if_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 };
@@ -67,10 +67,9 @@ static void test_names_equal(void)
 		check(names_equal(a, a));
 		check(!names_equal(a, b));
 		check(!names_equal(a, c));
-}
+	}
 
-static void test_parse_names_normal(void)
-{
+	if_test ("parse_names works for basic input") {
 		char in1[] = "line\n";
 		char in2[] = "a\nb\nc";
 		char **out = NULL;
@@ -85,10 +84,9 @@ static void test_parse_names_normal(void)
 		check_str(out[2], "c");
 		check(!out[3]);
 		free_names(out);
-}
+	}
 
-static void test_parse_names_drop_empty(void)
-{
+	if_test ("parse_names drops empty string") {
 		char in[] = "a\n\nb\n";
 		char **out = NULL;
 		parse_names(in, strlen(in), &out);
@@ -97,10 +95,9 @@ static void test_parse_names_drop_empty(void)
 		check_str(out[1], "b");
 		check(!out[2]);
 		free_names(out);
-}
+	}
 
-static void test_common_prefix(void)
-{
+	if_test ("common_prefix_size works") {
 		struct strbuf a = STRBUF_INIT;
 		struct strbuf b = STRBUF_INIT;
 		struct {
@@ -123,38 +120,25 @@ static void test_common_prefix(void)
 		}
 		strbuf_release(&a);
 		strbuf_release(&b);
-}
+	}
 
-static void test_u24_roundtrip(void)
-{
+	if_test ("put_be24 and get_be24 work") {
 		uint32_t in = 0x112233;
 		uint8_t dest[3];
 		uint32_t out;
 		put_be24(dest, in);
 		out = get_be24(dest);
 		check_int(in, ==, out);
-}
+	}
 
-static void test_u16_roundtrip(void)
-{
+	if_test ("put_be16 and get_be16 work") {
 		uint32_t in = 0xfef1;
 		uint8_t dest[3];
 		uint32_t out;
 		put_be16(dest, in);
 		out = get_be16(dest);
 		check_int(in, ==, out);
-}
-
-int cmd_main(int argc, const char *argv[])
-{
-	TEST(test_common_prefix(), "common_prefix_size works");
-	TEST(test_parse_names_normal(), "parse_names works for basic input");
-	TEST(test_parse_names_drop_empty(), "parse_names drops empty string");
-	TEST(test_binsearch(), "binary search with binsearch works");
-	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
-	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");
+	}
 
 	return test_done();
 }

t/unit-tests/t-strvec.c

@@ -19,35 +19,33 @@
 		} \
 	} while (0)
 
-static void t_static_init(void)
+int cmd_main(int argc, const char **argv)
 {
+	if_test ("static initialization") {
 		struct strvec vec = STRVEC_INIT;
 		check_pointer_eq(vec.v, empty_strvec);
 		check_uint(vec.nr, ==, 0);
 		check_uint(vec.alloc, ==, 0);
-}
+	}
 
-static void t_dynamic_init(void)
-{
+	if_test ("dynamic initialization") {
 		struct strvec vec;
 		strvec_init(&vec);
 		check_pointer_eq(vec.v, empty_strvec);
 		check_uint(vec.nr, ==, 0);
 		check_uint(vec.alloc, ==, 0);
-}
+	}
 
-static void t_clear(void)
-{
+	if_test ("clear") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_push(&vec, "foo");
 		strvec_clear(&vec);
 		check_pointer_eq(vec.v, empty_strvec);
 		check_uint(vec.nr, ==, 0);
 		check_uint(vec.alloc, ==, 0);
-}
+	}
 
-static void t_push(void)
-{
+	if_test ("push") {
 		struct strvec vec = STRVEC_INIT;
 
 		strvec_push(&vec, "foo");
@@ -57,26 +55,23 @@ static void t_push(void)
 		check_strvec(&vec, "foo", "bar", NULL);
 
 		strvec_clear(&vec);
-}
+	}
 
-static void t_pushf(void)
-{
+	if_test ("pushf") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushf(&vec, "foo: %d", 1);
 		check_strvec(&vec, "foo: 1", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_pushl(void)
-{
+	if_test ("pushl") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		check_strvec(&vec, "foo", "bar", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_pushv(void)
-{
+	if_test ("pushv") {
 		const char *strings[] = {
 			"foo", "bar", "baz", NULL,
 		};
@@ -86,130 +81,115 @@ static void t_pushv(void)
 		check_strvec(&vec, "foo", "bar", "baz", NULL);
 
 		strvec_clear(&vec);
-}
+	}
 
-static void t_replace_at_head(void)
-{
+	if_test ("replace at head") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_replace(&vec, 0, "replaced");
 		check_strvec(&vec, "replaced", "bar", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_replace_at_tail(void)
-{
+	if_test ("replace at tail") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_replace(&vec, 2, "replaced");
 		check_strvec(&vec, "foo", "bar", "replaced", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_replace_in_between(void)
-{
+	if_test ("replace in between") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_replace(&vec, 1, "replaced");
 		check_strvec(&vec, "foo", "replaced", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_replace_with_substring(void)
-{
+	if_test ("replace with substring") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", NULL);
 		strvec_replace(&vec, 0, vec.v[0] + 1);
 		check_strvec(&vec, "oo", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_remove_at_head(void)
-{
+	if_test ("remove at head") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_remove(&vec, 0);
 		check_strvec(&vec, "bar", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_remove_at_tail(void)
-{
+	if_test ("remove at tail") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_remove(&vec, 2);
 		check_strvec(&vec, "foo", "bar", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_remove_in_between(void)
-{
+	if_test ("remove in between") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_remove(&vec, 1);
 		check_strvec(&vec, "foo", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_pop_empty_array(void)
-{
+	if_test ("pop with empty array") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pop(&vec);
 		check_strvec(&vec, NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_pop_non_empty_array(void)
-{
+	if_test ("pop with non-empty array") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_pushl(&vec, "foo", "bar", "baz", NULL);
 		strvec_pop(&vec);
 		check_strvec(&vec, "foo", "bar", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_split_empty_string(void)
-{
+	if_test ("split empty string") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_split(&vec, "");
 		check_strvec(&vec, NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_split_single_item(void)
-{
+	if_test ("split single item") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_split(&vec, "foo");
 		check_strvec(&vec, "foo", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_split_multiple_items(void)
-{
+	if_test ("split multiple items") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_split(&vec, "foo bar baz");
 		check_strvec(&vec, "foo", "bar", "baz", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_split_whitespace_only(void)
-{
+	if_test ("split whitespace only") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_split(&vec, " \t\n");
 		check_strvec(&vec, NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_split_multiple_consecutive_whitespaces(void)
-{
+	if_test ("split multiple consecutive whitespaces") {
 		struct strvec vec = STRVEC_INIT;
 		strvec_split(&vec, "foo\n\t bar");
 		check_strvec(&vec, "foo", "bar", NULL);
 		strvec_clear(&vec);
-}
+	}
 
-static void t_detach(void)
-{
+	if_test ("detach") {
 		struct strvec vec = STRVEC_INIT;
 		const char **detached;
 
@@ -225,31 +205,7 @@ static void t_detach(void)
 
 		free((char *) detached[0]);
 		free(detached);
-}
+	}
 
-int cmd_main(int argc, const char **argv)
-{
-	TEST(t_static_init(), "static initialization");
-	TEST(t_dynamic_init(), "dynamic initialization");
-	TEST(t_clear(), "clear");
-	TEST(t_push(), "push");
-	TEST(t_pushf(), "pushf");
-	TEST(t_pushl(), "pushl");
-	TEST(t_pushv(), "pushv");
-	TEST(t_replace_at_head(), "replace at head");
-	TEST(t_replace_in_between(), "replace in between");
-	TEST(t_replace_at_tail(), "replace at tail");
-	TEST(t_replace_with_substring(), "replace with substring");
-	TEST(t_remove_at_head(), "remove at head");
-	TEST(t_remove_in_between(), "remove in between");
-	TEST(t_remove_at_tail(), "remove at tail");
-	TEST(t_pop_empty_array(), "pop with empty array");
-	TEST(t_pop_non_empty_array(), "pop with non-empty array");
-	TEST(t_split_empty_string(), "split empty string");
-	TEST(t_split_single_item(), "split single item");
-	TEST(t_split_multiple_items(), "split multiple items");
-	TEST(t_split_whitespace_only(), "split whitespace only");
-	TEST(t_split_multiple_consecutive_whitespaces(), "split multiple consecutive whitespaces");
-	TEST(t_detach(), "detach");
 	return test_done();
 }

t/unit-tests/test-lib.c

@@ -16,6 +16,8 @@ static struct {
 	unsigned running :1;
 	unsigned skip_all :1;
 	unsigned todo :1;
+	char location[100];
+	char description[100];
 } ctx = {
 	.lazy_plan = 1,
 	.result = RESULT_NONE,
@@ -125,6 +127,8 @@ void test_plan(int count)
 
 int test_done(void)
 {
+	if (ctx.running && ctx.location[0] && ctx.description[0])
+		test__run_end(1, ctx.location, "%s", ctx.description);
 	assert(!ctx.running);
 
 	if (ctx.lazy_plan)
@@ -167,13 +171,38 @@ void test_skip_all(const char *format, ...)
 	va_end(ap);
 }
 
+void test__run_describe(const char *location, const char *format, ...)
+{
+	va_list ap;
+	int len;
+
+	assert(ctx.running);
+	assert(!ctx.location[0]);
+	assert(!ctx.description[0]);
+
+	xsnprintf(ctx.location, sizeof(ctx.location), "%s",
+		  make_relative(location));
+
+	va_start(ap, format);
+	len = vsnprintf(ctx.description, sizeof(ctx.description), format, ap);
+	va_end(ap);
+	if (len < 0)
+		die("unable to format message: %s", format);
+	if (len >= sizeof(ctx.description))
+		BUG("ctx.description too small to format %s", format);
+}
+
 int test__run_begin(void)
 {
+	if (ctx.running && ctx.location[0] && ctx.description[0])
+		test__run_end(1, ctx.location, "%s", ctx.description);
 	assert(!ctx.running);
 
 	ctx.count++;
 	ctx.result = RESULT_NONE;
 	ctx.running = 1;
+	ctx.location[0] = '\0';
+	ctx.description[0] = '\0';
 
 	return ctx.skip_all;
 }
@@ -264,7 +293,12 @@ static void test_todo(void)
 
 int test_assert(const char *location, const char *check, int ok)
 {
-	assert(ctx.running);
+	if (!ctx.running) {
+		test_msg("BUG: check outside of test at %s",
+			 make_relative(location));
+		ctx.failed = 1;
+		return 0;
+	}
 
 	if (ctx.result == RESULT_SKIP) {
 		test_msg("skipping check '%s' at %s", check,

t/unit-tests/test-lib.h

@@ -14,6 +14,23 @@
 	test__run_end(test__run_begin() ? 0 : (t, 1),	\
 		      TEST_LOCATION(),  __VA_ARGS__)
 
+/*
+ * Run a test unless test_skip_all() has been called.  Acts like a
+ * conditional; the test body is expected as a statement or block after
+ * the closing parenthesis.  The description for each test should be
+ * unique.  E.g.:
+ *
+ *  if_test ("something else %d %d", arg1, arg2) {
+ *          prepare();
+ *          test_something_else(arg1, arg2);
+ *          cleanup();
+ *  }
+ */
+#define if_test(...)							\
+	if (test__run_begin() ?						\
+	    (test__run_end(0, TEST_LOCATION(),  __VA_ARGS__), 0) :	\
+	    (test__run_describe(TEST_LOCATION(),  __VA_ARGS__), 1))
+
 /*
  * Print a test plan, should be called before any tests. If the number
  * of tests is not known in advance test_done() will automatically
@@ -154,6 +171,9 @@ union test__tmp {
 
 extern union test__tmp test__tmp[2];
 
+__attribute__((format (printf, 2, 3)))
+void test__run_describe(const char *, const char *, ...);
+
 int test__run_begin(void);
 __attribute__((format (printf, 3, 4)))
 int test__run_end(int, const char *, const char *, ...);