On Thu, Jan 30, 2025 at 10:13:33AM +0100, Seyi Kuforiji wrote:
> Adapt strbuf test script to clar framework by using clar assertions
> where necessary. Test functions are created as standalone to test
> different test cases.
Same comment here regarding the second sentence.
> diff --git a/t/unit-tests/u-strbuf.c b/t/unit-tests/u-strbuf.c
> new file mode 100644
> index 0000000000..fec3c768d2
> --- /dev/null
> +++ b/t/unit-tests/u-strbuf.c
> @@ -0,0 +1,121 @@
> +#include "unit-test.h"
> +#include "strbuf.h"
It's somewhat sad that Git doesn't not think of this as a rename, as it
would've made the diff easier to read. You might want to try using
`git format-patch --find-renames=20' to ask Git to still consider this a
rename.
[snip]
> +static void assert_sane_strbuf(struct strbuf *buf)
> +{
> + /* Initialized strbufs should always have a non-NULL buffer */
> + cl_assert(buf->buf != NULL);
> + /* Buffers should always be NUL-terminated */
> + cl_assert(buf->buf[buf->len] == '\0');
> + /*
> + * Freshly-initialized strbufs may not have a dynamically allocated
> + * buffer
> + */
> + if (buf->len == 0 && buf->alloc == 0)
> + return;
> + /* alloc must be at least one byte larger than len */
> + cl_assert(buf->len < buf->alloc);
I think this condition can be simplified a bit:
/*
* 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);
> +void test_strbuf__add_empty_char(void)
> +{
> + setup(t_addch, "");
> +}
> +
> +void test_strbuf__add_multi_char(void)
Nit: I think "add_multi_char" is slightly misleading. What this test
exercises is that we can append to a non-empty buffer. So maybe
"append_char" would be a better fit?
> +{
> + setup_populated(t_addch, "initial value", "a");
> +}
> +
> +void test_strbuf__add_single_str(void)
> +{
> + setup(t_addstr, "hello there");
> +}
> +
> +void test_strbuf__add_multi_str(void)
Similarly, we could name this "append_str".
> +{
> + setup_populated(t_addstr, "initial value", "hello there");
> +}
Patrick
