| Message ID | c51025cc-26e5-41e2-be56-94e141a64f5d@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add and use for_test to simplify tests | expand |
On Wed, Jul 24, 2024 at 7:53 AM René Scharfe <l.s.r@web.de> wrote: > > The macro TEST only allows defining a test that consists of a single > expression. Add a new macro, for_test, which provides a way to define > unit tests that are made up of one or more statements. Perhaps obvious to you and others, but I'm at a loss; can you elaborate on why this is a significant problem? Why is having the tests in a separate function a significant downside, and what are the benefits of this new macro that counteract the "magic" here? I'm not seeing it. Macros like this require additional cognitive overhead to understand; I can't read `for_test ("for_test passing test") check_int(1, ==, 1);` and understand what's going on without going and reading the comment above `for_test`. Unlike `TEST(<func>, "<description>")`, I can't even _guess_ what it's doing. This macro is project-specific, and this knowledge has to be attained by everyone wishing to even read this code. I personally consider the patches that use this to be regressions in my ability to read and understand the tests. As an example, one of the diff hunks in the strbuf patch (patch #6 in the series) does this: - TEST(setup_populated(t_addch, "initial value", "a"), - "strbuf_addch appends to initial value"); + for_test ("strbuf_addch appends to initial value") { + struct strbuf sb = STRBUF_INIT; + t_addstr(&sb, "initial value"); + t_addch(&sb, 'a'); + t_release(&sb); + } But the main simplification in that patch (not using `setup_populated`) can be achieved without `for_test`: + static void t_addch_appends(void) + { + struct strbuf sb = STRBUF_INIT; + t_addstr(&sb, "initial value"); + t_addch(&sb, 'a'); + t_release(&sb); + } - TEST(setup_populated(t_addch, "initial value", "a"), - "strbuf_addch appends to initial value"); + TEST(t_addch_appends(), "strbuf_addch appends to initial value"); It seems to me that all `for_test` is getting us there is an inlining of the test logic, which seems like it's optimizing for vertical space in the file. Maybe it's because I'm coming from a C++ environment at $JOB that's using Google's gunit and gmock frameworks, where every test is in its own function and we usually don't even write the main function ourselves, but I have a preference for the separate functions. Maybe I'm in the minority, so only consider this at somewhere like a -0.5 on this series (fine with deferring to a reviewer who is significantly in favor of it, but if there aren't any, I'd lean towards not landing this).
On 24/07/2024 20:24, Kyle Lippincott wrote: > On Wed, Jul 24, 2024 at 7:53 AM René Scharfe <l.s.r@web.de> wrote: >> > Macros like this require additional cognitive overhead to understand; > I can't read `for_test ("for_test passing test") check_int(1, ==, 1);` > and understand what's going on without going and reading the comment > above `for_test`. Unlike `TEST(<func>, "<description>")`, I can't even > _guess_ what it's doing. This macro is project-specific, and this > knowledge has to be attained by everyone wishing to even read this > code. As well as the obscure name and oddness of using "continue" to exit a test early I think having two ways of defining tests also adds to the cognitive overhead of writing tests as one now has to decide which to use. As you say below it is not clear to me that the changes in the last three patches that start using for_test() in our existing tests are an improvement. This will be the last comment from me for a while as I'm about to go off the list for a couple of weeks Best Wishes Phillip > I personally consider the patches that use this to be regressions in > my ability to read and understand the tests. As an example, one of the > diff hunks in the strbuf patch (patch #6 in the series) does this: > - TEST(setup_populated(t_addch, "initial value", "a"), > - "strbuf_addch appends to initial value"); > + for_test ("strbuf_addch appends to initial value") { > + struct strbuf sb = STRBUF_INIT; > + t_addstr(&sb, "initial value"); > + t_addch(&sb, 'a'); > + t_release(&sb); > + } > > But the main simplification in that patch (not using > `setup_populated`) can be achieved without `for_test`: > > + static void t_addch_appends(void) > + { > + struct strbuf sb = STRBUF_INIT; > + t_addstr(&sb, "initial value"); > + t_addch(&sb, 'a'); > + t_release(&sb); > + } > > - TEST(setup_populated(t_addch, "initial value", "a"), > - "strbuf_addch appends to initial value"); > + TEST(t_addch_appends(), "strbuf_addch appends to initial value"); > > It seems to me that all `for_test` is getting us there is an inlining > of the test logic, which seems like it's optimizing for vertical space > in the file. Maybe it's because I'm coming from a C++ environment at > $JOB that's using Google's gunit and gmock frameworks, where every > test is in its own function and we usually don't even write the main > function ourselves, but I have a preference for the separate > functions. > > Maybe I'm in the minority, so only consider this at somewhere like a > -0.5 on this series (fine with deferring to a reviewer who is > significantly in favor of it, but if there aren't any, I'd lean > towards not landing this). >
Kyle Lippincott <spectral@google.com> writes: > But the main simplification in that patch (not using > `setup_populated`) can be achieved without `for_test`: > > + static void t_addch_appends(void) > + { > + struct strbuf sb = STRBUF_INIT; > + t_addstr(&sb, "initial value"); > + t_addch(&sb, 'a'); > + t_release(&sb); > + } > > - TEST(setup_populated(t_addch, "initial value", "a"), > - "strbuf_addch appends to initial value"); > + TEST(t_addch_appends(), "strbuf_addch appends to initial value"); Yup. I like that better when we are using TEST(), whether the other thing exists or not. > It seems to me that all `for_test` is getting us there is an inlining > of the test logic, which seems like it's optimizing for vertical space > in the file. I would consider that it optimizes for human readers who have to scan the file. Having to jump around from caller to callee to understand the whole thing (rather, the description being away from the callee, which is where the most of the logic is anyway) is one gripe I have with the approach taken by the TEST() thing. > Maybe it's because I'm coming from a C++ environment at > $JOB that's using Google's gunit and gmock frameworks, where every > test is in its own function and we usually don't even write the main > function ourselves, but I have a preference for the separate > functions. If we do not have to write the main at all, then it would make the separate function that implements a single test a lot more palatable, and we do not even have to implement and call TEST() macro ;-). Thanks.
On Thu, Jul 25, 2024 at 9:02 AM Junio C Hamano <gitster@pobox.com> wrote: > > Kyle Lippincott <spectral@google.com> writes: > > > Maybe it's because I'm coming from a C++ environment at > > $JOB that's using Google's gunit and gmock frameworks, where every > > test is in its own function and we usually don't even write the main > > function ourselves, but I have a preference for the separate > > functions. > > If we do not have to write the main at all, then it would make the > separate function that implements a single test a lot more palatable, > and we do not even have to implement and call TEST() macro ;-). I feel like you're trying to bait me into coming up with a way of avoiding the main function ;) But I don't think it can really be done in portable/vanilla C, unfortunately. I tried to think of a way to do this, and they all involved some other system coming along and identifying the tests and code-generating a main function, which also seems like too much magic to me.
Kyle Lippincott <spectral@google.com> writes: >> > Maybe it's because I'm coming from a C++ environment at >> > $JOB that's using Google's gunit and gmock frameworks, where every >> > test is in its own function and we usually don't even write the main >> > function ourselves, but I have a preference for the separate >> > functions. >> >> If we do not have to write the main at all, then it would make the >> separate function that implements a single test a lot more palatable, >> and we do not even have to implement and call TEST() macro ;-). > ... > I tried to think of a way to do > this, and they all involved some other system coming along and > identifying the tests and code-generating a main function, which also > seems like too much magic to me. I thought that automatically generating the boilerplates from the visible list of test functions and piecing them together with a synthetic main was what was brought up as how libgit2 project does this? It does not sound all that involved and I do not find it a rocket science. In any case, a well written framework soon becomes "magic" anyway. Our scripted test suite, back when Pasky and I invented it, was unknown to many people, would have looked very far away from any day-to-day shell scripts, and are full of magic. Modern versions gained even more magic than the what we had in the olden days. But I hear these days other projects imitate the pattern, so it may no longer so much "magic" to many audiences. If we use a well-written framework for C unit tests, I suspect that the same thing would happen eventually. An important point is the "well-written" part, which needs to hide the ugly implementation details well without revealing it in corners. As Phillip said, a loop construct similar to for() that you are not allowed to break or continue may not qualify exactly for that reason.
On Thu, Jul 25, 2024 at 07:41:19PM -0700, Junio C Hamano wrote: > Kyle Lippincott <spectral@google.com> writes: > > >> > Maybe it's because I'm coming from a C++ environment at > >> > $JOB that's using Google's gunit and gmock frameworks, where every > >> > test is in its own function and we usually don't even write the main > >> > function ourselves, but I have a preference for the separate > >> > functions. > >> > >> If we do not have to write the main at all, then it would make the > >> separate function that implements a single test a lot more palatable, > >> and we do not even have to implement and call TEST() macro ;-). > > ... > > I tried to think of a way to do > > this, and they all involved some other system coming along and > > identifying the tests and code-generating a main function, which also > > seems like too much magic to me. > > I thought that automatically generating the boilerplates from the > visible list of test functions and piecing them together with a > synthetic main was what was brought up as how libgit2 project does > this? It does not sound all that involved and I do not find it a > rocket science. As I said when mentioning how libgit2 does it, I'd be happy to present a working prototype for this if others think it was potentially useful. Until now I didn't hear any positive feedback thoguh, you're the first one saying that this might be something that is worth doing. I just want to avoid wasting time on something that has no chance of landing in the first place. Patrick
Patrick Steinhardt <ps@pks.im> writes: > As I said when mentioning how libgit2 does it, I'd be happy to present a > working prototype for this if others think it was potentially useful. > Until now I didn't hear any positive feedback thoguh, you're the first > one saying that this might be something that is worth doing. > > I just want to avoid wasting time on something that has no chance of > landing in the first place. It takes such a sizeable effort to make it pleasant to use the TEST() thing, it becomes somewhat questionable that it is worth the effort. What I was hoping to see was hinted in the part you did not quote ;-) If there is an existing well-written framework for C unit tests we can apply to our project, it may not have to be so much "magic". If you think theirs is not so good and worth copying, then let's not. Thanks.
On Fri, Jul 26, 2024 at 08:59:00AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > If there is an existing well-written framework for C unit tests we > can apply to our project, it may not have to be so much "magic". If > you think theirs is not so good and worth copying, then let's not. With "theirs" being the libgit2 one? I always enjoyed using it, and it is standalone nowadays and called "clar" [1]. The biggest downside of it is that it depends on Python to auto-generate the test "main" function, which is not really a good fit for the Git project. So ultimately, we'd have to adapt the heart of clar anyway. It's not all that involved, but something to keep in mind if we wanted to use it. Patrick [1]: https://github.com/clar-test/clar
Patrick Steinhardt <ps@pks.im> writes: > is standalone nowadays and called "clar" [1]. The biggest downside of it > is that it depends on Python to auto-generate the test "main" function, > which is not really a good fit for the Git project. Is that because Python is optional (like, we only use it for optional Perforce thing and import-zip in contrib), or are there other concerns? Unlike these components, unit tests are not even for the end-user consumers, so if it is Python that you find it a blocker, I do not see a reason to reject it. The thing looked like a simple script that does not use any tricky language construct or modules.
On Mon, Jul 29, 2024 at 11:55:52AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > is standalone nowadays and called "clar" [1]. The biggest downside of it > > is that it depends on Python to auto-generate the test "main" function, > > which is not really a good fit for the Git project. > > Is that because Python is optional (like, we only use it for > optional Perforce thing and import-zip in contrib), or are there > other concerns? > > Unlike these components, unit tests are not even for the end-user > consumers, so if it is Python that you find it a blocker, I do not > see a reason to reject it. The thing looked like a simple script > that does not use any tricky language construct or modules. No concerns other than adding another mandatory dependency for devs. We already depend on Perl and Shell, so adding Python to the stack feels suboptimal to me. As you say though, the script isn't all that involved. So it would also be possible to port it to Perl if we want to do that, I guess. Patrick
Am 30.07.24 um 06:49 schrieb Patrick Steinhardt: > On Mon, Jul 29, 2024 at 11:55:52AM -0700, Junio C Hamano wrote: >> Patrick Steinhardt <ps@pks.im> writes: >> >>> is standalone nowadays and called "clar" [1]. The biggest downside of it >>> is that it depends on Python to auto-generate the test "main" function, >>> which is not really a good fit for the Git project. >> >> Is that because Python is optional (like, we only use it for >> optional Perforce thing and import-zip in contrib), or are there >> other concerns? >> >> Unlike these components, unit tests are not even for the end-user >> consumers, so if it is Python that you find it a blocker, I do not >> see a reason to reject it. The thing looked like a simple script >> that does not use any tricky language construct or modules. > > No concerns other than adding another mandatory dependency for devs. We > already depend on Perl and Shell, so adding Python to the stack feels > suboptimal to me. > > As you say though, the script isn't all that involved. So it would also > be possible to port it to Perl if we want to do that, I guess. From the point of view of a "minimal C unit testing framework" surely the implementation language with the best dependency story would be C. Perhaps it could then also test itself. On the other hand, writing a string handling program in 2024 in C is probably not the smartest idea. Then again, generate.py uses a non-greedy regex to remove single-line comments, which seems wrong, and doesn't seem to support preprocessor directives like #if and #ifdef, so whole tests can't be disabled this way. And it uses pickle to cache results; does that mean it would be slow without it? Anyway, point being that parsing C isn't so easy using Python either. René
On Tue, Jul 30, 2024 at 04:00:07PM +0200, René Scharfe wrote: > Am 30.07.24 um 06:49 schrieb Patrick Steinhardt: > > On Mon, Jul 29, 2024 at 11:55:52AM -0700, Junio C Hamano wrote: > >> Patrick Steinhardt <ps@pks.im> writes: > >> > >>> is standalone nowadays and called "clar" [1]. The biggest downside of it > >>> is that it depends on Python to auto-generate the test "main" function, > >>> which is not really a good fit for the Git project. > >> > >> Is that because Python is optional (like, we only use it for > >> optional Perforce thing and import-zip in contrib), or are there > >> other concerns? > >> > >> Unlike these components, unit tests are not even for the end-user > >> consumers, so if it is Python that you find it a blocker, I do not > >> see a reason to reject it. The thing looked like a simple script > >> that does not use any tricky language construct or modules. > > > > No concerns other than adding another mandatory dependency for devs. We > > already depend on Perl and Shell, so adding Python to the stack feels > > suboptimal to me. > > > > As you say though, the script isn't all that involved. So it would also > > be possible to port it to Perl if we want to do that, I guess. > > From the point of view of a "minimal C unit testing framework" surely > the implementation language with the best dependency story would be C. > Perhaps it could then also test itself. On the other hand, writing a > string handling program in 2024 in C is probably not the smartest idea. I certainly wouldn't want to do it ;) I think Perl would be perfectly fine given that we already rely on it rather extensively in Git. > Then again, generate.py uses a non-greedy regex to remove single-line > comments, which seems wrong, and doesn't seem to support preprocessor > directives like #if and #ifdef, so whole tests can't be disabled this > way. I guess it doesn't have to be perfect, just good enough. clar comes with some niceties like being able to run only some tests, all tests of a particular suite or to exclude certain tests: # Run only submodule::add tests. $ ./libgit2_tests -t -ssubmodule::add:: TAP version 13 # start of suite 1: submodule::add ok 1 - submodule::add::url_absolute ok 2 - submodule::add::url_relative ok 3 - submodule::add::url_relative_to_origin ok 4 - submodule::add::url_relative_to_workdir ok 5 - submodule::add::path_exists_in_index ok 6 - submodule::add::file_exists_in_index ok 7 - submodule::add::submodule_clone ok 8 - submodule::add::submodule_clone_into_nonempty_dir_succeeds ok 9 - submodule::add::submodule_clone_twice_fails 1..9 # Run only a single test in the submodule::add suite. $ ./libgit2_tests -t -ssubmodule::add::url_absolute TAP version 13 # start of suite 1: submodule::add ok 1 - submodule::add::url_absolute 1..1 # Run all submodule tests, but exclude submodule::add. $ ./libgit2_tests -t -ssubmodule:: -xsubmodule::add:: TAP version 13 # start of suite 1: submodule::escape ok 1 - submodule::escape::from_gitdir ok 2 - submodule::escape::from_gitdir_windows # start of suite 2: submodule::init ok 3 - submodule::init::absolute_url ... So I'm not sure whether it's all that important to be able to recognize things like #if and #ifdef given that you can pick tests to run on the command line. The alternative would be of course to hook up LLVM and properly parse the C sources. But that feels like overthinking it ;) > And it uses pickle to cache results; does that mean it would be > slow without it? Good question. I plan to find some time this week to draft a PoC and will remember to benchmark this. Patrick
Am 31.07.24 um 07:19 schrieb Patrick Steinhardt: > On Tue, Jul 30, 2024 at 04:00:07PM +0200, René Scharfe wrote: >> >> From the point of view of a "minimal C unit testing framework" surely >> the implementation language with the best dependency story would be C. >> Perhaps it could then also test itself. On the other hand, writing a >> string handling program in 2024 in C is probably not the smartest idea. > > I certainly wouldn't want to do it ;) I think Perl would be perfectly > fine given that we already rely on it rather extensively in Git. Right. > clar comes with > some niceties like being able to run only some tests, all tests of a > particular suite or to exclude certain tests: > > # Run only submodule::add tests. > $ ./libgit2_tests -t -ssubmodule::add:: > TAP version 13 > # start of suite 1: submodule::add > ok 1 - submodule::add::url_absolute > ok 2 - submodule::add::url_relative > ok 3 - submodule::add::url_relative_to_origin > ok 4 - submodule::add::url_relative_to_workdir > ok 5 - submodule::add::path_exists_in_index > ok 6 - submodule::add::file_exists_in_index > ok 7 - submodule::add::submodule_clone > ok 8 - submodule::add::submodule_clone_into_nonempty_dir_succeeds > ok 9 - submodule::add::submodule_clone_twice_fails > 1..9 > > # Run only a single test in the submodule::add suite. > $ ./libgit2_tests -t -ssubmodule::add::url_absolute > TAP version 13 > # start of suite 1: submodule::add > ok 1 - submodule::add::url_absolute > 1..1 > > # Run all submodule tests, but exclude submodule::add. > $ ./libgit2_tests -t -ssubmodule:: -xsubmodule::add:: > TAP version 13 > # start of suite 1: submodule::escape > ok 1 - submodule::escape::from_gitdir > ok 2 - submodule::escape::from_gitdir_windows > # start of suite 2: submodule::init > ok 3 - submodule::init::absolute_url > ... > > So I'm not sure whether it's all that important to be able to recognize > things like #if and #ifdef given that you can pick tests to run on the > command line. My only reference are the regular (shell-based) tests, and those have to deal with prerequisites like PTHREADS, PCRE or LIBCURL. Such tests wouldn't even compile, and would thus be #if'd out. We'd have to move those into separate files instead and skip the whole file, it seems. > The alternative would be of course to hook up LLVM and properly parse > the C sources. But that feels like overthinking it ;) It's as simple as calling clang (or gcc) with the option -E. We'd need to disable -Wmissing-prototypes as well, though, as there will be no declarations before we generate clar-decls.h. Not sure how to do that all in a portable way. Anyway, another consequence is that adding, removing or renaming a single test function requires rebuilding all unit tests, because clar-decls.h changes and all of them include it. Not a problem right now. If we ever get to the same number of test files as in the regular test suite (ca. 1000) then this could become annoying. Apropos, I wonder how a Clar-style t-ctype would look like, but didn't dare to even start thinking about it, yet. René
On Wed, Jul 31, 2024 at 06:48:33PM +0200, René Scharfe wrote: > Am 31.07.24 um 07:19 schrieb Patrick Steinhardt: > > On Tue, Jul 30, 2024 at 04:00:07PM +0200, René Scharfe wrote: > Apropos, I wonder how a Clar-style t-ctype would look like, but didn't > dare to even start thinking about it, yet. Oh, I already had it converted locally, but didn't end up sending it out to keep things focussed for now. Below diff is how it could look like. Patrick diff --git a/Makefile b/Makefile index cfdce5770b..ed67feb32e 100644 --- a/Makefile +++ b/Makefile @@ -1332,13 +1332,13 @@ THIRD_PARTY_SOURCES += compat/regex/% THIRD_PARTY_SOURCES += sha1collisiondetection/% THIRD_PARTY_SOURCES += sha1dc/% +UNIT_TESTS_SUITES += ctype 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 UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/unit-test.o -UNIT_TEST_PROGRAMS += t-ctype UNIT_TEST_PROGRAMS += t-example-decorate UNIT_TEST_PROGRAMS += t-hash UNIT_TEST_PROGRAMS += t-mem-pool diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/ctype.c similarity index 70% rename from t/unit-tests/t-ctype.c rename to t/unit-tests/ctype.c index d6ac1fe678..2f6696e828 100644 --- a/t/unit-tests/t-ctype.c +++ b/t/unit-tests/ctype.c @@ -1,18 +1,12 @@ -#include "test-lib.h" +#include "unit-test.h" #define TEST_CHAR_CLASS(class, string) do { \ size_t len = ARRAY_SIZE(string) - 1 + \ BUILD_ASSERT_OR_ZERO(ARRAY_SIZE(string) > 0) + \ BUILD_ASSERT_OR_ZERO(sizeof(string[0]) == sizeof(char)); \ - int skip = test__run_begin(); \ - if (!skip) { \ - for (int i = 0; i < 256; i++) { \ - if (!check_int(class(i), ==, !!memchr(string, i, len)))\ - test_msg(" i: 0x%02x", i); \ - } \ - check(!class(EOF)); \ - } \ - test__run_end(!skip, TEST_LOCATION(), #class " works"); \ + for (int i = 0; i < 256; i++) \ + cl_assert_equal_i(class(i), !!memchr(string, i, len)); \ + cl_assert(!class(EOF)); \ } while (0) #define DIGIT "0123456789" @@ -33,21 +27,72 @@ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \ "\x7f" -int cmd_main(int argc, const char **argv) { +void test_ctype__isspace(void) +{ TEST_CHAR_CLASS(isspace, " \n\r\t"); +} + +void test_TEST_CHAR_ctype__CLASSisdigit(void) +{ TEST_CHAR_CLASS(isdigit, DIGIT); +} + +void test_TEST_CHAR_ctype__CLASSisalpha(void) +{ TEST_CHAR_CLASS(isalpha, LOWER UPPER); +} + +void test_ctype__isalnum(void) +{ TEST_CHAR_CLASS(isalnum, LOWER UPPER DIGIT); +} + +void test_is_glob_ctype__special(void) +{ TEST_CHAR_CLASS(is_glob_special, "*?[\\"); +} + +void test_is_regex_ctype__special(void) +{ TEST_CHAR_CLASS(is_regex_special, "$()*+.?[\\^{|"); +} + +void test_is_pathspec_ctype__magic(void) +{ TEST_CHAR_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~"); +} + +void test_ctype__isascii(void) +{ TEST_CHAR_CLASS(isascii, ASCII); +} + +void test_ctype__islower(void) +{ TEST_CHAR_CLASS(islower, LOWER); +} + +void test_ctype__isupper(void) +{ TEST_CHAR_CLASS(isupper, UPPER); +} + +void test_ctype__iscntrl(void) +{ TEST_CHAR_CLASS(iscntrl, CNTRL); +} + +void test_ctype__ispunct(void) +{ TEST_CHAR_CLASS(ispunct, PUNCT); +} + +void test_ctype__isxdigit(void) +{ TEST_CHAR_CLASS(isxdigit, DIGIT "abcdefABCDEF"); - TEST_CHAR_CLASS(isprint, LOWER UPPER DIGIT PUNCT " "); +} - return test_done(); +void test_ctype__isprint(void) +{ + TEST_CHAR_CLASS(isprint, LOWER UPPER DIGIT PUNCT " "); }
diff --git a/.clang-format b/.clang-format index 6408251577..863dc87dfc 100644 --- a/.clang-format +++ b/.clang-format @@ -151,6 +151,7 @@ Cpp11BracedListStyle: false # function calls. Taken from: # git grep -h '^#define [^[:space:]]*for_\?each[^[:space:]]*(' | # sed "s/^#define / - '/; s/(.*$/'/" | sort | uniq +# Added for_test from t/unit-tests/test-lib.h manually as a special case. ForEachMacros: - 'for_each_builtin' - 'for_each_string_list_item' @@ -168,6 +169,7 @@ ForEachMacros: - 'strintmap_for_each_entry' - 'strmap_for_each_entry' - 'strset_for_each_entry' + - 'for_test' # The maximum number of consecutive empty lines to keep. MaxEmptyLinesToKeep: 1 diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c index 79c12b01cd..5e49fb1e7e 100644 --- a/t/helper/test-example-tap.c +++ b/t/helper/test-example-tap.c @@ -94,5 +94,38 @@ int cmd__example_tap(int argc, const char **argv) test_res = TEST(t_empty(), "test with no checks"); TEST(check_int(test_res, ==, 0), "test with no checks returns 0"); + for_test ("for_test passing test") + check_int(1, ==, 1); + for_test ("for_test failing test") + check_int(1, ==, 2); + for_test ("for_test passing TEST_TODO()") + TEST_TODO(check(0)); + for_test ("for_test failing TEST_TODO()") + TEST_TODO(check(1)); + for_test ("for_test test_skip()") { + check(0); + test_skip("missing prerequisite"); + check(1); + } + for_test ("for_test test_skip() inside TEST_TODO()") + TEST_TODO((test_skip("missing prerequisite"), 1)); + for_test ("for_test TEST_TODO() after failing check") { + check(0); + TEST_TODO(check(0)); + } + for_test ("for_test failing check after TEST_TODO()") { + check(1); + TEST_TODO(check(0)); + check(0); + } + for_test ("for_test messages from failing string and char comparison") { + check_str("\thello\\", "there\"\n"); + check_str("NULL", NULL); + check_char('a', ==, '\n'); + check_char('\\', ==, '\''); + } + for_test ("for_test test with no checks") + ; /* nothing */ + return test_done(); } diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh index fe221f3bdb..5185154414 100755 --- a/t/t0080-unit-test-output.sh +++ b/t/t0080-unit-test-output.sh @@ -50,7 +50,40 @@ test_expect_success 'TAP output from unit tests' - <<\EOT # BUG: test has no checks at t/helper/test-example-tap.c:94 not ok 18 - test with no checks ok 19 - test with no checks returns 0 - 1..19 + ok 20 - for_test passing test + # check "1 == 2" failed at t/helper/test-example-tap.c:100 + # left: 1 + # right: 2 + not ok 21 - for_test failing test + not ok 22 - for_test passing TEST_TODO() # TODO + # todo check 'check(1)' succeeded at t/helper/test-example-tap.c:104 + not ok 23 - for_test failing TEST_TODO() + # check "0" failed at t/helper/test-example-tap.c:106 + # skipping test - missing prerequisite + # skipping check '1' at t/helper/test-example-tap.c:108 + ok 24 - for_test test_skip() # SKIP + # skipping test - missing prerequisite + ok 25 - for_test test_skip() inside TEST_TODO() # SKIP + # check "0" failed at t/helper/test-example-tap.c:113 + not ok 26 - for_test TEST_TODO() after failing check + # check "0" failed at t/helper/test-example-tap.c:119 + not ok 27 - for_test failing check after TEST_TODO() + # check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:122 + # left: "\011hello\\\\" + # right: "there\"\012" + # check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:123 + # left: "NULL" + # right: NULL + # check "'a' == '\n'" failed at t/helper/test-example-tap.c:124 + # left: 'a' + # right: '\012' + # check "'\\\\' == '\\''" failed at t/helper/test-example-tap.c:125 + # left: '\\\\' + # right: '\\'' + not ok 28 - for_test messages from failing string and char comparison + # BUG: test has no checks at t/helper/test-example-tap.c:127 + not ok 29 - for_test test with no checks + 1..29 EOF ! test-tool example-tap >actual && diff --git a/t/unit-tests/test-lib.h b/t/unit-tests/test-lib.h index 2de6d715d5..598c6ff9f3 100644 --- a/t/unit-tests/test-lib.h +++ b/t/unit-tests/test-lib.h @@ -14,6 +14,26 @@ test__run_end(test__run_begin() ? 0 : (t, 1), \ TEST_LOCATION(), __VA_ARGS__) +/* + * Run a test unless test_skip_all() has been called. Acts like a for + * loop that runs at most once, with the test description between the + * parentheses and the test body as a statement or block after them. + * Supports continue to end the test early, but not break. The + * description for each test should be unique. E.g.: + * + * for_test ("something else %d %d", arg1, arg2) { + * prepare(); + * test_something_else(arg1, arg2); + * cleanup(); + * } + */ +#define for_test(...) \ + for (int for_test_running_ = test__run_begin() ? \ + (test__run_end(0, TEST_LOCATION(), __VA_ARGS__), 0) : 1;\ + for_test_running_; \ + test__run_end(1, TEST_LOCATION(), __VA_ARGS__), \ + for_test_running_ = 0) + /* * Print a test plan, should be called before any tests. If the number * of tests is not known in advance test_done() will automatically
The macro TEST only allows defining a test that consists of a single expression. Add a new macro, for_test, which provides a way to define unit tests that are made up of one or more statements. for_test allows defining self-contained tests en bloc, a bit like test_expect_success does for regular tests. It acts like a for loop that runs at most once; the test body is executed if test_skip_all() had not been called before. Signed-off-by: René Scharfe <l.s.r@web.de> --- .clang-format | 2 ++ t/helper/test-example-tap.c | 33 +++++++++++++++++++++++++++++++++ t/t0080-unit-test-output.sh | 35 ++++++++++++++++++++++++++++++++++- t/unit-tests/test-lib.h | 20 ++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) -- 2.45.2