[v2,6/6] t-strbuf: use for_test

Commit Message

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

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.  The functions setup() and setup_populated() here
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 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.

Use the fully checking t_addstr() for adding initial values instead of
only doing only a length comparison -- there's no need for skipping the
other checks.

This results in test cases that look much more like strbuf usage in
production code, only with checked strbuf functions replaced by checking
wrappers.

Signed-off-by: René Scharfe <[e-mail]>
---
 t/unit-tests/t-strbuf.c | 122 ++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 62 deletions(-)

--
2.45.2

Comments

Phillip Wood July 23, 2024, 1:23 p.m. UTC | #1

Hi René

On 21/07/2024 07:26, René Scharfe wrote:

I'm still ambivalent about patches 3-6 but if we do this can we visually 
separate the initialization and cleanup from the main test body please. 
For example

> +	for_test ("strbuf_addch adds char") {
> +		struct strbuf sb = STRBUF_INIT;
+
> +		t_addch(&sb, 'a');
+
> +		t_release(&sb);
> +	}

Best Wishes

Phillip

Patch

diff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c
index 6027dafef7..82cae4cbe3 100644
--- a/t/unit-tests/t-strbuf.c
+++ b/t/unit-tests/t-strbuf.c
@@ -1,32 +1,6 @@ 
 #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);
-	strbuf_release(&buf);
-	check_uint(buf.len, ==, 0);
-	check_uint(buf.alloc, ==, 0);
-}
-
 static int assert_sane_strbuf(struct strbuf *buf)
 {
 	/* Initialized strbufs should always have a non-NULL buffer */
@@ -45,31 +19,8 @@  static int assert_sane_strbuf(struct strbuf *buf)
 	return check_uint(buf->len, <, buf->alloc);
 }

-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;
 	size_t orig_len = buf->len;

@@ -85,9 +36,8 @@  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)
+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;
@@ -105,18 +55,66 @@  static void t_addstr(struct strbuf *buf, const void *data)
 	check_str(buf->buf + orig_len, text);
 }

+static void t_release(struct strbuf *sb)
+{
+	strbuf_release(sb);
+	check_uint(sb->len, ==, 0);
+	check_uint(sb->alloc, ==, 0);
+}
+
 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"),
-	     "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"),
-	     "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);
+	}
+
+	for_test ("strbuf_addch adds char") {
+		struct strbuf sb = STRBUF_INIT;
+		t_addch(&sb, 'a');
+		t_release(&sb);
+	}
+
+	for_test ("strbuf_addch adds NUL char") {
+		struct strbuf sb = STRBUF_INIT;
+		t_addch(&sb, '\0');
+		t_release(&sb);
+	}
+
+	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);
+	}
+
+	for_test ("strbuf_addstr adds string") {
+		struct strbuf sb = STRBUF_INIT;
+		t_addstr(&sb, "hello there");
+		t_release(&sb);
+	}
+
+	for_test ("strbuf_addstr appends string to initial value") {
+		struct strbuf sb = STRBUF_INIT;
+		t_addstr(&sb, "initial value");
+		t_addstr(&sb, "hello there");
+		t_release(&sb);
+	}

 	return test_done();
 }