From: Jeff King <peff@peff•net>
To: Patrick Steinhardt <ps@pks•im>
Cc: phillip.wood@dunelm•org.uk, git@vger•kernel.org,
Junio C Hamano <gitster@pobox•com>, Taylor Blau <me@ttaylorr•com>
Subject: Re: my complaints with clar
Date: Fri, 5 Dec 2025 13:30:57 -0500 [thread overview]
Message-ID: <20251205183057.GA33447@coredump.intra.peff.net> (raw)
In-Reply-To: <aTFsA-jJqcRZJs53@pks.im>
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
next prev parent reply other threads:[~2025-12-05 18:30 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-12 7:55 [PATCH 0/9] asan bonanza Jeff King
2025-11-12 7:56 ` [PATCH 1/9] compat/mmap: mark unused argument in git_munmap() Jeff King
2025-11-12 8:01 ` [PATCH 2/9] pack-bitmap: handle name-hash lookups in incremental bitmaps Jeff King
2025-11-12 11:25 ` Patrick Steinhardt
2025-11-13 2:55 ` Taylor Blau
2025-11-18 8:59 ` Jeff King
2025-11-12 8:02 ` [PATCH 3/9] Makefile: turn on NO_MMAP when building with ASan Jeff King
2025-11-12 8:17 ` Collin Funk
2025-11-12 10:31 ` Jeff King
2025-11-12 20:06 ` Collin Funk
2025-11-12 11:26 ` Patrick Steinhardt
2025-11-13 3:12 ` Taylor Blau
2025-11-13 6:34 ` Patrick Steinhardt
2025-11-18 8:49 ` Jeff King
2025-11-13 16:30 ` Junio C Hamano
2025-11-14 7:00 ` Patrick Steinhardt
2025-11-15 2:13 ` Jeff King
2025-11-12 8:05 ` [PATCH 4/9] cache-tree: avoid strtol() on non-string buffer Jeff King
2025-11-12 11:26 ` Patrick Steinhardt
2025-11-13 3:09 ` Taylor Blau
2025-11-18 8:40 ` Jeff King
2025-11-18 8:38 ` Jeff King
2025-11-12 8:06 ` [PATCH 5/9] fsck: assert newline presence in fsck_ident() Jeff King
2025-11-12 8:06 ` [PATCH 6/9] fsck: avoid strcspn() " Jeff King
2025-11-12 8:06 ` [PATCH 7/9] fsck: remove redundant date timestamp check Jeff King
2025-11-12 8:10 ` [PATCH 8/9] fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated Jeff King
2025-11-12 11:25 ` Patrick Steinhardt
2025-11-12 19:36 ` Junio C Hamano
2025-11-15 2:12 ` Jeff King
2025-11-12 8:10 ` [PATCH 9/9] t: enable ASan's strict_string_checks option Jeff King
2025-11-13 3:17 ` [PATCH 0/9] asan bonanza Taylor Blau
2025-11-18 9:11 ` [PATCH v2 " Jeff King
2025-11-18 9:11 ` [PATCH v2 1/9] compat/mmap: mark unused argument in git_munmap() Jeff King
2025-11-18 9:12 ` [PATCH v2 2/9] pack-bitmap: handle name-hash lookups in incremental bitmaps Jeff King
2025-11-18 9:12 ` [PATCH v2 3/9] Makefile: turn on NO_MMAP when building with ASan Jeff King
2025-11-18 9:12 ` [PATCH v2 4/9] cache-tree: avoid strtol() on non-string buffer Jeff King
2025-11-18 14:30 ` Phillip Wood
2025-11-23 6:19 ` Junio C Hamano
2025-11-23 15:51 ` Phillip Wood
2025-11-23 18:06 ` Junio C Hamano
2025-11-24 22:30 ` Jeff King
2025-11-24 23:09 ` Junio C Hamano
2025-11-26 15:09 ` Jeff King
2025-11-26 17:22 ` Junio C Hamano
2025-11-30 13:13 ` [PATCH 0/4] more robust functions for parsing int from buf Jeff King
2025-11-30 13:14 ` [PATCH 1/4] parse: prefer bool to int for boolean returns Jeff King
2025-12-04 11:23 ` Patrick Steinhardt
2025-11-30 13:15 ` [PATCH 2/4] parse: add functions for parsing from non-string buffers Jeff King
2025-11-30 13:46 ` my complaints with clar Jeff King
2025-12-01 14:16 ` Phillip Wood
2025-12-04 11:09 ` Patrick Steinhardt
2025-12-05 18:30 ` Jeff King [this message]
2025-12-04 11:23 ` [PATCH 2/4] parse: add functions for parsing from non-string buffers Patrick Steinhardt
2025-12-05 16:11 ` Phillip Wood
2026-01-20 20:54 ` Junio C Hamano
2026-01-21 5:27 ` Jeff King
2025-11-30 13:15 ` [PATCH 3/4] cache-tree: use parse_int_from_buf() Jeff King
2025-11-30 13:16 ` [PATCH 4/4] fsck: use parse_unsigned_from_buf() for parsing timestamp Jeff King
2025-11-18 9:12 ` [PATCH v2 5/9] fsck: assert newline presence in fsck_ident() Jeff King
2025-11-18 9:12 ` [PATCH v2 6/9] fsck: avoid strcspn() " Jeff King
2025-11-18 9:12 ` [PATCH v2 7/9] fsck: remove redundant date timestamp check Jeff King
2025-11-18 9:12 ` [PATCH v2 8/9] fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated Jeff King
2025-11-18 9:12 ` [PATCH v2 9/9] t: enable ASan's strict_string_checks option Jeff King
2025-11-23 5:49 ` [PATCH v2 0/9] asan bonanza Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251205183057.GA33447@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox