| Message ID | 20250130091334.39922-3-kuforiji98@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | t/unit-tests: convert unit-tests to use clar | expand |
On Thu, Jan 30, 2025 at 10:13:32AM +0100, Seyi Kuforiji wrote: > Adapts example decorate test script to clar framework by using clar > assertions where necessary. Test functions are created as standalone to > test different test cases. Again, I don't think that the second sentence doesn't add much information to the commit message and can probably just be dropped. It would be more interesting to talk about any design decisions that weren't immediately obvious, as these may also make the reviewer wonder. One thing that would be worth to explain here are the `initialize()` and `cleanup()` functions. Readers not familiar with the clar framework may not know that those get picked up by the framework automatically and that they are executed before and after every single test. > Mentored-by: Patrick Steinhardt <ps@pks.im> > Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> > --- > Makefile | 2 +- > t/meson.build | 2 +- > t/unit-tests/t-example-decorate.c | 74 ------------------------------ > t/unit-tests/u-example-decorate.c | 76 +++++++++++++++++++++++++++++++ > 4 files changed, 78 insertions(+), 76 deletions(-) > delete mode 100644 t/unit-tests/t-example-decorate.c > create mode 100644 t/unit-tests/u-example-decorate.c > > diff --git a/Makefile b/Makefile > index 2d9dad119a..732d765f1c 100644 > --- a/Makefile > +++ b/Makefile > @@ -1338,6 +1338,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/% > THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/% > > CLAR_TEST_SUITES += u-ctype > +CLAR_TEST_SUITES += u-example-decorate > CLAR_TEST_SUITES += u-hash > CLAR_TEST_SUITES += u-hashmap > CLAR_TEST_SUITES += u-mem-pool > @@ -1349,7 +1350,6 @@ CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES)) > CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o > CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o > > -UNIT_TEST_PROGRAMS += t-example-decorate > UNIT_TEST_PROGRAMS += t-oid-array > UNIT_TEST_PROGRAMS += t-oidmap > UNIT_TEST_PROGRAMS += t-oidtree > diff --git a/t/meson.build b/t/meson.build > index af597f9804..c7e08eca6f 100644 > --- a/t/meson.build > +++ b/t/meson.build > @@ -1,5 +1,6 @@ > clar_test_suites = [ > 'unit-tests/u-ctype.c', > + 'unit-tests/u-example-decorate.c', > 'unit-tests/u-hash.c', > 'unit-tests/u-hashmap.c', > 'unit-tests/u-mem-pool.c', > @@ -45,7 +46,6 @@ clar_unit_tests = executable('unit-tests', > test('unit-tests', clar_unit_tests) > > unit_test_programs = [ > - 'unit-tests/t-example-decorate.c', > 'unit-tests/t-oid-array.c', > 'unit-tests/t-oidmap.c', > 'unit-tests/t-oidtree.c', > diff --git a/t/unit-tests/t-example-decorate.c b/t/unit-tests/t-example-decorate.c > deleted file mode 100644 > index bfc776e223..0000000000 > --- a/t/unit-tests/t-example-decorate.c > +++ /dev/null > @@ -1,74 +0,0 @@ > -#define USE_THE_REPOSITORY_VARIABLE > - > -#include "test-lib.h" > -#include "object.h" > -#include "decorate.h" > -#include "repository.h" > - > -struct test_vars { > - struct object *one, *two, *three; > - struct decoration n; > - int decoration_a, decoration_b; > -}; > - > -static void t_add(struct test_vars *vars) > -{ > - void *ret = add_decoration(&vars->n, vars->one, &vars->decoration_a); > - > - check(ret == NULL); > - ret = add_decoration(&vars->n, vars->two, NULL); > - check(ret == NULL); > -} > - > -static void t_readd(struct test_vars *vars) > -{ > - void *ret = add_decoration(&vars->n, vars->one, NULL); > - > - check(ret == &vars->decoration_a); > - ret = add_decoration(&vars->n, vars->two, &vars->decoration_b); > - check(ret == NULL); > -} > - > -static void t_lookup(struct test_vars *vars) > -{ > - void *ret = lookup_decoration(&vars->n, vars->one); > - > - check(ret == NULL); > - ret = lookup_decoration(&vars->n, vars->two); > - check(ret == &vars->decoration_b); > - ret = lookup_decoration(&vars->n, vars->three); > - check(ret == NULL); > -} > - > -static void t_loop(struct test_vars *vars) > -{ > - int objects_noticed = 0; > - > - for (size_t i = 0; i < vars->n.size; i++) { > - if (vars->n.entries[i].base) > - objects_noticed++; > - } > - check_int(objects_noticed, ==, 2); > -} > - > -int cmd_main(int argc UNUSED, const char **argv UNUSED) > -{ > - struct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } }; > - struct test_vars vars = { 0 }; > - > - vars.one = lookup_unknown_object(the_repository, &one_oid); > - vars.two = lookup_unknown_object(the_repository, &two_oid); > - vars.three = lookup_unknown_object(the_repository, &three_oid); > - > - TEST(t_add(&vars), > - "Add 2 objects, one with a non-NULL decoration and one with a NULL decoration."); > - TEST(t_readd(&vars), > - "When re-adding an already existing object, the old decoration is returned."); > - TEST(t_lookup(&vars), > - "Lookup returns the added declarations, or NULL if the object was never added."); > - TEST(t_loop(&vars), "The user can also loop through all entries."); > - > - clear_decoration(&vars.n, NULL); > - > - return test_done(); > -} > diff --git a/t/unit-tests/u-example-decorate.c b/t/unit-tests/u-example-decorate.c > new file mode 100644 > index 0000000000..3a457d41fc > --- /dev/null > +++ b/t/unit-tests/u-example-decorate.c > @@ -0,0 +1,76 @@ > +#define USE_THE_REPOSITORY_VARIABLE > + > +#include "unit-test.h" > +#include "object.h" > +#include "decorate.h" > +#include "repository.h" > + > +struct test_vars { > + struct object *one, *two, *three; > + struct decoration n; > + int decoration_a, decoration_b; > +}; > + > +static struct test_vars vars; > + > +void test_example_decorate__add(void) > +{ > + void *ret = add_decoration(&vars.n, vars.one, &vars.decoration_a); > + cl_assert(ret == NULL); > + ret = add_decoration(&vars.n, vars.two, NULL); > + cl_assert(ret == NULL); > +} We could avoid the separate variable here, e.g. like this: void test_example_decorate__add(void) { cl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL); cl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL); } The same is true for some of the other test, too. The test itself could be a bit more thorough in verifying what has been done, but that is not a new issue that your conversion introduces as the old testcase was somewhat light on that, too. So this is nothing that I think you need to change. > +void test_example_decorate__readd(void) > +{ > + void *ret; > + > + cl_assert(add_decoration(&vars.n, vars.one, &vars.decoration_a) == NULL); > + cl_assert(add_decoration(&vars.n, vars.two, NULL) == NULL); Okay. This wasn't present before because tests were actually dependent on the data that previous tests wrote. Now that we decouple the tests from one another we have to make sure to recreate the expected setup. Which I think is a good thing, as it makes tests fully self-contained and the setup easier to understand. I guess this is another thing that you should be explaining in the commit message, as it is not obvious at all. > + ret = add_decoration(&vars.n, vars.one, NULL); > + cl_assert(ret == &vars.decoration_a); > + ret = add_decoration(&vars.n, vars.two, &vars.decoration_b); > + cl_assert(ret == NULL); > +} > + > +void test_example_decorate__lookup(void) > +{ > + void *ret; > + > + add_decoration(&vars.n, vars.two, &vars.decoration_b); > + add_decoration(&vars.n, vars.one, NULL); > + > + ret = lookup_decoration(&vars.n, vars.two); > + cl_assert(ret == &vars.decoration_b); > + ret = lookup_decoration(&vars.n, vars.one); > + cl_assert(ret == NULL); > +} > + > +void test_example_decorate__loop(void) > +{ > + int objects_noticed = 0; > + > + add_decoration(&vars.n, vars.one, &vars.decoration_a); > + add_decoration(&vars.n, vars.two, &vars.decoration_b); > + > + for (size_t i = 0; i < vars.n.size; i++) { > + if (vars.n.entries[i].base) > + objects_noticed++; > + } Nit: we can get rid of the curly braces here. > + cl_assert_equal_i(objects_noticed, 2); > +} > + > +void test_example_decorate__initialize(void) > +{ > + struct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } }; > + > + vars.one = lookup_unknown_object(the_repository, &one_oid); > + vars.two = lookup_unknown_object(the_repository, &two_oid); > + vars.three = lookup_unknown_object(the_repository, &three_oid); > +} > + > +void test_example_decorate__cleanup(void) > +{ > + clear_decoration(&vars.n, NULL); > +} Let's move both `initialize()` and `cleanup()` to the top so that it becomes easy to see that all tests use this commen setup. Patrick
diff --git a/Makefile b/Makefile index 2d9dad119a..732d765f1c 100644 --- a/Makefile +++ b/Makefile @@ -1338,6 +1338,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/% THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/% CLAR_TEST_SUITES += u-ctype +CLAR_TEST_SUITES += u-example-decorate CLAR_TEST_SUITES += u-hash CLAR_TEST_SUITES += u-hashmap CLAR_TEST_SUITES += u-mem-pool @@ -1349,7 +1350,6 @@ CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES)) CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o -UNIT_TEST_PROGRAMS += t-example-decorate UNIT_TEST_PROGRAMS += t-oid-array UNIT_TEST_PROGRAMS += t-oidmap UNIT_TEST_PROGRAMS += t-oidtree diff --git a/t/meson.build b/t/meson.build index af597f9804..c7e08eca6f 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1,5 +1,6 @@ clar_test_suites = [ 'unit-tests/u-ctype.c', + 'unit-tests/u-example-decorate.c', 'unit-tests/u-hash.c', 'unit-tests/u-hashmap.c', 'unit-tests/u-mem-pool.c', @@ -45,7 +46,6 @@ clar_unit_tests = executable('unit-tests', test('unit-tests', clar_unit_tests) unit_test_programs = [ - 'unit-tests/t-example-decorate.c', 'unit-tests/t-oid-array.c', 'unit-tests/t-oidmap.c', 'unit-tests/t-oidtree.c', diff --git a/t/unit-tests/t-example-decorate.c b/t/unit-tests/t-example-decorate.c deleted file mode 100644 index bfc776e223..0000000000 --- a/t/unit-tests/t-example-decorate.c +++ /dev/null @@ -1,74 +0,0 @@ -#define USE_THE_REPOSITORY_VARIABLE - -#include "test-lib.h" -#include "object.h" -#include "decorate.h" -#include "repository.h" - -struct test_vars { - struct object *one, *two, *three; - struct decoration n; - int decoration_a, decoration_b; -}; - -static void t_add(struct test_vars *vars) -{ - void *ret = add_decoration(&vars->n, vars->one, &vars->decoration_a); - - check(ret == NULL); - ret = add_decoration(&vars->n, vars->two, NULL); - check(ret == NULL); -} - -static void t_readd(struct test_vars *vars) -{ - void *ret = add_decoration(&vars->n, vars->one, NULL); - - check(ret == &vars->decoration_a); - ret = add_decoration(&vars->n, vars->two, &vars->decoration_b); - check(ret == NULL); -} - -static void t_lookup(struct test_vars *vars) -{ - void *ret = lookup_decoration(&vars->n, vars->one); - - check(ret == NULL); - ret = lookup_decoration(&vars->n, vars->two); - check(ret == &vars->decoration_b); - ret = lookup_decoration(&vars->n, vars->three); - check(ret == NULL); -} - -static void t_loop(struct test_vars *vars) -{ - int objects_noticed = 0; - - for (size_t i = 0; i < vars->n.size; i++) { - if (vars->n.entries[i].base) - objects_noticed++; - } - check_int(objects_noticed, ==, 2); -} - -int cmd_main(int argc UNUSED, const char **argv UNUSED) -{ - struct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } }; - struct test_vars vars = { 0 }; - - vars.one = lookup_unknown_object(the_repository, &one_oid); - vars.two = lookup_unknown_object(the_repository, &two_oid); - vars.three = lookup_unknown_object(the_repository, &three_oid); - - TEST(t_add(&vars), - "Add 2 objects, one with a non-NULL decoration and one with a NULL decoration."); - TEST(t_readd(&vars), - "When re-adding an already existing object, the old decoration is returned."); - TEST(t_lookup(&vars), - "Lookup returns the added declarations, or NULL if the object was never added."); - TEST(t_loop(&vars), "The user can also loop through all entries."); - - clear_decoration(&vars.n, NULL); - - return test_done(); -} diff --git a/t/unit-tests/u-example-decorate.c b/t/unit-tests/u-example-decorate.c new file mode 100644 index 0000000000..3a457d41fc --- /dev/null +++ b/t/unit-tests/u-example-decorate.c @@ -0,0 +1,76 @@ +#define USE_THE_REPOSITORY_VARIABLE + +#include "unit-test.h" +#include "object.h" +#include "decorate.h" +#include "repository.h" + +struct test_vars { + struct object *one, *two, *three; + struct decoration n; + int decoration_a, decoration_b; +}; + +static struct test_vars vars; + +void test_example_decorate__add(void) +{ + void *ret = add_decoration(&vars.n, vars.one, &vars.decoration_a); + cl_assert(ret == NULL); + ret = add_decoration(&vars.n, vars.two, NULL); + cl_assert(ret == NULL); +} + +void test_example_decorate__readd(void) +{ + void *ret; + + cl_assert(add_decoration(&vars.n, vars.one, &vars.decoration_a) == NULL); + cl_assert(add_decoration(&vars.n, vars.two, NULL) == NULL); + + ret = add_decoration(&vars.n, vars.one, NULL); + cl_assert(ret == &vars.decoration_a); + ret = add_decoration(&vars.n, vars.two, &vars.decoration_b); + cl_assert(ret == NULL); +} + +void test_example_decorate__lookup(void) +{ + void *ret; + + add_decoration(&vars.n, vars.two, &vars.decoration_b); + add_decoration(&vars.n, vars.one, NULL); + + ret = lookup_decoration(&vars.n, vars.two); + cl_assert(ret == &vars.decoration_b); + ret = lookup_decoration(&vars.n, vars.one); + cl_assert(ret == NULL); +} + +void test_example_decorate__loop(void) +{ + int objects_noticed = 0; + + add_decoration(&vars.n, vars.one, &vars.decoration_a); + add_decoration(&vars.n, vars.two, &vars.decoration_b); + + for (size_t i = 0; i < vars.n.size; i++) { + if (vars.n.entries[i].base) + objects_noticed++; + } + cl_assert_equal_i(objects_noticed, 2); +} + +void test_example_decorate__initialize(void) +{ + struct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } }; + + vars.one = lookup_unknown_object(the_repository, &one_oid); + vars.two = lookup_unknown_object(the_repository, &two_oid); + vars.three = lookup_unknown_object(the_repository, &three_oid); +} + +void test_example_decorate__cleanup(void) +{ + clear_decoration(&vars.n, NULL); +}
Adapts example decorate test script to clar framework by using clar assertions where necessary. Test functions are created as standalone to test different test cases. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> --- Makefile | 2 +- t/meson.build | 2 +- t/unit-tests/t-example-decorate.c | 74 ------------------------------ t/unit-tests/u-example-decorate.c | 76 +++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 76 deletions(-) delete mode 100644 t/unit-tests/t-example-decorate.c create mode 100644 t/unit-tests/u-example-decorate.c