| Message ID | b3b8df048725c25b14860513b7950b158a6990ea.1724159966.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce clar testing framework | expand |
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(); > -}
On Wed, Aug 28, 2024 at 02:17:05PM +0100, Phillip Wood wrote: > 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. Will do. > > # ./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? The `-v` switch is actually doing nothing when generating TAP output. > > -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 True indeed. This is the default argument parsing and output from clar, so it's nothing that we can change. That being said, I guess the best way to address this is to use our own option parsing here instead of using whatever clar provides, and then we can also print our own usage. Will amend accordingly. > > -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. Can do. > > + 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. This is something I'd address in the future, once we have macros that can do relative comparisons. > > + 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. Can do. > 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. I don't think it's a good fit here, as setup and teardown would hit the system under test. I rather think they should be used in cases where you e.g. always have to setup a repository for your tests. Patrick
Hi Patrick On 03/09/2024 08:45, Patrick Steinhardt wrote: > On Wed, Aug 28, 2024 at 02:17:05PM +0100, Phillip Wood wrote: >> Hi Patrick >> >> On 20/08/2024 15:02, Patrick Steinhardt wrote: >> 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. > > I don't think it's a good fit here, as setup and teardown would hit the > system under test. I rather think they should be used in cases where you > e.g. always have to setup a repository for your tests. I'm not sure I follow. I was suggesting we define test_strvec__initialize() to initialize a global strvec which the tests use and is then freed by test_strvec__cleanup() like the tests/adding.c example the clar's README.md. That would allow use to remove the setup and teardown from each test. As I understand it clar's setup/cleanup functionality is usable without setting up a sandbox directory for each test. I'll take a look at v7 in the next few days - I suspect we're getting to the point where it's ready to be merged. Best Wishes Phillip > Patrick
On Tue, Sep 03, 2024 at 10:48:01AM +0100, phillip.wood123@gmail.com wrote: > Hi Patrick > > On 03/09/2024 08:45, Patrick Steinhardt wrote: > > On Wed, Aug 28, 2024 at 02:17:05PM +0100, Phillip Wood wrote: > > > Hi Patrick > > > > > > On 20/08/2024 15:02, Patrick Steinhardt wrote: > > > 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. > > > > I don't think it's a good fit here, as setup and teardown would hit the > > system under test. I rather think they should be used in cases where you > > e.g. always have to setup a repository for your tests. > > I'm not sure I follow. I was suggesting we define test_strvec__initialize() > to initialize a global strvec which the tests use and is then freed by > test_strvec__cleanup() like the tests/adding.c example the clar's README.md. > That would allow use to remove the setup and teardown from each test. As I > understand it clar's setup/cleanup functionality is usable without setting > up a sandbox directory for each test. What I'm saying is that `strvec_init()` itself is part of the system under test, so evicting that into a `__initialize()` function doesn't quite make sense to me. If there was for example a bug somewhere in the strvec code we might bring the global `struct strvec` into a state that is completely unusable, and thus all subsequent tests would fail. We could of course work around that by always zeroing out the struct, but because of that I just don't think it's a good fit. I rather see the usefulness of `__initialize()` in setting up auxiliary data structures that are a dependency of the system under test, but which are not the system under test itself. > I'll take a look at v7 in the next few days - I suspect we're getting to the > point where it's ready to be merged. Thanks!
On 04/09/2024 07:37, Patrick Steinhardt wrote: > On Tue, Sep 03, 2024 at 10:48:01AM +0100, phillip.wood123@gmail.com wrote: >> On 03/09/2024 08:45, Patrick Steinhardt wrote: >>> On Wed, Aug 28, 2024 at 02:17:05PM +0100, Phillip Wood wrote: >>>> On 20/08/2024 15:02, Patrick Steinhardt wrote: >>>> 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. >>> >>> I don't think it's a good fit here, as setup and teardown would hit the >>> system under test. I rather think they should be used in cases where you >>> e.g. always have to setup a repository for your tests. >> >> I'm not sure I follow. I was suggesting we define test_strvec__initialize() >> to initialize a global strvec which the tests use and is then freed by >> test_strvec__cleanup() like the tests/adding.c example the clar's README.md. >> That would allow use to remove the setup and teardown from each test. As I >> understand it clar's setup/cleanup functionality is usable without setting >> up a sandbox directory for each test. > > What I'm saying is that `strvec_init()` itself is part of the system > under test, Oh, I see - I'd misunderstood what you meant by "system under test" > so evicting that into a `__initialize()` function doesn't > quite make sense to me. If there was for example a bug somewhere in the > strvec code we might bring the global `struct strvec` into a state that > is completely unusable, and thus all subsequent tests would fail. We > could of course work around that by always zeroing out the struct, but > because of that I just don't think it's a good fit. Isn't the point of strvec_init() to ensure that the `struct strvec` passed to it is usable? If it does not do that then having all the tests fail is good as it tells us there is a bug. I was hoping that __initialize() would be a useful replacement for the TEST(setup(test_fn()), "description") pattern in some of our existing tests. I find hoisting the common initialization and cleanup out of each tests makes them easier to review as I can review the setup code once and not worry about it when reviewing each test. It also avoids a lot of repetition when writing tests and keeps the test bodies concise. > I rather see the usefulness of `__initialize()` in setting up auxiliary > data structures that are a dependency of the system under test, but > which are not the system under test itself. That's certainly a good use for them Best Wishes Phillip >> I'll take a look at v7 in the next few days - I suspect we're getting to the >> point where it's ready to be merged. > > Thanks!
diff --git a/Makefile b/Makefile index e38146b5eb0..56ce6c00e44 100644 --- a/Makefile +++ b/Makefile @@ -1336,6 +1336,7 @@ THIRD_PARTY_SOURCES += sha1dc/% THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/% THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/% +UNIT_TESTS_SUITES += strvec UNIT_TESTS_PROG = $(UNIT_TEST_BIN)/unit-tests$(X) UNIT_TESTS_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TESTS_SUITES)) UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/clar/clar.o @@ -1356,7 +1357,6 @@ UNIT_TEST_PROGRAMS += t-reftable-record UNIT_TEST_PROGRAMS += t-reftable-tree UNIT_TEST_PROGRAMS += t-strbuf UNIT_TEST_PROGRAMS += t-strcmp-offset -UNIT_TEST_PROGRAMS += t-strvec UNIT_TEST_PROGRAMS += t-trailer UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS)) UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS)) diff --git a/t/unit-tests/strvec.c b/t/unit-tests/strvec.c new file mode 100644 index 00000000000..d11ed0f28de --- /dev/null +++ b/t/unit-tests/strvec.c @@ -0,0 +1,222 @@ +#include "unit-test.h" +#include "strbuf.h" +#include "strvec.h" + +#define check_strvec(vec, ...) \ + do { \ + const char *expect[] = { __VA_ARGS__ }; \ + cl_assert(ARRAY_SIZE(expect) > 0); \ + 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); \ + for (size_t i = 0; i < ARRAY_SIZE(expect); i++) \ + cl_assert_equal_s((vec)->v[i], expect[i]); \ + } while (0) + +void test_strvec__init(void) +{ + struct strvec vec = STRVEC_INIT; + 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(); -}
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: # ./t/unit-tests/bin/unit-tests TAP version 13 # start of suite 1: strvec ok 1 - strvec::init ok 2 - strvec::dynamic_init ok 3 - strvec::clear ok 4 - strvec::push ok 5 - strvec::pushft_pushf ok 6 - strvec::pushl ok 7 - strvec::pushv ok 8 - strvec::replace_at_head ok 9 - strvec::replace_at_tail ok 10 - strvec::replace_in_between ok 11 - strvec::replace_with_substring ok 12 - strvec::remove_at_head ok 13 - strvec::remove_at_tail ok 14 - strvec::remove_in_between ok 15 - strvec::pop_empty_array ok 16 - strvec::pop_non_empty_array ok 17 - strvec::split_empty_string ok 18 - strvec::split_single_item ok 19 - strvec::split_multiple_items ok 20 - strvec::split_whitespace_only ok 21 - strvec::split_multiple_consecutive_whitespaces ok 22 - strvec::detach 1..22 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) -q Only report tests that had an error -Q Quit as soon as a test fails -t Display results in tap format -l Print suite names -r[filename] Write summary file (to the optional filename) Furthermore, running `make unit-tests` runs the binary along with all the other unit tests we have. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Makefile | 2 +- t/unit-tests/strvec.c | 222 ++++++++++++++++++++++++++++++++++++++++ t/unit-tests/t-strvec.c | 211 -------------------------------------- 3 files changed, 223 insertions(+), 212 deletions(-) create mode 100644 t/unit-tests/strvec.c delete mode 100644 t/unit-tests/t-strvec.c