From: Phillip Wood Date: Wed, 28 Aug 2024 13:17:05 GMT Subject: Re: [PATCH v6 11/13] t/unit-tests: convert strvec tests to use clar Message-ID: In-Reply-To: Hi Patrick On 20/08/2024 15:02, Patrick Steinhardt wrote: > Convert the strvec tests to use the new clar unit testing framework. > This is a first test balloon that demonstrates how the testing infra for > clar-based tests looks like. > > The tests are part of the "t/unit-tests/bin/unit-tests" binary. When > running that binary, it generates TAP output: It would be interesting to see a comparison between the current framework and clar of the output from a failing test - the TAP output for passing tests is pretty much the same regardless of the framework used. > # ./t/unit-tests/bin/unit-tests > TAP version 13 > # start of suite 1: strvec > ok 1 - strvec::init > [...] > The binary also supports some parameters that allow us to run only a > subset of unit tests or alter the output: > > $ ./t/unit-tests/bin/unit-tests -h > Usage: ./t/unit-tests/bin/unit-tests [options] > > Options: > -sname Run only the suite with `name` (can go to individual test name) > -iname Include the suite with `name` > -xname Exclude the suite with `name` > -v Increase verbosity (show suite names) The output above seems to include the suite name - are we running the tests with '-v' from our Makefile? > -q Only report tests that had an error This option is incompatible with TAP output. As we force TAP output we should find a way to stop displaying this help. > -Q Quit as soon as a test fails > -t Display results in tap format We force TAP output by adding '-t' to argv in main() so this line is not very helpful > -l Print suite names > -r[filename] Write summary file (to the optional filename) > diff --git a/t/unit-tests/strvec.c b/t/unit-tests/strvec.c > [..] > +#define check_strvec(vec, ...) \ > + do { \ > + const char *expect[] = { __VA_ARGS__ }; \ > + cl_assert(ARRAY_SIZE(expect) > 0); \ As there are a lot occurrences of ARRAY_SIZE(expect) it is probably worth adding size_t expect_len = ARRAY_SIZE(expect); above. > + cl_assert_equal_p(expect[ARRAY_SIZE(expect) - 1], NULL); \ > + cl_assert_equal_i((vec)->nr, ARRAY_SIZE(expect) - 1); \ > + cl_assert((vec)->nr <= (vec)->alloc); \ The conversion here loses the values of nr and alloc which is a shame as they would be useful when debugging a test failure. > + for (size_t i = 0; i < ARRAY_SIZE(expect); i++) \ > + cl_assert_equal_s((vec)->v[i], expect[i]); \ The original test also printed the array index of the failing check. As the elements of the test vectors all seem to be unique that is less of a worry than if we had tests with repeating elements. > + } while (0) > + > +void test_strvec__init(void) > +{ > + struct strvec vec = STRVEC_INIT; If we're rewriting the tests perhaps we can take the opportunity to add a blank line to each one after the variable declarations in accordance with our coding guidelines. It might be a good opportunity to show the set-up and tear-down facilities in clar as well instead of repeating the initialization in each test. Best Wishes Phillip > + cl_assert_equal_p(vec.v, empty_strvec); > + cl_assert_equal_i(vec.nr, 0); > + cl_assert_equal_i(vec.alloc, 0); > +} > + > +void test_strvec__dynamic_init(void) > +{ > + struct strvec vec; > + strvec_init(&vec); > + cl_assert_equal_p(vec.v, empty_strvec); > + cl_assert_equal_i(vec.nr, 0); > + cl_assert_equal_i(vec.alloc, 0); > +} > + > +void test_strvec__clear(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_push(&vec, "foo"); > + strvec_clear(&vec); > + cl_assert_equal_p(vec.v, empty_strvec); > + cl_assert_equal_i(vec.nr, 0); > + cl_assert_equal_i(vec.alloc, 0); > +} > + > +void test_strvec__push(void) > +{ > + struct strvec vec = STRVEC_INIT; > + > + strvec_push(&vec, "foo"); > + check_strvec(&vec, "foo", NULL); > + > + strvec_push(&vec, "bar"); > + check_strvec(&vec, "foo", "bar", NULL); > + > + strvec_clear(&vec); > +} > + > +void test_strvec__pushf(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_pushf(&vec, "foo: %d", 1); > + check_strvec(&vec, "foo: 1", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__pushl(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_pushl(&vec, "foo", "bar", "baz", NULL); > + check_strvec(&vec, "foo", "bar", "baz", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__pushv(void) > +{ > + const char *strings[] = { > + "foo", "bar", "baz", NULL, > + }; > + struct strvec vec = STRVEC_INIT; > + > + strvec_pushv(&vec, strings); > + check_strvec(&vec, "foo", "bar", "baz", NULL); > + > + strvec_clear(&vec); > +} > + > +void test_strvec__replace_at_head(void) > +{ > + 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); > +} > + > +void test_strvec__replace_at_tail(void) > +{ > + 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); > +} > + > +void test_strvec__replace_in_between(void) > +{ > + 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); > +} > + > +void test_strvec__replace_with_substring(void) > +{ > + 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); > +} > + > +void test_strvec__remove_at_head(void) > +{ > + 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); > +} > + > +void test_strvec__remove_at_tail(void) > +{ > + 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); > +} > + > +void test_strvec__remove_in_between(void) > +{ > + 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); > +} > + > +void test_strvec__pop_empty_array(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_pop(&vec); > + check_strvec(&vec, NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__pop_non_empty_array(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_pushl(&vec, "foo", "bar", "baz", NULL); > + strvec_pop(&vec); > + check_strvec(&vec, "foo", "bar", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__split_empty_string(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_split(&vec, ""); > + check_strvec(&vec, NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__split_single_item(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_split(&vec, "foo"); > + check_strvec(&vec, "foo", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__split_multiple_items(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_split(&vec, "foo bar baz"); > + check_strvec(&vec, "foo", "bar", "baz", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__split_whitespace_only(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_split(&vec, " \t\n"); > + check_strvec(&vec, NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__split_multiple_consecutive_whitespaces(void) > +{ > + struct strvec vec = STRVEC_INIT; > + strvec_split(&vec, "foo\n\t bar"); > + check_strvec(&vec, "foo", "bar", NULL); > + strvec_clear(&vec); > +} > + > +void test_strvec__detach(void) > +{ > + struct strvec vec = STRVEC_INIT; > + const char **detached; > + > + strvec_push(&vec, "foo"); > + > + detached = strvec_detach(&vec); > + cl_assert_equal_s(detached[0], "foo"); > + cl_assert_equal_p(detached[1], NULL); > + > + cl_assert_equal_p(vec.v, empty_strvec); > + cl_assert_equal_i(vec.nr, 0); > + cl_assert_equal_i(vec.alloc, 0); > + > + free((char *) detached[0]); > + free(detached); > +} > diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c > deleted file mode 100644 > index c4bac8fc91b..00000000000 > --- a/t/unit-tests/t-strvec.c > +++ /dev/null > @@ -1,211 +0,0 @@ > -#include "test-lib.h" > -#include "strbuf.h" > -#include "strvec.h" > - > -#define check_strvec(vec, ...) \ > - do { \ > - const char *expect[] = { __VA_ARGS__ }; \ > - if (check_uint(ARRAY_SIZE(expect), >, 0) && \ > - check_pointer_eq(expect[ARRAY_SIZE(expect) - 1], NULL) && \ > - check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \ > - check_uint((vec)->nr, <=, (vec)->alloc)) { \ > - for (size_t i = 0; i < ARRAY_SIZE(expect); i++) { \ > - if (!check_str((vec)->v[i], expect[i])) { \ > - test_msg(" i: %"PRIuMAX, \ > - (uintmax_t)i); \ > - break; \ > - } \ > - } \ > - } \ > - } while (0) > - > -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); > - } > - > - 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); > - } > - > - 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); > - } > - > - if_test ("push") { > - struct strvec vec = STRVEC_INIT; > - > - strvec_push(&vec, "foo"); > - check_strvec(&vec, "foo", NULL); > - > - strvec_push(&vec, "bar"); > - check_strvec(&vec, "foo", "bar", NULL); > - > - strvec_clear(&vec); > - } > - > - if_test ("pushf") { > - struct strvec vec = STRVEC_INIT; > - strvec_pushf(&vec, "foo: %d", 1); > - check_strvec(&vec, "foo: 1", NULL); > - strvec_clear(&vec); > - } > - > - 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); > - } > - > - if_test ("pushv") { > - const char *strings[] = { > - "foo", "bar", "baz", NULL, > - }; > - struct strvec vec = STRVEC_INIT; > - > - strvec_pushv(&vec, strings); > - check_strvec(&vec, "foo", "bar", "baz", NULL); > - > - strvec_clear(&vec); > - } > - > - 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); > - } > - > - 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); > - } > - > - 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); > - } > - > - 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); > - } > - > - 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); > - } > - > - 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); > - } > - > - 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); > - } > - > - if_test ("pop with empty array") { > - struct strvec vec = STRVEC_INIT; > - strvec_pop(&vec); > - check_strvec(&vec, NULL); > - strvec_clear(&vec); > - } > - > - 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); > - } > - > - if_test ("split empty string") { > - struct strvec vec = STRVEC_INIT; > - strvec_split(&vec, ""); > - check_strvec(&vec, NULL); > - strvec_clear(&vec); > - } > - > - if_test ("split single item") { > - struct strvec vec = STRVEC_INIT; > - strvec_split(&vec, "foo"); > - check_strvec(&vec, "foo", NULL); > - strvec_clear(&vec); > - } > - > - 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); > - } > - > - if_test ("split whitespace only") { > - struct strvec vec = STRVEC_INIT; > - strvec_split(&vec, " \t\n"); > - check_strvec(&vec, NULL); > - strvec_clear(&vec); > - } > - > - 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); > - } > - > - if_test ("detach") { > - struct strvec vec = STRVEC_INIT; > - const char **detached; > - > - strvec_push(&vec, "foo"); > - > - detached = strvec_detach(&vec); > - check_str(detached[0], "foo"); > - check_pointer_eq(detached[1], NULL); > - > - check_pointer_eq(vec.v, empty_strvec); > - check_uint(vec.nr, ==, 0); > - check_uint(vec.alloc, ==, 0); > - > - free((char *) detached[0]); > - free(detached); > - } > - > - return test_done(); > -}