Hi, this is the seventh version fo my patch series that introduces the clar unit testing framework. Changes compared to v6: - Add some explanations for why we introduce clar in the first place to the second commit message. - Split out implementation of the test driver, that is the main function, into a separate commit. - Reimplement argument parsing ourselves. This allows more flexibility and gets rid of some options that do not make sense for us. - Add an empty line between declarations and code. - Improve the test messages in ctype tests to also mention our expectations. - Adapt `cl_failf()` to not use `xstrfmt()`, but `snprintf()` with a static buffer instead. Thanks! Patrick Johannes Schindelin (4): clar: avoid compile error with mingw-w64 clar(win32): avoid compile error due to unused `fs_copy()` clar: stop including `shellapi.h` unnecessarily clar: add CMake support Patrick Steinhardt (10): t: do not pass GIT_TEST_OPTS to unit tests with prove t: import the clar unit testing framework t/clar: fix compatibility with NonStop Makefile: fix sparse dependency on GENERATED_H Makefile: make hdr-check depend on generated headers Makefile: do not use sparse on third-party sources Makefile: wire up the clar unit testing framework t/unit-tests: implement test driver t/unit-tests: convert strvec tests to use clar t/unit-tests: convert ctype tests to use clar .gitignore | 1 + Documentation/technical/unit-tests.txt | 2 + Makefile | 53 +- contrib/buildsystems/CMakeLists.txt | 53 ++ t/Makefile | 4 +- t/run-test.sh | 2 +- t/unit-tests/.gitignore | 2 + t/unit-tests/clar-generate.awk | 50 ++ t/unit-tests/clar/.github/workflows/ci.yml | 23 + t/unit-tests/clar/COPYING | 15 + t/unit-tests/clar/README.md | 329 ++++++++ t/unit-tests/clar/clar.c | 842 +++++++++++++++++++++ t/unit-tests/clar/clar.h | 173 +++++ t/unit-tests/clar/clar/fixtures.h | 50 ++ t/unit-tests/clar/clar/fs.h | 524 +++++++++++++ t/unit-tests/clar/clar/print.h | 211 ++++++ t/unit-tests/clar/clar/sandbox.h | 159 ++++ t/unit-tests/clar/clar/summary.h | 143 ++++ t/unit-tests/clar/generate.py | 266 +++++++ t/unit-tests/clar/test/.gitignore | 4 + t/unit-tests/clar/test/Makefile | 39 + t/unit-tests/clar/test/clar_test.h | 16 + t/unit-tests/clar/test/main.c | 40 + t/unit-tests/clar/test/main.c.sample | 27 + t/unit-tests/clar/test/resources/test/file | 1 + t/unit-tests/clar/test/sample.c | 84 ++ t/unit-tests/{t-ctype.c => ctype.c} | 71 +- t/unit-tests/strvec.c | 241 ++++++ t/unit-tests/t-strvec.c | 211 ------ t/unit-tests/unit-test.c | 45 ++ t/unit-tests/unit-test.h | 10 + 31 files changed, 3457 insertions(+), 234 deletions(-) create mode 100644 t/unit-tests/clar-generate.awk create mode 100644 t/unit-tests/clar/.github/workflows/ci.yml create mode 100644 t/unit-tests/clar/COPYING create mode 100644 t/unit-tests/clar/README.md create mode 100644 t/unit-tests/clar/clar.c create mode 100644 t/unit-tests/clar/clar.h create mode 100644 t/unit-tests/clar/clar/fixtures.h create mode 100644 t/unit-tests/clar/clar/fs.h create mode 100644 t/unit-tests/clar/clar/print.h create mode 100644 t/unit-tests/clar/clar/sandbox.h create mode 100644 t/unit-tests/clar/clar/summary.h create mode 100755 t/unit-tests/clar/generate.py create mode 100644 t/unit-tests/clar/test/.gitignore create mode 100644 t/unit-tests/clar/test/Makefile create mode 100644 t/unit-tests/clar/test/clar_test.h create mode 100644 t/unit-tests/clar/test/main.c create mode 100644 t/unit-tests/clar/test/main.c.sample create mode 100644 t/unit-tests/clar/test/resources/test/file create mode 100644 t/unit-tests/clar/test/sample.c rename t/unit-tests/{t-ctype.c => ctype.c} (68%) create mode 100644 t/unit-tests/strvec.c delete mode 100644 t/unit-tests/t-strvec.c create mode 100644 t/unit-tests/unit-test.c create mode 100644 t/unit-tests/unit-test.h Range-diff against v6: 1: e48a6461137 = 1: b67f10ec0b0 t: do not pass GIT_TEST_OPTS to unit tests with prove 2: 1710e9f9ff7 ! 2: 55a9b46e65f t: import the clar unit testing framework @@ Metadata ## Commit message ## t: import the clar unit testing framework + Our unit testing framework is a homegrown solution. While it supports + most of our needs, it is likely that the volume of unit tests will grow + quite a bit in the future such that we can exercise low-level subsystems + directly. This surfaces several shortcomings that the current solution + has: + + - There is no way to run only one specific tests. While some of our + unit tests wire this up manually, others don't. In general, it + requires quite a bit of boilerplate to get this set up correctly. + + - Failures do not cause a test to stop execution directly. Instead, + the test author needs to return manually whenever an assertion + fails. This is rather verbose and is not done correctly in most of + our unit tests. + + - Wiring up a new testcase requires both implementing the test + function and calling it in the respective test suite's main + function, which is creating code duplication. + + We can of course fix all of these issues ourselves, but that feels + rather pointless when there are already so many unit testing frameworks + out there that have those features. + + We line out some requirements for any unit testing framework in + "Documentation/technical/unit-tests.txt". The "clar" unit testing + framework, which isn't listed in that table yet, ticks many of the + boxes: + + - It is licensed under ISC, which is compatible. + + - It is easily vendorable because it is rather tiny at around 1200 + lines of code. + + - It is easily hackable due to the same reason. + + - It has TAP support. + + - It has skippable tests. + + - It preprocesses test files in order to extract test functions, which + then get wired up automatically. + + While it's not perfect, the fact that clar originates from the libgit2 + project means that it should be rather easy for us to collaborate with + upstream to plug any gaps. + Import the clar unit testing framework at commit 1516124 (Merge pull request #97 from pks-t/pks-whitespace-fixes, 2024-08-15). The framework will be wired up in subsequent commits. 3: 5c21aa87aa2 = 3: f24401f0a87 t/clar: fix compatibility with NonStop 4: 06d2bce0d82 = 4: 658a601c541 clar: avoid compile error with mingw-w64 5: f88b3421a09 = 5: 0b8a6ac5fed clar(win32): avoid compile error due to unused `fs_copy()` 6: 5fb4c55be33 = 6: c50e7a0ea68 clar: stop including `shellapi.h` unnecessarily 7: e0dcbd5ca83 = 7: b8f3f16dd27 Makefile: fix sparse dependency on GENERATED_H 8: 77a03f8df70 = 8: 3d3fe443b9a Makefile: make hdr-check depend on generated headers 9: c91dd7327e3 = 9: 7d0f494850a Makefile: do not use sparse on third-party sources 10: 115c15aa9ae ! 10: 9c74c5ae019 Makefile: wire up the clar unit testing framework @@ Commit message anything. Thus, it would cause a compiler error if a function name was mistyped and thus not picked up by "generate.py". + The test driver "unit-test.c" is an empty stub for now. It will get + implemented in the next commit. + Signed-off-by: Patrick Steinhardt <ps@xxxxxx> ## .gitignore ## @@ t/unit-tests/unit-test.c (new) @@ +#include "unit-test.h" + -+int cmd_main(int argc, const char **argv) ++int cmd_main(int argc UNUSED, const char **argv UNUSED) +{ -+ const char **argv_copy; -+ int ret; -+ -+ /* Append the "-t" flag such that the tests generate TAP output. */ -+ ALLOC_ARRAY(argv_copy, argc + 2); -+ COPY_ARRAY(argv_copy, argv, argc); -+ argv_copy[argc++] = "-t"; -+ argv_copy[argc] = NULL; -+ -+ ret = clar_test(argc, (char **) argv_copy); -+ -+ free(argv_copy); -+ return ret; ++ return 0; +} ## t/unit-tests/unit-test.h (new) ## -: ----------- > 11: 8bd5b3e2b29 t/unit-tests: implement test driver 11: b3b8df04872 ! 12: 3c3b9eacdfb t/unit-tests: convert strvec tests to use clar @@ Commit message 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: + running that binary with an injected error, it generates TAP output: # ./t/unit-tests/bin/unit-tests TAP version 13 @@ Commit message ok 1 - strvec::init ok 2 - strvec::dynamic_init ok 3 - strvec::clear - ok 4 - strvec::push - ok 5 - strvec::pushft_pushf + not ok 4 - strvec::push + --- + reason: | + String mismatch: (&vec)->v[i] != expect[i] + 'foo' != 'fo' (at byte 2) + at: + file: 't/unit-tests/strvec.c' + line: 48 + function: 'test_strvec__push' + --- + ok 5 - strvec::pushf ok 6 - strvec::pushl ok 7 - strvec::pushv ok 8 - strvec::replace_at_head @@ t/unit-tests/strvec.c (new) +#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); \ ++ size_t expect_len = ARRAY_SIZE(expect); \ ++ cl_assert(expect_len > 0); \ ++ cl_assert_equal_p(expect[expect_len - 1], NULL); \ ++ cl_assert_equal_i((vec)->nr, expect_len - 1); \ + cl_assert((vec)->nr <= (vec)->alloc); \ -+ for (size_t i = 0; i < ARRAY_SIZE(expect); i++) \ ++ for (size_t i = 0; i < expect_len; 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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +void test_strvec__pop_empty_array(void) +{ + struct strvec vec = STRVEC_INIT; ++ + strvec_pop(&vec); + check_strvec(&vec, NULL); + strvec_clear(&vec); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +void test_strvec__split_empty_string(void) +{ + struct strvec vec = STRVEC_INIT; ++ + strvec_split(&vec, ""); + check_strvec(&vec, NULL); + strvec_clear(&vec); @@ t/unit-tests/strvec.c (new) +void test_strvec__split_single_item(void) +{ + struct strvec vec = STRVEC_INIT; ++ + strvec_split(&vec, "foo"); + check_strvec(&vec, "foo", NULL); + strvec_clear(&vec); @@ t/unit-tests/strvec.c (new) +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); @@ t/unit-tests/strvec.c (new) +void test_strvec__split_whitespace_only(void) +{ + struct strvec vec = STRVEC_INIT; ++ + strvec_split(&vec, " \t\n"); + check_strvec(&vec, NULL); + strvec_clear(&vec); @@ t/unit-tests/strvec.c (new) +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); 12: 1ac2e48a7f2 ! 13: c8360db2f86 t/unit-tests: convert ctype tests to use clar @@ Commit message on failure: # start of suite 1: ctype - ok 1 - ctype::isspace - not ok 2 - ctype::isdigit + not ok 1 - ctype::isspace --- reason: | Test failed. - 0x61 is classified incorrectly + 0x0d is classified incorrectly: expected 0, got 1 at: file: 't/unit-tests/ctype.c' - line: 38 - function: 'test_ctype__isdigit' + line: 36 + function: 'test_ctype__isspace' --- + ok 2 - ctype::isdigit ok 3 - ctype::isalpha ok 4 - ctype::isalnum ok 5 - ctype::is_glob_special @@ t/unit-tests/t-ctype.c => t/unit-tests/ctype.c - test_msg(" i: 0x%02x", i); \ - } \ - check(!class(EOF)); \ -- } \ -+ for (int i = 0; i < 256; i++) \ -+ if (class(i) != !!memchr(string, i, len)) \ -+ cl_failf("0x%02x is classified incorrectly", i); \ ++ for (int i = 0; i < 256; i++) { \ ++ int actual = class(i), expect = !!memchr(string, i, len); \ ++ if (actual != expect) \ ++ cl_failf("0x%02x is classified incorrectly: expected %d, got %d", \ ++ i, expect, actual); \ + } \ + cl_assert(!class(EOF)); \ } while (0) @@ t/unit-tests/unit-test.h +#include "strbuf.h" + +#define cl_failf(fmt, ...) do { \ -+ char *desc = xstrfmt(fmt, __VA_ARGS__); \ ++ char desc[4096]; \ ++ snprintf(desc, sizeof(desc), fmt, __VA_ARGS__); \ + clar__fail(__FILE__, __func__, __LINE__, "Test failed.", desc, 1); \ -+ free(desc); \ +} while (0) 13: 131036c398e = 14: d51c146cd9d clar: add CMake support -- 2.46.0.421.g159f2d50e7.dirty