| Message ID | 4b27cee7-98eb-41ac-a68b-44f42e15a5d2@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add and use if_test to simplify tests | expand |
On Tue, Jul 30, 2024 at 7:07 AM René Scharfe <l.s.r@web.de> wrote: > > Checks outside of tests are caught at runtime and reported like this: > > Assertion failed: (ctx.running), function test_assert, file test-lib.c, line 267. > > The assert() call aborts the unit test and doesn't reveal the location > or even the type of the offending check, as test_assert() is called by > all of them. > > Handle it like the opposite case, a test without any checks: Don't > abort, but report the location of the actual check, along with a message > explaining the situation. The output for example above becomes: > > # BUG: check outside of test at t/helper/test-example-tap.c:75 > > ... and the unit test program continues and indicates the error in its > exit code at the end. > > Signed-off-by: René Scharfe <l.s.r@web.de> > --- > t/helper/test-example-tap.c | 2 ++ > t/t0080-unit-test-output.sh | 5 +++-- > t/unit-tests/test-lib.c | 7 ++++++- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c > index d072ad559f..79c12b01cd 100644 > --- a/t/helper/test-example-tap.c > +++ b/t/helper/test-example-tap.c > @@ -72,6 +72,8 @@ static void t_empty(void) > > int cmd__example_tap(int argc, const char **argv) > { > + check(1); Let's include a comment that describes why we have this outside of the TEST() macros so that people don't try to "fix" it, and so that people realize it's not meant to be a _good_ example :) > + > test_res = TEST(check_res = check_int(1, ==, 1), "passing test"); > TEST(t_res(1), "passing test and assertion return 1"); > test_res = TEST(check_res = check_int(1, ==, 2), "failing test"); > diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh > index 9ec47b7360..fe221f3bdb 100755 > --- a/t/t0080-unit-test-output.sh > +++ b/t/t0080-unit-test-output.sh > @@ -7,9 +7,10 @@ TEST_PASSES_SANITIZE_LEAK=true > > test_expect_success 'TAP output from unit tests' - <<\EOT > cat >expect <<-EOF && > + # BUG: check outside of test at t/helper/test-example-tap.c:75 > ok 1 - passing test > ok 2 - passing test and assertion return 1 > - # check "1 == 2" failed at t/helper/test-example-tap.c:77 > + # check "1 == 2" failed at t/helper/test-example-tap.c:79 > # left: 1 > # right: 2 > not ok 3 - failing test > @@ -46,7 +47,7 @@ test_expect_success 'TAP output from unit tests' - <<\EOT > # left: '\\\\' > # right: '\\'' > not ok 17 - messages from failing string and char comparison > - # BUG: test has no checks at t/helper/test-example-tap.c:92 > + # 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 > diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c > index 3c513ce59a..989dc758e6 100644 > --- a/t/unit-tests/test-lib.c > +++ b/t/unit-tests/test-lib.c > @@ -264,7 +264,12 @@ static void test_todo(void) > > int test_assert(const char *location, const char *check, int ok) > { > - assert(ctx.running); > + if (!ctx.running) { > + test_msg("BUG: check outside of test at %s", > + make_relative(location)); Below, `test_msg` emits a message like `skipping check '1 == 2' at <loc>`. Should we include 'check' as part of the message here, or is it not possible or not useful for some reason? > + ctx.failed = 1; > + return 0; > + } > > if (ctx.result == RESULT_SKIP) { > test_msg("skipping check '%s' at %s", check, > -- > 2.46.0
Am 31.07.24 um 23:03 schrieb Kyle Lippincott: > On Tue, Jul 30, 2024 at 7:07 AM René Scharfe <l.s.r@web.de> wrote: >> >> diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c >> index d072ad559f..79c12b01cd 100644 >> --- a/t/helper/test-example-tap.c >> +++ b/t/helper/test-example-tap.c >> @@ -72,6 +72,8 @@ static void t_empty(void) >> >> int cmd__example_tap(int argc, const char **argv) >> { >> + check(1); > > Let's include a comment that describes why we have this outside of the > TEST() macros so that people don't try to "fix" it, and so that people > realize it's not meant to be a _good_ example :) Well, the other examples have no such comments, either, but they are tests (using TEST or if_test), so they at least have descriptions. So fair enough. >> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c >> index 3c513ce59a..989dc758e6 100644 >> --- a/t/unit-tests/test-lib.c >> +++ b/t/unit-tests/test-lib.c >> @@ -264,7 +264,12 @@ static void test_todo(void) >> >> int test_assert(const char *location, const char *check, int ok) >> { >> - assert(ctx.running); >> + if (!ctx.running) { >> + test_msg("BUG: check outside of test at %s", >> + make_relative(location)); > > Below, `test_msg` emits a message like `skipping check '1 == 2' at > <loc>`. Should we include 'check' as part of the message here, or is > it not possible or not useful for some reason? It's possible, of course. Didn't do it because I didn't consider that we might have multiple checks per line and the complementary "test has no checks" message only mentions the line number. The latter is followed by the test description in the next line, though. So yeah, good point. > >> + ctx.failed = 1; >> + return 0; >> + } >> >> if (ctx.result == RESULT_SKIP) { >> test_msg("skipping check '%s' at %s", check, >> -- >> 2.46.0
diff --git a/t/helper/test-example-tap.c b/t/helper/test-example-tap.c index d072ad559f..79c12b01cd 100644 --- a/t/helper/test-example-tap.c +++ b/t/helper/test-example-tap.c @@ -72,6 +72,8 @@ static void t_empty(void) int cmd__example_tap(int argc, const char **argv) { + check(1); + test_res = TEST(check_res = check_int(1, ==, 1), "passing test"); TEST(t_res(1), "passing test and assertion return 1"); test_res = TEST(check_res = check_int(1, ==, 2), "failing test"); diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh index 9ec47b7360..fe221f3bdb 100755 --- a/t/t0080-unit-test-output.sh +++ b/t/t0080-unit-test-output.sh @@ -7,9 +7,10 @@ TEST_PASSES_SANITIZE_LEAK=true test_expect_success 'TAP output from unit tests' - <<\EOT cat >expect <<-EOF && + # BUG: check outside of test at t/helper/test-example-tap.c:75 ok 1 - passing test ok 2 - passing test and assertion return 1 - # check "1 == 2" failed at t/helper/test-example-tap.c:77 + # check "1 == 2" failed at t/helper/test-example-tap.c:79 # left: 1 # right: 2 not ok 3 - failing test @@ -46,7 +47,7 @@ test_expect_success 'TAP output from unit tests' - <<\EOT # left: '\\\\' # right: '\\'' not ok 17 - messages from failing string and char comparison - # BUG: test has no checks at t/helper/test-example-tap.c:92 + # 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 diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c index 3c513ce59a..989dc758e6 100644 --- a/t/unit-tests/test-lib.c +++ b/t/unit-tests/test-lib.c @@ -264,7 +264,12 @@ static void test_todo(void) int test_assert(const char *location, const char *check, int ok) { - assert(ctx.running); + if (!ctx.running) { + test_msg("BUG: check outside of test at %s", + make_relative(location)); + ctx.failed = 1; + return 0; + } if (ctx.result == RESULT_SKIP) { test_msg("skipping check '%s' at %s", check,
Checks outside of tests are caught at runtime and reported like this: Assertion failed: (ctx.running), function test_assert, file test-lib.c, line 267. The assert() call aborts the unit test and doesn't reveal the location or even the type of the offending check, as test_assert() is called by all of them. Handle it like the opposite case, a test without any checks: Don't abort, but report the location of the actual check, along with a message explaining the situation. The output for example above becomes: # BUG: check outside of test at t/helper/test-example-tap.c:75 ... and the unit test program continues and indicates the error in its exit code at the end. Signed-off-by: René Scharfe <l.s.r@web.de> --- t/helper/test-example-tap.c | 2 ++ t/t0080-unit-test-output.sh | 5 +++-- t/unit-tests/test-lib.c | 7 ++++++- 3 files changed, 11 insertions(+), 3 deletions(-) -- 2.46.0