Subject: Re: my complaints with clar

From: Jeff King <[email-masked]>
Date: Fri, 5 Dec 2025 13:30:57 -0500

On Thu, Dec 04, 2025 at 12:09:55PM +0100, Patrick Steinhardt wrote:

> > Patrick's got a PR open for that at
> > https://github.com/clar-test/clar/pull/117 it seems to have got stuck
> > because of a lack of review.
> 
> Yeah, this is indeed a long-standing issue. As Phillip mentioned I've
> already had the fix pending, but I lost track and just never merged it.
> Phillip now left a review, and I've polished the PR a bit. I'll wait a
> few more days before merging it, and then these issues will be a thing
> of the past :)

Great, thanks.

> > ---- 8< ----
> > diff --git a/t/unit-tests/clar/clar/print.h b/t/unit-tests/clar/clar/print.h
> > index 89b66591d75..6a2321b399d 100644
> > --- a/t/unit-tests/clar/clar/print.h
> > +++ b/t/unit-tests/clar/clar/print.h
> > @@ -164,7 +164,7 @@ static void clar_print_tap_ontest(const char
> > *suite_name, const char *test_name,
> >                          printf("      file: '");
> > print_escaped(error->file); printf("'\n");
> >                          printf("      line: %" PRIuMAX "\n",
> > error->line_number);
> >                          printf("      function: '%s'\n", error->function);
> > -                        printf("    ---\n");
> > +                        printf("    ...\n");
> >                  }
> > 
> >                  break;
> > ---- >8 ----
> 
> Indeed, this was a plain bug. I've merged your upstream PR, thanks!
> 
> I'll send a pull request soonish to update our own version of clar.
> 
> Thanks for the feedback! Hope that the pending changes will improve the
> status quo.

It is certainly better for the TAP parser not to choke on the output,
but the more fundamental issue remains that the output is never stored
or relayed anywhere in our CI runs.

I looked at implementing --verbose-log in our unit-tests clar wrapper,
but it's tricky. For one thing, we have to know where to store the
test-results (so understanding $TEST_OUTPUT_DIRECTORY and how it may be
set). Plus, we would then need to duplicate much of the output, going to
both the log and to stdout. And all of the output routines are inside
clar. So we'd need hooks there.

The regular test suite uses "tee" to duplicate the output without the
script having to worry about it. We could use the same trick here.

And an easy way to solve both issues is to just call it from the test
suite, letting it handle --verbose-log itself!

Something like this seems to work:

  #!/bin/sh
  test_description='run clar unit tests'
  . ./test-lib.sh
  exec "$TEST_DIRECTORY/unit-tests/bin/unit-tests" ${immediate:+-i}

We have to "exec" there so that we skip out on the exit handler that
checks we called test_done. And we obviously cannot call that, because
it outputs extra TAP.

But the exec means we also miss some cleanup, like removing our trash
directory.

Probably we'd want a mode in the shell test suite that says "this thing
is going to generate TAP, just stay out of its way". We used to have
test_external, but it was removed in 5beca49a0b (test-lib: simplify by
removing test_external, 2022-07-28). The "modern" alternative is:

  test_expect_success 'run unit-tests' '
	unit-tests/bin/unit-tests
  '

which feels worse (the outer layer of TAP output is all-or-nothing, and
the fact that the unit tests produce TAP is irrelevant). I think it
roughly accomplishes the same thing, but it might be worth looking again
at the reasons for dropping test_external in the first place.

-Peff