Hi Seyi
On 31/01/2025 22:14, Seyi Kuforiji wrote:
Adapt strbuf test script to clar framework by using clar assertions
where necessary.
This patch looks correct but it looses the nice messages we get from
check_char() and check_uint(a, </<=/>/>=, b). I think it would be worth
adding equivalent assertions to clar to avoid that.
/* wrapper that supplies tests with an empty, initialized strbuf */
@@ -9,8 +9,8 @@ static void setup(void (*f)(struct strbuf*, const void*),
f(&buf, data);
strbuf_release(&buf);
- check_uint(buf.len, ==, 0);
- check_uint(buf.alloc, ==, 0);
+ cl_assert_equal_i(buf.len, 0);
+ cl_assert_equal_i(buf.alloc, 0);
It is a shame that we do not have an assertion for checking two unsigned
integers are equal. In this case it probably does not matter but large
unsigned values will be printed as negative numbers which could be
rather confusing for someone trying to debug a test failure.
-static int assert_sane_strbuf(struct strbuf *buf)
+static void assert_sane_strbuf(struct strbuf *buf)
{
/* Initialized strbufs should always have a non-NULL buffer */
- if (!check(!!buf->buf))
- return 0;
+ cl_assert(buf->buf != NULL);
/* Buffers should always be NUL-terminated */
- if (!check_char(buf->buf[buf->len], ==, '\0'))
- return 0;
+ cl_assert(buf->buf[buf->len] == '\0');
It is unfortunate that this looses the helpful diagnostic message that
shows the values of the two chars being compared that is printed by
check_char() when the check fails. I think it would be worth porting
check_char() arcoss to clar. When we decided to move to the new test
framework we planned to add new assertions to clar to match our existing
framework.
/*
- * Freshly-initialized strbufs may not have a dynamically allocated
- * buffer
- */
- if (buf->len == 0 && buf->alloc == 0)
- return 1;
- /* alloc must be at least one byte larger than len */
- return check_uint(buf->len, <, buf->alloc);
+ * In case the buffer contains anything, `alloc` must alloc must
+ * be at least one byte larger than `len`.
+ */
+ if (buf->len)
+ cl_assert(buf->len < buf->alloc);
This is another case where we loose a helpful diagnostic message because
clar lacks cl_assert_lt_u(a, b) to check that unsigned integer a is less
than unsigned integer b. I think it would be worth adding the assertions
that are missing show that we have the equivalent of
check_int(a, <, b)
check_int(a, <=, b)
check_int(a, >, b)
check_int(a, >=, b)
and their unsigned companions so we do not regress the diagnostic output
when a test fails.
Looking at the comment that is deleted above I wonder if we should add
else
cl_assert_equal_i(buf->alloc, 0);
To check that alloc is zero when len is zero.
Best Wishes
Phillip
}
-static void t_static_init(void)
+void test_strbuf__static_init(void)
{
struct strbuf buf = STRBUF_INIT;
- check_uint(buf.len, ==, 0);
- check_uint(buf.alloc, ==, 0);
- check_char(buf.buf[0], ==, '\0');
+ cl_assert_equal_i(buf.len, 0);
+ cl_assert_equal_i(buf.alloc, 0);
+ cl_assert(buf.buf[0] == '\0');
}
-static void t_dynamic_init(void)
+void test_strbuf__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');
+ assert_sane_strbuf(&buf);
+ cl_assert_equal_i(buf.len, 0);
+ cl_assert(buf.alloc >= 1024);
+ cl_assert(buf.buf[0] == '\0');
strbuf_release(&buf);
}
@@ -73,16 +69,12 @@ static void t_addch(struct strbuf *buf, const void *data)
size_t orig_alloc = buf->alloc;
size_t orig_len = buf->len;
- if (!check(assert_sane_strbuf(buf)))
- return;
+ assert_sane_strbuf(buf);
strbuf_addch(buf, ch);
- if (!check(assert_sane_strbuf(buf)))
- return;
- if (!(check_uint(buf->len, ==, orig_len + 1) &&
- check_uint(buf->alloc, >=, orig_alloc)))
- return; /* avoid de-referencing buf->buf */
- check_char(buf->buf[buf->len - 1], ==, ch);
- check_char(buf->buf[buf->len], ==, '\0');
+ assert_sane_strbuf(buf);
+ cl_assert_equal_i(buf->len, orig_len + 1);
+ cl_assert(buf->alloc >= orig_alloc);
+ cl_assert(buf->buf[buf->len] == '\0');
}
static void t_addstr(struct strbuf *buf, const void *data)
@@ -92,31 +84,36 @@ static void t_addstr(struct strbuf *buf, const void *data)
size_t orig_alloc = buf->alloc;
size_t orig_len = buf->len;
- if (!check(assert_sane_strbuf(buf)))
- return;
+ assert_sane_strbuf(buf);
strbuf_addstr(buf, text);
- if (!check(assert_sane_strbuf(buf)))
- return;
- if (!(check_uint(buf->len, ==, orig_len + len) &&
- check_uint(buf->alloc, >=, orig_alloc) &&
- check_uint(buf->alloc, >, orig_len + len) &&
- check_char(buf->buf[orig_len + len], ==, '\0')))
- return;
- check_str(buf->buf + orig_len, text);
+ assert_sane_strbuf(buf);
+ cl_assert_equal_i(buf->len, orig_len + len);
+ cl_assert(buf->alloc >= orig_alloc);
+ cl_assert(buf->buf[buf->len] == '\0');
+ cl_assert_equal_s(buf->buf + orig_len, text);
}
-int cmd_main(int argc UNUSED, const char **argv UNUSED)
+void test_strbuf__add_single_char(void)
{
- 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");
-
- return test_done();
+ setup(t_addch, "a");
+}
+
+void test_strbuf__add_empty_char(void)
+{
+ setup(t_addch, "");
+}
+
+void test_strbuf__add_append_char(void)
+{
+ setup_populated(t_addch, "initial value", "a");
+}
+
+void test_strbuf__add_single_str(void)
+{
+ setup(t_addstr, "hello there");
+}
+
+void test_strbuf__add_append_str(void)
+{
+ setup_populated(t_addstr, "initial value", "hello there");
}
