On Tue, Jul 30, 2024 at 7:07 AM René Scharfe <l.s.r@xxxxxx> 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@xxxxxx>
> ---
> 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
