Re: [PATCH 6/6] t-strbuf: use TEST_RUN

[Phillip Wood <phillip.wood123@xxxxxxxxx>]

On 02/07/2024 16:14, phillip.wood123@xxxxxxxxx wrote:
> Getting rid of the untyped test arguments is 
> definitely a benefit of this approach.

That got me thinking how we might make type-safe setup()
functions. The diff below shows how we could define a macro to
generate the functions. DEFINE_SETUP_FN(char, ch) defines setup_ch()
that takes a test function and a char that is passed to the test with
the initialized strbuf. I'm not sure that's the way we want to go for
this test file but I thought I'd post it in case it is useful for
future tests.

Best Wishes

Phillip

---- >8 ----
diff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c
index 6027dafef70..8fc9a8b38df 100644
--- a/t/unit-tests/t-strbuf.c
+++ b/t/unit-tests/t-strbuf.c
@@ -1,27 +1,60 @@
 #include "test-lib.h"
 #include "strbuf.h"
 
-/* wrapper that supplies tests with an empty, initialized strbuf */
-static void setup(void (*f)(struct strbuf*, const void*),
-		  const void *data)
-{
-	struct strbuf buf = STRBUF_INIT;
-
-	f(&buf, data);
-	strbuf_release(&buf);
-	check_uint(buf.len, ==, 0);
-	check_uint(buf.alloc, ==, 0);
-}
-
-/* wrapper that supplies tests with a populated, initialized strbuf */
-static void setup_populated(void (*f)(struct strbuf*, const void*),
-			    const char *init_str, const void *data)
-{
-	struct strbuf buf = STRBUF_INIT;
-
-	strbuf_addstr(&buf, init_str);
-	check_uint(buf.len, ==, strlen(init_str));
-	f(&buf, data);
+/*
+ * Define a type safe wrapper function that supplies test functions
+ * with an initialized strbuf populated with an optional string and
+ * some data and then frees the strbuf when the test function
+ * returns. For example given the test function
+ *
+ *      t_foo(struct strbuf *buf, struct foo *data)
+ *
+ * the type safe wrapper function
+ *
+ *     setup_foo(void(*)(struct strbuf*, const struct foo*),
+ *     		 const char *init_str, const struct foo*)
+ *
+ * can be defined with
+ *
+ *     DEFINE_SETUP_FN(const struct foo*, foo)
+ *
+ * and used to run t_foo() with
+ *
+ *     TEST(setup_foo(t_foo, "initial string", &my_foo), "test foo");
+ */
+#define DEFINE_SETUP_FN(type, suffix) \
+	static void marshal_##suffix(void(*test_fn)(void),		     \
+				      struct strbuf *buf, const void *ctx)   \
+	{								     \
+		type data = *(type *)ctx;				     \
+		((void(*)(struct strbuf*, type)) test_fn)(buf, data);	     \
+	}								     \
+									     \
+	static void setup_##suffix(void(*test_fn)(struct strbuf*, type),     \
+				   const char *init_str, type data)	     \
+	{								     \
+		void *ctx = &data;					     \
+		do_setup(init_str, (void(*)(void)) test_fn, ctx,	     \
+			 marshal_##suffix);				     \
+	}
+
+/*
+ * Helper function for DEFINE_SETUP_FN() that initializes the strbuf,
+ * calls the test function and releases the strbuf
+ */
+static void do_setup(const char* init_str, void(*f)(void), const void *ctx,
+		     void(*marshal)(void(*)(void), struct strbuf*, const void*))
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	if (init_str) {
+		strbuf_addstr(&buf, init_str);
+		if (!check_uint(buf.len, ==, strlen(init_str))) {
+			strbuf_release(&buf);
+			return;
+		}
+	}
+	marshal(f, &buf, ctx);
 	strbuf_release(&buf);
 	check_uint(buf.len, ==, 0);
 	check_uint(buf.alloc, ==, 0);
@@ -66,10 +99,9 @@ static void t_dynamic_init(void)
 	strbuf_release(&buf);
 }
 
-static void t_addch(struct strbuf *buf, const void *data)
+DEFINE_SETUP_FN(char, ch)
+static void t_addch(struct strbuf *buf, char ch)
 {
-	const char *p_ch = data;
-	const char ch = *p_ch;
 	size_t orig_alloc = buf->alloc;
 	size_t orig_len = buf->len;
 
@@ -85,9 +117,9 @@ static void t_addch(struct strbuf *buf, const void *data)
 	check_char(buf->buf[buf->len], ==, '\0');
 }
 
-static void t_addstr(struct strbuf *buf, const void *data)
+DEFINE_SETUP_FN(const char*, str)
+static void t_addstr(struct strbuf *buf, const char *text)
 {
-	const char *text = data;
 	size_t len = strlen(text);
 	size_t orig_alloc = buf->alloc;
 	size_t orig_len = buf->len;
@@ -110,12 +142,12 @@ 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");
-	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"),
+	TEST(setup_ch(t_addch, NULL, 'a'), "strbuf_addch adds char");
+	TEST(setup_ch(t_addch, NULL, '\0'), "strbuf_addch adds NUL char");
+	TEST(setup_ch(t_addch, "initial value", 'a'),
 	     "strbuf_addch appends to initial value");
-	TEST(setup(t_addstr, "hello there"), "strbuf_addstr adds string");
-	TEST(setup_populated(t_addstr, "initial value", "hello there"),
+	TEST(setup_str(t_addstr, NULL, "hello there"), "strbuf_addstr adds string");
+	TEST(setup_str(t_addstr, "initial value", "hello there"),
 	     "strbuf_addstr appends string to initial value");
 
 	return test_done();