patch, 4 partsmeson: parse TAP output generated by our tests

Hi,

this patch series starts to parse TAP output generated by our tests when executing them via Meson. This has the benefit that Meson starts to understand skipped tests and reports how many subtests have been executed:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed
    
    Ok:                9
    Fail:              0
    Skipped:           1
    ```

This new feature is only enabled with Meson 1.8 and newer, which contains a bugfix that we have upstreamed [1] to make the TAP parser work in `meson test --interactive` mode.

Despite the changes to Meson itself, this patch series also contains a couple of fixes for our test suite that caused us to not generate proper TAP output.

Thanks!

Patrick

[1]: https://github.com/mesonbuild/meson/pull/13980

---
Patrick Steinhardt (4):
      t: fix cases where output breaks TAP format
      t/test-lib: don't print shell traces to stdout
      meson: introduce kwargs variable for tests
      meson: parse TAP output generated by our tests

 contrib/credential/netrc/meson.build         |  2 +-
 contrib/subtree/meson.build                  |  2 +-
 meson.build                                  | 12 ++++++++++
 t/meson.build                                |  6 ++---
 t/t0000-basic.sh                             | 35 +++++++++++++++-------------
 t/t1007-hash-object.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t7401-submodule-summary.sh                 |  4 ++--
 t/t9500-gitweb-standalone-no-errors.sh       | 14 +++++------
 t/test-lib.sh                                |  4 ++--
 11 files changed, 51 insertions(+), 36 deletions(-)

--- base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75 change-id: 20250429-pks-meson-tap-1eed604a02a3

The TAP format does not allow arbitrary output outside of a specific test case. If a test suite wants to print any such diagnostic output, then this output has to be prefixed with "#" to mark it accordingly. A bunch of our tests generate output outside of `test_expect_*` testcases anyway without such a mark, which breaks strict TAP parsers.

Upon further inspection, all of the output generated by such tests is rather uninteresting. Refactor them so that we don't break the TAP format.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1007-hash-object.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t7401-submodule-summary.sh                 |  4 ++--
 t/t9500-gitweb-standalone-no-errors.sh       | 14 +++++++-------
 5 files changed, 13 insertions(+), 13 deletions(-)

Hi Patrick

On 06/05/2025 11:59, Patrick Steinhardt wrote:

I think there is an argument that these tests are broken and we should be running these commands inside test_expect_success(). However this patch doesn't make things substantially worse because although we lose the output from test_create_repo that probably isn't going to matter. The changes to the highlighting prereq look fine too.

Best Wishes

Phillip

On Tue, May 06, 2025 at 02:17:09PM +0100, Phillip Wood wrote:

Yeah, agreed, our modern style when writing tests should always use `test_expect_success()` indeed. So an alternative to this commit would thus be to use `test_expect_success()` as you propose. Let me know your preference, I'm happy to adapt if you think this is preferable.

Patrick

On 07/05/2025 07:52, Patrick Steinhardt wrote:

If you feel like re-rolling using test_expect_success() that would definitely be an improvement but I don't think your patch makes things worse than they already are so don't feel you have to.

Thanks

Phillip

Patrick Steinhardt <ps@pks.im> writes:

This is different because `test_expect_*` sets up the stdout and stderr redirection to new file descriptors, but any code outside that wouldn't have that redirection. Okay.

The changes make sense WRT to the description.

We have several flags like "--verbose", "--verbose-only" or "-x" that cause us to generate shell traces. The generated tracing output is split up in these cases so that the test's stdout is printed to file descriptor 3 whereas its stderr is printed to file descriptor 4. Depending on which options have been given, we then end up either:

  - Redirecting both file descriptors to a file.

  - Redirecting them to stdout and stderr, respectively.

  - Closing them in case we're running in none-verbose mode.

The second case causes problems though when passing output to a TAP parser. We print the test's stdout to the console's stdout, and that results in broken TAP output.

Fix the issue by instead redirecting the test's stdout to the shell's stderr. This makes it impossible to discern stdout from stderr, but going by my own experience I never came across a usecase where I would have needed this distinction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 35 +++++++++++++++++++----------------
 t/test-lib.sh    |  4 ++--
 2 files changed, 21 insertions(+), 18 deletions(-)

Meson has the ability to create a kwargs dictionary that can then be passed to any function call with the `kwargs:` positional argument. This allows one to deduplicate common parameters that one wishes to pass to several different function invocations.

Our tests already have one common parameter that we use everywhere, "timeout", and we're about to add a second common parameter in the next commit. Let's prepare for this by introducing `test_kwargs` so that we can deduplicate these common arguments.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 2 +-
 contrib/subtree/meson.build          | 2 +-
 meson.build                          | 4 ++++
 t/meson.build                        | 6 +++---
 4 files changed, 9 insertions(+), 5 deletions(-)

Patrick Steinhardt <ps@pks.im> writes:

This is a nice cleanup and a good to know. Looks good.

[snip]

By default, Meson only knows to pay respect to the exit code of tests to judge whether or not it ran successfully. This can be changed though by specifying the "protocol" parameter. Next to the default "exitcode" protocol, Meson also supports the "tap" output that our tests already know to generate.

Unfortunately, the "tap" protocol was incompatible with `meson test --interactive` and caused a hang. We have upstreamed a fix [1] though, so with the recent release of Meson 1.8 that fix is finally out and we can start using the "tap" protocol when running with a recent-enough version of this build tool.

With this change in place, Meson now properly detects how many subtests ran and whether test suites have been skipped:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed

    Ok:                9
    Fail:              0
    Skipped:           1
    ```

[1]: https://github.com/mesonbuild/meson/pull/13980

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 8 ++++++++
 1 file changed, 8 insertions(+)

Patrick Steinhardt <ps@pks.im> writes:

The change itself looks good. But I do have a question about this:

  $ meson -version
  1.8.99

  $ meson test --interactive t3206-range-diff
  ninja: Entering directory `/home/karthik/code/git/build'
  [1/28] Generating GIT-VERSION-FILE with a custom command (wrapped by
meson to set env)
  1/1 t3206-range-diff RUNNING
  >>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
MESON_TEST_ITERATION=1 MALLOC_PERTURB_=119
GIT_BUILD_DIR=/home/karthik/code/git/build
ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1
UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
/nix/store/xg75pc4yyfd5n2fimhb98ps910q5lm5n-bash-5.2p37/bin/sh
t3206-range-diff.sh
  ok 1 - setup
  ok 2 - simple A..B A..C (unmodified)
  ok 3 - simple B...C (unmodified)
  ok 4 - simple A B C (unmodified)
  ok 5 - simple A..B A..C (unmodified) with --abbrev
  ok 6 - A^! and A^-<n> (unmodified)
  ok 7 - A^{/..} is not mistaken for a range
  ok 8 - trivial reordering
  ok 9 - removed a commit
  ok 10 - added a commit
  ok 11 - new base, A B C
  ok 12 - new base, B...C
  ok 13 - changed commit
  ok 14 - changed commit with --no-patch diff option
  ok 15 - changed commit with --stat diff option
  ok 16 - changed commit with sm config
  ok 17 - renamed file
  ok 18 - file with mode only change
  ok 19 - file added and later removed
  ok 20 - no commits on one side
  ok 21 - changed message
  ok 22 - dual-coloring
  ok 23 - format-patch --range-diff=topic
  ok 24 - format-patch --range-diff=main..topic
  ok 25 - --range-diff implies --cover-letter for multi-patch series
  ok 26 - explicit --no-cover-letter defeats implied --cover-letter
  ok 27 - format-patch --range-diff as commentary
  ok 28 - format-patch --range-diff reroll-count with a non-integer
  ok 29 - format-patch --range-diff reroll-count with a integer
  ok 30 - format-patch --range-diff with v0
  ok 31 - range-diff overrides diff.noprefix internally
  ok 32 - basic with modified format.pretty with suffix
  ok 33 - basic with modified format.pretty without "commit "
  ok 34 - range-diff compares notes by default
  ok 35 - range-diff with --no-notes
  ok 36 - range-diff with multiple --notes
  ok 37 - range-diff with --notes=custom does not show default notes
  ok 38 - format-patch --range-diff does not compare notes by default
  ok 39 - format-patch --notes=custom --range-diff only compares custom notes
  ok 40 - format-patch --range-diff with --no-notes
  ok 41 - format-patch --range-diff with --notes
  ok 42 - format-patch --range-diff with format.notes config
  ok 43 - format-patch --range-diff with multiple notes
  ok 44 - --left-only/--right-only
  ok 45 - ranges with pathspecs
  ok 46 - submodule changes are shown irrespective of diff.submodule
  ok 47 - --diff-merges
  # passed all 47 test(s)
  1..47
  1/1 t3206-range-diff IGNORED         1.76s

  Ok:                0
  Fail:              0
  Ignored:           1

  $ meson test t3206-range-diff
  ninja: Entering directory `/home/karthik/code/git/build'
  [1/28] Generating GIT-VERSION-FILE with a custom command (wrapped by
meson to set env)
  1/1 t3206-range-diff        OK              1.70s   47 subtests passed

  Ok:                1
  Fail:              0

  Full log written to
  /home/karthik/code/git/build/meson-logs/testlog.txt

Shouldn't the '--interactive' flag also produce 'Ok: 1'. Instead it is printing out 'Ignored: 1'. This is while I was testing on your series. Seems to be fine on master.

On Thu, May 15, 2025 at 12:48:23AM -0700, Karthik Nayak wrote:

[snip]

The answer is unfortunately "no". In interactive mode the expectation is that the user will, well, interact with the test. This has two consequences:

  - The standard streams will be directly connected to the user's
    console. This has the consequence that Meson won't be able to parse
    the generated output anymore, and thus it labels the tests as
    "ignored" because it cannot derive their status.

  - Even if Meson intercepted the output it very likely wouldn't be able
    to parse it. After all, we're in interactive mode, which means that
    the user may be directly communicating with the tests. E.g. if you
    use `test_pause`, then you'll be dropped to a shell and communicate
    with it. The consequence is that the output won't follow proper TAP
    format anymore.

So it's basically impossible to parse TAP in interactive mode, which is why Meson then ignores the results. If all you want is to see verbose output you can do that with `meson test -v`. But interactive mode really indicates that you _want_ to get hands-on with the tests.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

Okay I understand the constraints here, thanks for the explanation. I think the series looks good to me with this! :)

On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:

Please hold off with merging this to "seen" just yet. I have missed that this introduces issues with MinGW, which I want to have a look at first before resubmitting. I didn't see those issues in a previous iteration, so I'm not sure whether it was introduced by this series or not.

Patrick

On Tue, May 06, 2025 at 02:29:35PM +0200, Patrick Steinhardt wrote:

Hm, this seems to have just been a CI flake. No idea why all of these MinGW jobs failed all at once, but a rerun of the pipeline fixed all of them.

Patrick

On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:

Junio, I noticed that this series isn't yet part of the "What's cooking" report. Is that intentional or an oversight?

Thanks!

Patrick

Hridoy Ahmed Hy I am back

Patrick Steinhardt <ps@pks.im> writes:

Neither. I saw a lively discussion on the patches and was expecting to see a finalized updated version, which I would apply.

The "Please hold off" message in the middle did not help X-<.

Junio C Hamano <gitster@pobox.com> writes:

So the four patches are now sitting somewhere in 'seen'. Is it the one that causes this failure, I have to wonder?

https://github.com/git/git/actions/runs/15169816296/job/42656836511#step:4:2113

It is curious that only osx-meson is affected.

Thanks.

On Wed, May 21, 2025 at 02:26:23PM -0700, Junio C Hamano wrote:

There wasn't any discussion that led to something actionable as far as I'm concerned, which is why there wasn't a newer revision yet.

Ah, interesting. Seems like macOS has since updated to a newer version of Meson, so it now uses the TAP parser. And there are some tests that only execute on macOS and that cause us to emit output to stdout/stderr, which will thus break the TAP format.

I'll have a look and will send a newer version soonish.

Patrick

On Fri, May 23, 2025 at 12:03:24PM +0200, Patrick Steinhardt wrote:

Okay, the problem actually isn't the TAP format -- Meson copes with the broken output, but I'll fix it regardless in the next version of this patch series. The problem is that we have a test that unexpectedly passes on macOS:

    ▶  868/1023 - git grep .fi a                             UNEXPECTEDPASS

The test in question is this one:

    test_expect_failure !CYGWIN 'git grep .fi a' '
        git grep .fi a
    '

The test passes if '.' matches a NUL byte, which we expect to only happen on Cygwin. 064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7, 2025-04-17) mentions that this behaviour was probably imported from FreeBSD, which makes me wonder whether macOS eventually also inherited the same code given its BSD lineage.

I think we probably want something like the below patch to fix this. We could also have a prereq, but that prereq would look almost the exact same as the test. It does make me question the value of the test itself as the behaviour is completely platform specific.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

Yup, I was wondering about the same thing. Thanks for confirming. It is unfortunate that we have blanket USES_BSD_REGEXP prerequisite ;-)

Curious.

Don't we run the same set of tests on macOS without Meson? the exact same test must be passing unexpectedly. Why do we see the complaint on only osx-meson job without osx-{clang,reftable,gcc} jobs?

On 23/05/2025 16:58, Junio C Hamano wrote:

Yep, I very nearly sent that patch with a hunk which deleted that test. In the end, I decided to play it more conservatively. :)

Yes, for about a decade, git built on cygwin with the system regex library would have shown an 'unexpected pass' on this test. However, unless you look for it, you wouldn't know; make and prove are quite happy with the situation, since it is an unexpected _pass_:

  $ git diff
  diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
  index b7d83f9a5d..3bd91da970 100755
  --- a/t/t7815-grep-binary.sh
  +++ b/t/t7815-grep-binary.sh
  @@ -63,7 +63,7 @@ test_expect_success 'git grep ile a' '
          git grep ile a
   '
   
  -test_expect_failure !CYGWIN 'git grep .fi a' '
  +test_expect_failure 'git grep .fi a' '
          git grep .fi a
   '
   
  $ cd t
  $ ./t7815-grep-binary.sh
  ok 1 - setup
  ok 2 - git grep ina a
  ok 3 - git grep -ah ina a
  ok 4 - git grep -I ina a
  ok 5 - git grep -c ina a
  ok 6 - git grep -l ina a
  ok 7 - git grep -L bar a
  ok 8 - git grep -q ina a
  ok 9 - git grep -F ile a
  ok 10 - git grep -Fi iLE a
  ok 11 - git grep ile a
  not ok 12 - git grep .fi a # TODO known breakage
  ok 13 - grep respects binary diff attribute
  ok 14 - grep --cached respects binary diff attribute
  ok 15 - grep --cached respects binary diff attribute (2)
  ok 16 - grep revision respects binary diff attribute
  ok 17 - grep respects not-binary diff attribute
  ok 18 - setup textconv filters
  ok 19 - grep does not honor textconv
  ok 20 - grep --textconv honors textconv
  ok 21 - grep --no-textconv does not honor textconv
  ok 22 - grep --textconv blob honors textconv
  # still have 1 known breakage(s)
  # passed all remaining 21 test(s)
  1..22
  $ echo $?
  0
  $ 

Again, prove doesn't care:

  $ prove t7815-grep-binary.sh
  t7815-grep-binary.sh .. ok    
  All tests successful.
  Files=1, Tests=22,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.09 cusr  0.13 csys =  0.25 CPU)
  Result: PASS
  $ echo $?
  0
  $ 
 
When I tested the patch (without !CYGWIN) with 'meson test' it didn't care
either - that was *before* Patrick's recent patch to make meson parse TAP
output. ;)

Question: should meson (or indeed prove) fail the test because of an
unexpected _pass_?

ATB, Ramsay Jones

On 23/05/2025 17:40, Ramsay Jones wrote:

Sigh! Of course, when I did the above I was on Linux! It looks like the following on cygwin:

  $ ./t7815-grep-binary.sh
  ok 1 - setup
  ok 2 - git grep ina a
  ok 3 - git grep -ah ina a
  ok 4 - git grep -I ina a
  ok 5 - git grep -c ina a
  ok 6 - git grep -l ina a
  ok 7 - git grep -L bar a
  ok 8 - git grep -q ina a
  ok 9 - git grep -F ile a
  ok 10 - git grep -Fi iLE a
  ok 11 - git grep ile a
  ok 12 - git grep .fi a # TODO known breakage vanished
  ok 13 - grep respects binary diff attribute
  ok 14 - grep --cached respects binary diff attribute
  ok 15 - grep --cached respects binary diff attribute (2)
  ok 16 - grep revision respects binary diff attribute
  ok 17 - grep respects not-binary diff attribute
  ok 18 - setup textconv filters
  ok 19 - grep does not honor textconv
  ok 20 - grep --textconv honors textconv
  ok 21 - grep --no-textconv does not honor textconv
  ok 22 - grep --textconv blob honors textconv
  # 1 known breakage(s) vanished; please update test(s)
  # passed all remaining 21 test(s)
  1..22
  $ echo $?
  0
  $ 

Note that #12 is 'ok' and '... please update test(s)'

Sorry about that!

ATB, Ramsay Jones

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Yes, it is a very good question. I do not mind if the answer is "it should, and the make and prove shouldn't let unexpected pass go unnoticed". The difference between the build systems bothers me

Thanks.

On Fri, May 23, 2025 at 12:33:23PM -0700, Junio C Hamano wrote:

Indeed, a good question. The TAP specification [1] has this to say:

    Should a todo test point begin succeeding, the harness may report it
    in some way that indicates that whatever was supposed to be done has
    been, and it should be promoted to a normal Test Point.

    Harnesses must not treat failing TODO test points as a test failure.

    Harneses should report TODO test points found as a list of items
    needing work, if that is appropriate for their use case.

So if my reading of this is correct then Meson isn't wrong in reporting this as an error -- "in some way" basically gives it full permission to do so. So this is plain old undefined behaviour we rely on :/

I don't think it's inherently a bad thing to fail on unexpected passes. After all, it shows that our assumption that the test fails is broken, and that we should have a look why that is. But I can see arguments both ways.

The pragmatic solution would be to just fix the unexpected pass and then proceed with wiring up TAP support.

Patrick

https://testanything.org/tap-version-14-specification.html

On 26/05/2025 13:44, Patrick Steinhardt wrote:

Personally I'd be very happy if our test suite failed on an unexpected pass. Currently it is easy to miss, especially if the unexpected pass occurs in a CI run. Missing an unexpected pass means we don't change 'test_expect_failure' to 'test_expect_pass' and a future regression that causes the test to fail again will go unnoticed.

Best Wishes

Phillip

Phillip Wood wrote:

Indeed. Perhaps related (apologies if it's a wild tangent), having a way to expose an unexpectedly failed prereq would be nice.

For example, we currently (well, last time I checked, which was a month or so ago) fail the GPG2 prereq. I submitted small patch series to fix that nearly a year ago¹, but when I ran the tests in our CI, they turned up some preexisting failures.

I spent a little time trying to reproduce and resolve the failures, but was never able to make it work. These tests pass when run locally which makes it painful to track down.

It would have been ideal if they failed when added, so that it could have been worked out during the review period, while it was fresh in the minds of the folks working in that area.

We are simply not noticing these failures in our CI, which feels worse than simply not having test coverage. It gives a false of security that t/t1016-compatObjectFormat.sh is passing. In reality, the tests might be reporting a real issue that we've been missing for ages.

¹ <20240703153738.916469-1-tmz@pobox.com>

On 5/26/25 8:44 AM, Patrick Steinhardt wrote:

I don't see how you can possibly compare this to UB. It is a documented variant behavior and thus "implementation-defined", not "undefined".

Also,

https://www.gnu.org/software/automake/manual/automake.html#Generalities-about-Testing

""" It’s not uncommon, especially during early development stages, that some tests fail for known reasons, and that the developer doesn’t want to tackle these failures immediately (this is especially true when the failing tests deal with corner cases). In this situation, the better policy is to declare that each of those failures is an expected failure (or xfail). In case a test that is expected to fail ends up passing instead, many testing environments will flag the result as a special kind of failure called unexpected pass (or xpass). """

As Phillip noted, treating them as ordinary passes undermines the reason for having them.

On Mon, May 26, 2025 at 09:54:40AM -0400, Eli Schwartz wrote:

Fair enough, "implementation-defined" is the better way to put it.

Yup, and I tend to agree.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

OK. So perhaps Make-driven CI jobs also follow suit? In the same run that osx-meson job failed, osx-gcc job notices a passed TODO and happily declares "All tests successful.".

https://github.com/git/git/actions/runs/15221271947/job/42817168362#step:4:1933

On Tue, May 27, 2025 at 09:04:55AM -0700, Junio C Hamano wrote:

prove itself doesn't treat unexpected passes as errors, and I couldn't find an option to adapt it. We can do something like the below patch, which makes us fail in case there are any fixed tests. The only downside is that prove will report test failures like this:

    t1400-update-ref.sh ................................ Dubious, test returned 1 (wstat 256, 0x100)
    All 299 subtests passed
        (1 TODO test unexpectedly succeeded)

It's not great because prove reports it as dubious. But it isn't all that bad either because we directly mention the unexpected success for one of the tests.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

So, what should happen to succeeding TODO is left unspecified.

Hi,

this patch series starts to parse TAP output generated by our tests when executing them via Meson. This has the benefit that Meson starts to understand skipped tests and reports how many subtests have been executed:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed
    
    Ok:                9
    Fail:              0
    Skipped:           1
    ```

This new feature is only enabled with Meson 1.8 and newer, which contains a bugfix that we have upstreamed [1] to make the TAP parser work in `meson test --interactive` mode.

Despite the changes to Meson itself, this patch series also contains a couple of fixes for our test suite that caused us to not generate proper TAP output.

Changes in v2:
  - Add a patch to fix an unexpectedly passing test on macOS.
  - A couple more fixes for broken TAP output.
  - Link to v1: https://lore.kernel.org/r/20250506-pks-meson-tap-v1-0-5aaab2942a4c@pks.im

Note that I've rebased the series on top of 845c48a16a7 (The seventeenth batch, 2025-05-23), mostly so that I get hold of the recent CI changes for GitLab that make the MSVC+Meson run unconditionally. There was no conflicts though, so it should be fine to retain the old merge base.

Thanks!

Patrick

[1]: https://github.com/mesonbuild/meson/pull/13980

---
Patrick Steinhardt (6):
      t: fix cases where output breaks TAP format
      t/test-lib: don't print shell traces to stdout
      t/test-lib: fix TAP format for BASH_XTRACEFD warning
      t7815: fix unexpectedly passing test on macOS
      meson: introduce kwargs variable for tests
      meson: parse TAP output generated by our tests

 contrib/credential/netrc/meson.build         |  2 +-
 contrib/subtree/meson.build                  |  2 +-
 meson.build                                  | 12 ++++++++++
 t/meson.build                                |  6 ++---
 t/t0000-basic.sh                             | 35 +++++++++++++++-------------
 t/t0050-filesystem.sh                        |  5 ----
 t/t1007-hash-object.sh                       |  2 +-
 t/t3600-rm.sh                                |  5 ----
 t/t4000-diff-format.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t7401-submodule-summary.sh                 |  4 ++--
 t/t7815-grep-binary.sh                       |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh       | 14 +++++------
 t/t9822-git-p4-path-encoding.sh              | 13 +++++++----
 t/t9835-git-p4-metadata-encoding-python2.sh  |  4 ++--
 t/t9836-git-p4-metadata-encoding-python3.sh  |  4 ++--
 t/t9903-bash-prompt.sh                       |  4 ----
 t/test-lib.sh                                |  9 ++++---
 19 files changed, 70 insertions(+), 61 deletions(-)

Range-diff versus v1:

1:  8ffeaa53d63 ! 1:  022430e0434 t: fix cases where output breaks TAP format
    @@ Commit message
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    + ## t/t0050-filesystem.sh ##
    +@@ t/t0050-filesystem.sh: aumlcdiar=$(printf '\141\314\210')
    + 
    + if test_have_prereq CASE_INSENSITIVE_FS
    + then
    +-	say "will test on a case insensitive filesystem"
    + 	test_case=test_expect_failure
    + else
    + 	test_case=test_expect_success
    +@@ t/t0050-filesystem.sh: fi
    + 
    + if test_have_prereq UTF8_NFD_TO_NFC
    + then
    +-	say "will test on a unicode corrupting filesystem"
    + 	test_unicode=test_expect_failure
    + else
    + 	test_unicode=test_expect_success
    + fi
    + 
    +-test_have_prereq SYMLINKS ||
    +-	say "will test on a filesystem lacking symbolic links"
    +-
    + if test_have_prereq CASE_INSENSITIVE_FS
    + then
    + test_expect_success "detection of case insensitive filesystem during repo init" '
    +
      ## t/t1007-hash-object.sh ##
     @@ t/t1007-hash-object.sh: setup_repo() {
      
    @@ t/t1007-hash-object.sh: setup_repo() {
      
      	setup_repo
     
    + ## t/t3600-rm.sh ##
    +@@ t/t3600-rm.sh: test_expect_success 'Initialize test directory' '
    + 	git commit -m "add normal files"
    + '
    + 
    +-if test_have_prereq !FUNNYNAMES
    +-then
    +-	say 'Your filesystem does not allow tabs in filenames.'
    +-fi
    +-
    + test_expect_success FUNNYNAMES 'add files with funny names' '
    + 	touch -- "tab	embedded" "newline${LF}embedded" &&
    + 	git add -- "tab	embedded" "newline${LF}embedded" &&
    +
    + ## t/t4000-diff-format.sh ##
    +@@ t/t4000-diff-format.sh: test_expect_success 'git diff-files -p after editing work tree.' '
    + # that's as far as it comes
    + if [ "$(git config --get core.filemode)" = false ]
    + then
    +-	say 'filemode disabled on the filesystem'
    ++	skip_all='filemode disabled on the filesystem'
    + 	test_done
    + fi
    + 
    +
      ## t/t4041-diff-submodule-option.sh ##
     @@ t/t4041-diff-submodule-option.sh: commit_file () {
      	git commit "$@" -m "Commit $*" >/dev/null
    @@ t/t9500-gitweb-standalone-no-errors.sh: test_expect_success \
      	cat >>gitweb_config.perl <<-\EOF
      	our $highlight_bin = "highlight";
      	$feature{'highlight'}{'override'} = 1;
    +
    + ## t/t9822-git-p4-path-encoding.sh ##
    +@@ t/t9822-git-p4-path-encoding.sh: test_description='Clone repositories with non ASCII paths'
    + UTF8_ESCAPED="a-\303\244_o-\303\266_u-\303\274.txt"
    + ISO8859_ESCAPED="a-\344_o-\366_u-\374.txt"
    + 
    +-ISO8859="$(printf "$ISO8859_ESCAPED")" &&
    +-echo content123 >"$ISO8859" &&
    +-rm "$ISO8859" || {
    ++test_lazy_prereq FS_ACCEPTS_ISO_8859_1 '
    ++	ISO8859="$(printf "$ISO8859_ESCAPED")" &&
    ++	echo content123 >"$ISO8859" 2>/dev/null &&
    ++	rm "$ISO8859"
    ++'
    ++
    ++if ! test_have_prereq FS_ACCEPTS_ISO_8859_1
    ++then
    + 	skip_all="fs does not accept ISO-8859-1 filenames"
    + 	test_done
    +-}
    ++fi
    + 
    + test_expect_success 'start p4d' '
    + 	start_p4d
    +
    + ## t/t9835-git-p4-metadata-encoding-python2.sh ##
    +@@ t/t9835-git-p4-metadata-encoding-python2.sh: failing, and produces maximally sane output in git.'
    + # These tests are specific to Python 2. Write a custom script that executes
    + # git-p4 directly with the Python 2 interpreter to ensure that we use that
    + # version even if Git was compiled with Python 3.
    +-python_target_binary=$(which python2)
    ++python_target_binary=$(which python2 2>/dev/null)
    + if test -n "$python_target_binary"
    + then
    + 	mkdir temp_python
    +@@ t/t9835-git-p4-metadata-encoding-python2.sh: then
    + fi
    + 
    + git p4-python2 >err
    +-if ! grep 'valid commands' err
    ++if ! grep -q 'valid commands' err
    + then
    + 	skip_all="skipping python2 git p4 tests; python2 not available"
    + 	test_done
    +
    + ## t/t9836-git-p4-metadata-encoding-python3.sh ##
    +@@ t/t9836-git-p4-metadata-encoding-python3.sh: failing, and produces maximally sane output in git.'
    + # These tests are specific to Python 3. Write a custom script that executes
    + # git-p4 directly with the Python 3 interpreter to ensure that we use that
    + # version even if Git was compiled with Python 2.
    +-python_target_binary=$(which python3)
    ++python_target_binary=$(which python3 2>/dev/null)
    + if test -n "$python_target_binary"
    + then
    + 	mkdir temp_python
    +@@ t/t9836-git-p4-metadata-encoding-python3.sh: then
    + fi
    + 
    + git p4-python3 >err
    +-if ! grep 'valid commands' err
    ++if ! grep -q 'valid commands' err
    + then
    + 	skip_all="skipping python3 git p4 tests; python3 not available"
    + 	test_done
    +
    + ## t/t9903-bash-prompt.sh ##
    +@@ t/t9903-bash-prompt.sh: test_expect_success 'prompt - unborn branch' '
    + 	test_cmp expected "$actual"
    + '
    + 
    +-if test_have_prereq !FUNNYNAMES; then
    +-	say 'Your filesystem does not allow newlines in filenames.'
    +-fi
    +-
    + test_expect_success FUNNYNAMES 'prompt - with newline in path' '
    +     repo_with_newline="repo
    + with
2:  2d6afea4853 = 2:  35e2b7f3a08 t/test-lib: don't print shell traces to stdout
-:  ----------- > 3:  996920aa372 t/test-lib: fix TAP format for BASH_XTRACEFD warning
-:  ----------- > 4:  ae584ae3400 t7815: fix unexpectedly passing test on macOS
3:  d2ba541c2f6 = 5:  5cfbf9dde27 meson: introduce kwargs variable for tests
4:  73649d77893 ! 6:  d242cde7831 meson: parse TAP output generated by our tests
    @@ Commit message
             Skipped:           1
             ```
     
    +    Note that when running `meson test --interactive` the test results will
    +    now be marked as "ignored". This is because in interactive mode the file
    +    descriptors will remain connected to the user's terminal, and it is
    +    expected that the user interacts with the tests (e.g., spawn a debugger
    +    or use `test_pause`). As such, the TAP output cannot be parsed reliably
    +    by Meson in that case, so the tests are marked as ignored accordingly.
    +
         [1]: https://github.com/mesonbuild/meson/pull/13980
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>

--- base-commit: 845c48a16a7f7b2c44d8cb137b16a4a1f0140229 change-id: 20250429-pks-meson-tap-1eed604a02a3

The TAP format does not allow arbitrary output outside of a specific test case. If a test suite wants to print any such diagnostic output, then this output has to be prefixed with "#" to mark it accordingly. A bunch of our tests generate output outside of `test_expect_*` testcases anyway without such a mark, which breaks strict TAP parsers.

Upon further inspection, all of the output generated by such tests is rather uninteresting. Refactor them so that we don't break the TAP format.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0050-filesystem.sh                        |  5 -----
 t/t1007-hash-object.sh                       |  2 +-
 t/t3600-rm.sh                                |  5 -----
 t/t4000-diff-format.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t7401-submodule-summary.sh                 |  4 ++--
 t/t9500-gitweb-standalone-no-errors.sh       | 14 +++++++-------
 t/t9822-git-p4-path-encoding.sh              | 13 +++++++++----
 t/t9835-git-p4-metadata-encoding-python2.sh  |  4 ++--
 t/t9836-git-p4-metadata-encoding-python3.sh  |  4 ++--
 t/t9903-bash-prompt.sh                       |  4 ----
 12 files changed, 27 insertions(+), 36 deletions(-)

On Tue, May 27, 2025 at 10:03 AM Patrick Steinhardt <ps@pks.im> wrote:

Nit: Can we avoid the word "refactor" for changes such as those made
by this patch which clearly are not refactoring[*].

[*]: From Wikipedia: "... code refactoring is the process of restructuring existing source code—changing the factoring—without changing its external behavior."

Yuck, but certainly the simplest "fix" in this particular case considering that, ultimately, this entire script ought to be reworked since it cd's around outside of tests with abandon. It would be nice to see this script get overhauled eventually but such an undertaking doesn't need to be part of this patch series.

Unlike the case with t1007, in which the entire script needs an overhaul, it is much easier to fix the problems in this script without papering over them via ">/dev/null". In particular, it would be preferable to resolve the issue by wrapping test_expect_success around the code which currently resides outside of any test. So, for example, the above could become:

    test_expect_success 'setup submodule 1' '
        test_create_repo sm1 &&
        add_file . foo &&
        head1=$(add_file sm1 foo1 foo2) &&
        fullhead1=$(cd sm1; git rev-parse --verify HEAD)
    '

Note that I also dropped the ">/dev/null" redirect from the add_file() invocation.

The same comment applies to similar changes made by this patch to other scripts, such as t4060, t7401.

Was the problem here that the `echo content123 > "$..."` was potentially spitting out an error message to stderr, thus you had to redirect it to /dev/null to silence it? If so, did the file get created in the error case? What I'm wondering is whether you also should use `rm -f` when removing the file.

On Tue, May 27, 2025 at 03:47:16PM -0400, Eric Sunshine wrote:

Fair. We can say "adapt" instead.

Yeah, a bunch of test scripts fall into this category indeed.

Yes, it isn't particularly hard. But it does result in a bunch of shuffling that makes the patch way harder to read.

Ah, this redirect is not required anymore. I had it in a previous version due to the exact problem that you mentioned, that echo spit out an error.

The idea here is that some systems fail to create the file in the first place, which will cause the echo to fail. In that case, the file has not been created either, so there is no need to remove it.

Patrick

On Wed, May 28, 2025 at 11:55 AM Patrick Steinhardt <ps@pks.im> wrote:

I'm not sure why such a change would require shuffling code around (or perhaps I misunderstand the idea you are trying to convey). Unlike t1007 which needs a major overhaul, each block of code which currently exists outside of test_expect_success in this script can simply be wrapped in test_expect_success (with a distinct "setup whatever" title) in situ without shuffling the code around. Yes, such changes would be noisy, but it would be very localized noise in each case, thus not particularly difficult to review.

As such, since such a fix is so simple (even if a bit noisy) I'd rather see it done properly rather than merely papering over the problem. However, I'm just one reviewer; others, including yourself, may feel differently.

Okay. I'm perfectly fine with you turning this into a lazy prereq -- it's cleaner, more modern, and possibly easier to understand as a prereq -- so no problem there. I asked about it merely because I didn't understand why it was included in this patch, and the commit message didn't explain its presence. It would do very well as a separate patch, either within this series or standalone.

On Wed, May 28, 2025 at 04:14:17PM -0400, Eric Sunshine wrote:

I was mostly shying away from this because it's hard to argue why I'm doing these cleanups in one test file, but not in others. And the reindents to make the diff way noisier. Anyway, I've split up this commit into multiple now and did some more refactorings to make this a bit more palatable.

I'll send the revised version to the mailing list later today.

Thanks!

Patrick

We have several flags like "--verbose", "--verbose-only" or "-x" that cause us to generate shell traces. The generated tracing output is split up in these cases so that the test's stdout is printed to file descriptor 3 whereas its stderr is printed to file descriptor 4. Depending on which options have been given, we then end up either:

  - Redirecting both file descriptors to a file.

  - Redirecting them to stdout and stderr, respectively.

  - Closing them in case we're running in none-verbose mode.

The second case causes problems though when passing output to a TAP parser. We print the test's stdout to the console's stdout, and that results in broken TAP output.

Fix the issue by instead redirecting the test's stdout to the shell's stderr. This makes it impossible to discern stdout from stderr, but going by my own experience I never came across a usecase where I would have needed this distinction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 35 +++++++++++++++++++----------------
 t/test-lib.sh    |  4 ++--
 2 files changed, 21 insertions(+), 18 deletions(-)

Patrick Steinhardt <ps@pks.im> writes:

OK, so both stdout and stderr go to stderr, mixing everything into a single stream. Do we need to worry about funny buffering making the test output harder to verify? I mean, we only have to care about the ordering of lines within the original standard output (or standard error) stream independently, but now if the test thinks it wrote A to its stderr, then B to its stdout, and then C to its stderr, would we get them in the single output stream as A followed by B followed by C, or can sometimes buffered output can give us A then C then finally B?

Just an idle thought. What makes me more confused is that the updated t0000 tests seem to say that we now check standard output and standard error separately.

On Tue, May 27, 2025 at 12:47:00PM -0700, Junio C Hamano wrote:

Good question -- I think that the answer is yes, this can indeed cause intermingled output. The redirect to the best of my knowledge would not impact the buffering behaviour of stdout/stderr. I'm not really sure how to address this though.

To stdout goes any output that is expected to be parsed in the TAP format, so all the "ok 1" or "not ok 2" lines. Everything else goes to stderr.

Patrick

When the Bash version is too old to support BASH_XTRACEFD we print a warning to stderr. This warning breaks the TAP format because it is not prefixed with a "#". Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

In t7815, we have the following test:

    test_expect_failure !CYGWIN 'git grep .fi a' '
        git grep .fi a
    '

The test passes if '.' matches a NUL byte, which we expect to only happen on Cygwin. The upcoming changes to support parsing TAP output in Meson surface that this test is also unexpectedly passing on macOS though.

It is unclear how long the test has been passing on macOS already. 064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7, 2025-04-17) mentions that the test started to pass for Cygwin once it has imported a newer implementation of regcomp(3p) et all, which was inherited from FreeBSD. Given the BSD lineage of macOS it is feasible that it also inherited similar code eventually that made the test pass now.

It is somewhat dubious what the test actually brings to the table given that it is quite platform specific. Ideally, we would fix this mess by having a configure-time check whether regcomp(3p) works as expected, including NUL bytes, and use our bundled version of the regex library in case it doesn't. Like this, we could ensure that all platforms work the same in this edge case and mark the new behaviour as expected.

This change is outside of the scope of this patch series, which only introduce support for TAP. So instead of fixing the bigger issue, ignore the test on Darwin like we already do for Cygwin.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7815-grep-binary.sh | 2 +-
 t/test-lib.sh          | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

By default, Meson only knows to pay respect to the exit code of tests to judge whether or not it ran successfully. This can be changed though by specifying the "protocol" parameter. Next to the default "exitcode" protocol, Meson also supports the "tap" output that our tests already know to generate.

Unfortunately, the "tap" protocol was incompatible with `meson test --interactive` and caused a hang. We have upstreamed a fix [1] though, so with the recent release of Meson 1.8 that fix is finally out and we can start using the "tap" protocol when running with a recent-enough version of this build tool.

With this change in place, Meson now properly detects how many subtests ran and whether test suites have been skipped:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed

    Ok:                9
    Fail:              0
    Skipped:           1
    ```

Note that when running `meson test --interactive` the test results will now be marked as "ignored". This is because in interactive mode the file descriptors will remain connected to the user's terminal, and it is expected that the user interacts with the tests (e.g., spawn a debugger or use `test_pause`). As such, the TAP output cannot be parsed reliably by Meson in that case, so the tests are marked as ignored accordingly.

[1]: https://github.com/mesonbuild/meson/pull/13980

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 8 ++++++++
 1 file changed, 8 insertions(+)

Meson has the ability to create a kwargs dictionary that can then be passed to any function call with the `kwargs:` positional argument. This allows one to deduplicate common parameters that one wishes to pass to several different function invocations.

Our tests already have one common parameter that we use everywhere, "timeout", and we're about to add a second common parameter in the next commit. Let's prepare for this by introducing `test_kwargs` so that we can deduplicate these common arguments.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 2 +-
 contrib/subtree/meson.build          | 2 +-
 meson.build                          | 4 ++++
 t/meson.build                        | 6 +++---
 4 files changed, 9 insertions(+), 5 deletions(-)

Hi,

this patch series starts to parse TAP output generated by our tests when executing them via Meson. This has the benefit that Meson starts to understand skipped tests and reports how many subtests have been executed:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed
    
    Ok:                9
    Fail:              0
    Skipped:           1
    ```

This new feature is only enabled with Meson 1.8 and newer, which contains a bugfix that we have upstreamed [1] to make the TAP parser work in `meson test --interactive` mode.

Despite the changes to Meson itself, this patch series also contains a couple of fixes for our test suite that caused us to not generate proper TAP output.

Changes in v2:
  - Add a patch to fix an unexpectedly passing test on macOS.
  - A couple more fixes for broken TAP output.
  - Link to v1: https://lore.kernel.org/r/20250506-pks-meson-tap-v1-0-5aaab2942a4c@pks.im

Changes in v3:
  - Split up the patch that silences output into multiple patches and
    rework them a bit.
  - Remove redirect that was retained by accident from an earlier
    version.
  - Slight rewording of a commit message.
  - Treat unexpected passes as failure in prove(1) and when executing
    the test directly.
  - Link to v2: https://lore.kernel.org/r/20250527-pks-meson-tap-v2-0-ae360f77786e@pks.im

Thanks!

Patrick

[1]: https://github.com/mesonbuild/meson/pull/13980

---
Patrick Steinhardt (10):
      t: stop announcing prereqs
      t: silence output from `test_create_repo()`
      t9822: use prereq to check for ISO-8859-1 support
      t983*: use prereq to check for Python-specific git-b4(1) support
      t/test-lib: don't print shell traces to stdout
      t/test-lib: fix TAP format for BASH_XTRACEFD warning
      t7815: fix unexpectedly passing test on macOS
      test-lib: fail on unexpectedly passing tests
      meson: introduce kwargs variable for tests
      meson: parse TAP output generated by our tests

 contrib/credential/netrc/meson.build         |  2 +-
 contrib/subtree/meson.build                  |  2 +-
 meson.build                                  | 12 +++++++++
 t/meson.build                                |  6 ++---
 t/t0000-basic.sh                             | 39 +++++++++++++++-------------
 t/t0050-filesystem.sh                        | 30 +++++----------------
 t/t1007-hash-object.sh                       |  2 +-
 t/t3600-rm.sh                                |  5 ----
 t/t4000-diff-format.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             | 22 +++++++++-------
 t/t4060-diff-submodule-option-diff-format.sh |  9 ++++---
 t/t7401-submodule-summary.sh                 | 18 ++++++++-----
 t/t7815-grep-binary.sh                       |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh       | 16 +++++-------
 t/t9822-git-p4-path-encoding.sh              | 13 +++++++---
 t/t9835-git-p4-metadata-encoding-python2.sh  | 24 +++++++++--------
 t/t9836-git-p4-metadata-encoding-python3.sh  | 24 +++++++++--------
 t/t9903-bash-prompt.sh                       |  4 ---
 t/test-lib.sh                                | 18 ++++++++++---
 19 files changed, 133 insertions(+), 117 deletions(-)

Range-diff versus v2:

 1:  b5ae3aba1ad <  -:  ----------- t: fix cases where output breaks TAP format
 -:  ----------- >  1:  15702b96125 t: stop announcing prereqs
 -:  ----------- >  2:  444a5e8a72f t: silence output from `test_create_repo()`
 -:  ----------- >  3:  dd24e16f93c t9822: use prereq to check for ISO-8859-1 support
 -:  ----------- >  4:  5ea96164181 t983*: use prereq to check for Python-specific git-b4(1) support
 2:  69d4b420eb9 =  5:  783cf673a22 t/test-lib: don't print shell traces to stdout
 3:  ec921bbb183 =  6:  726795c9a11 t/test-lib: fix TAP format for BASH_XTRACEFD warning
 4:  e463fb29a8a =  7:  df1be586474 t7815: fix unexpectedly passing test on macOS
 -:  ----------- >  8:  e0b06f9ffcb test-lib: fail on unexpectedly passing tests
 5:  58173827436 =  9:  0f34503b28b meson: introduce kwargs variable for tests
 6:  1242bbf74f3 = 10:  d28b9306b46 meson: parse TAP output generated by our tests

--- base-commit: 845c48a16a7f7b2c44d8cb137b16a4a1f0140229 change-id: 20250429-pks-meson-tap-1eed604a02a3

We have a couple of cases where our tests end up announcing that a certain prerequisite is or isn't fulfilled. While this is supposed to help the developer it has the downside that it breaks the TAP format.

We could convert these cases to just have a "#" prefix, but it feels rather unlikely that these are generally useful in the first place. We already do announce why a specific test is being skipped, so we should try to use this mechanism to the best extent possible.

Stop announcing these prereqs to fix the TAP format. Where possible, convert the tests to rely on the prerequisites themselves to announce why a test ran or didn't ran.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0050-filesystem.sh                  | 30 ++++++------------------------
 t/t3600-rm.sh                          |  5 -----
 t/t4000-diff-format.sh                 |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 16 +++++++---------
 t/t9903-bash-prompt.sh                 |  4 ----
 5 files changed, 14 insertions(+), 43 deletions(-)

Patrick Steinhardt <ps@pks.im> writes:

So `test_case` seems to be defined here, but never used in this file, so we can remove this whole block. Okay.

Here `test_unicode` is actually used in two tests, so we cannot remove it. We actually want to assert that the tests fails I assume, otherwise we could just modify the tests to simply have a prereq on `UTF8_NFD_TO_NFC`.

This is a no-op debug message which can be removed.

Okay, so here we remove the unnecessary 'if...else' statement and directly check the prereq.

[snip]

Okay this is a bit different, we set a new prereq which are used in tests below. Previously we checked the exit status of the command, now we check if there is an output. This shouldn't matter.

Nice ;)

[snip]

Tests in t9822 depend on filesystem support for ISO-8859-1 encoding. We thus have a block of code that acts as a prerequisite -- if we fail to write a file with an ISO-8859-1-encoded file name to disk then we skip all tests.

When the prerequisite fails though we end up printing an error message to stderr, which breaks the TAP format. Fix this by converting the code to a proper prerequisite, which handles output redirection for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9822-git-p4-path-encoding.sh | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

There are a couple users of `test_create_repo()` that use this function outside of any test case. This function is nowadays only a thin wrapper around `git init`, which by default prints a message to stdout that the repository has been initialized. The resulting output may thus confuse TAP parsers.

Refactor these users to instead create the repository in a "setup" test case so that we don't explicitly have to silence them. There's one exception in t1007: we use `push_repo()` and its `pop_repo()` equivalent multiple times, so to reduce the noise introduced by this patch we instead silence this invocation.

While at it, convert callsites to use git-init(1) directly as the `test_create_repo()` function has been deprecated in f0d4d398e28 (test-lib: split up and deprecate test_create_repo(), 2021-05-10).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1007-hash-object.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             | 22 +++++++++++++---------
 t/t4060-diff-submodule-option-diff-format.sh |  9 ++++++---
 t/t7401-submodule-summary.sh                 | 18 +++++++++++-------
 4 files changed, 31 insertions(+), 20 deletions(-)

On Fri, May 30, 2025 at 9:31 AM Patrick Steinhardt <ps@pks.im> wrote:

Nit: We now have two tests with identical titles ("setup submodule")
in this script. [1] and [2] suggested using distinct titles. Don't
know if this is worth a reroll, though.

[1]: https://lore.kernel.org/git/CAPig+cSYhY+LQ5pD+a1O16Rxwo_js45WqfcW8wtC2daYmNyMCQ@mail.gmail.com/ [2]: https://lore.kernel.org/git/CAPig+cR+eham=uSamUFmTuWDhZ6_r3dnMm1z+X25aw2K6maN7w@mail.gmail.com/

The tests in t9835 and t9836 verify that git-b4(1) works with both Python 2 and 3, respectively. To determine whether we have those Python versions in the first place we create a wrapper script that directly executes the git-b4(1) script with `python2` or `python3` binaries. We then condition the execution of tests on whether that wrapper script can be executed successfully.

The logic that does all of this is not contained in a prerequisite block though, so the output it generates causes us to break the TAP format. Refactor the logic to use `test_lazy_prereq()` to fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9835-git-p4-metadata-encoding-python2.sh | 24 +++++++++++++-----------
 t/t9836-git-p4-metadata-encoding-python3.sh | 24 +++++++++++++-----------
 2 files changed, 26 insertions(+), 22 deletions(-)

Patrick Steinhardt wrote:

s/b4/p4/ in the commit subject and message.

You did make me wonder if someone had added some sort of b4 integration into git though. :)

Tangentially, with Python 2 having gone EOL upstream in January 2022¹, how long does it make sense to keep supporting it?

Debian Bullseye² and RHEL 8³ both have python 3.9. Are there systems in our targeted support matrix which do _not_ have python3?

It's not something to fix for this patch, but perhaps if we no longer feel obligated to support python2, someone may choose to remove it from the code and simplify it a bit.

¹ https://www.python.org/doc/sunset-python-2/
² https://packages.debian.org/search?keywords=python3 (even
  buster, AKA old, old stable has python 3.7)
³ https://git.rockylinux.org/staging/src-rhel/rpms/python39
  (RHEL makes finding a decent URL a pain)

On Fri, May 30, 2025 at 10:08:15AM -0400, Todd Zullinger wrote:

D'oh. I use b4 all the time, which probably explains this.

I don't think so.

I wouldn't mind. We still have test coverage of Python 2 via the Ubuntu 20.04 CI job, so at least it's still getting tested. But that is not a good enough reason to actually keep it around.

Anyway, fixed locally. I'll wait a bit for additional feedback though before sending a new version. Thanks!

Patrick

We have several flags like "--verbose", "--verbose-only" or "-x" that cause us to generate shell traces. The generated tracing output is split up in these cases so that the test's stdout is printed to file descriptor 3 whereas its stderr is printed to file descriptor 4. Depending on which options have been given, we then end up either:

  - Redirecting both file descriptors to a file.

  - Redirecting them to stdout and stderr, respectively.

  - Closing them in case we're running in none-verbose mode.

The second case causes problems though when passing output to a TAP parser. We print the test's stdout to the console's stdout, and that results in broken TAP output.

Fix the issue by instead redirecting the test's stdout to the shell's stderr. This makes it impossible to discern stdout from stderr, but going by my own experience I never came across a usecase where I would have needed this distinction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 35 +++++++++++++++++++----------------
 t/test-lib.sh    |  4 ++--
 2 files changed, 21 insertions(+), 18 deletions(-)

Patrick Steinhardt <ps@pks.im> writes:

Fair enough. I wonder how we can retain this distinction, perhaps write stdout to user provided file. Or add some prefix to distinguish the two when output to stderr. Nevertheless, this should be fine.

The changes itself look good to me.

[snip]

When the Bash version is too old to support BASH_XTRACEFD we print a warning to stderr. This warning breaks the TAP format because it is not prefixed with a "#". Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patrick Steinhardt <ps@pks.im> writes:

Nit: The important bit here is that TAP treats anything that starts with a
'#' as a comment. Everything else is parsed. Since this is warning which
shouldn't be parsed, we add the '#'. Perhaps worthwhile adding this
extra context to the commit. Anyways not worth a re-roll.

[snip]

In t7815, we have the following test:

    test_expect_failure !CYGWIN 'git grep .fi a' '
        git grep .fi a
    '

The test passes if '.' matches a NUL byte, which we expect to only happen on Cygwin. The upcoming changes to support parsing TAP output in Meson surface that this test is also unexpectedly passing on macOS though.

It is unclear how long the test has been passing on macOS already. 064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7, 2025-04-17) mentions that the test started to pass for Cygwin once it has imported a newer implementation of regcomp(3p) et all, which was inherited from FreeBSD. Given the BSD lineage of macOS it is feasible that it also inherited similar code eventually that made the test pass now.

It is somewhat dubious what the test actually brings to the table given that it is quite platform specific. Ideally, we would fix this mess by having a configure-time check whether regcomp(3p) works as expected, including NUL bytes, and use our bundled version of the regex library in case it doesn't. Like this, we could ensure that all platforms work the same in this edge case and mark the new behaviour as expected.

This change is outside of the scope of this patch series, which only introduce support for TAP. So instead of fixing the bigger issue, ignore the test on Darwin like we already do for Cygwin.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7815-grep-binary.sh | 2 +-
 t/test-lib.sh          | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Patrick Steinhardt <ps@pks.im> writes:

Nit: s/introduce/introduces

The change makes sense to me.

[snip]

On Fri, May 30, 2025, at 15:31, Patrick Steinhardt wrote:

This last sentence was difficult for me when I first read it. It seems that there are multiple verb tenses and it has many words without any pauses. Maybe consider restructuring with some comma breaks or something.

    The upcoming changes to support parsing TAP output in Meson is
    showing that this test, suprisingly, passes on macOS as well.

“started to pass” followed by “has imported” doesn’t sound right.

s/et all,/et al.,/

On Sun, Jun 01, 2025 at 11:19:21AM +0200, Kristoffer Haugsbakk wrote:

Hm, why exactly is that? To me it reads perfectly fine, which might be caused by me not being a native speaker. Anyway, reworded this to the following:

    It is unclear how long the test has been passing on macOS already.
    064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7,
    2025-04-17) mentions that the test started to pass for Cygwin. This was
    attributed to a new implementation of regcomp(3p) and friends, which was
    inherited from FreeBSD. Given the BSD lineage of macOS it is feasible
    that it also inherited similar code eventually that made the test pass
    now.

Hope that works better. Thanks!

Patrick

When tests are executed via `test_expect_failure` we rather obviously expect the test itself to fail. If it unexpectedly does not fail then we count the test as a "fixed" test and announce that a known breakage has vanished:

    ok 1 - setup
    ok 2 - create refs/heads/main # TODO known breakage vanished
    ok 3 - create refs/heads/main with oldvalue verification
    ...
    ok 299 - update-ref should also create reflog for HEAD
    # 1 known breakage(s) vanished; please update test(s)
    # passed all remaining 298 test(s)
    1..299

While we announce that tests should be updated, the overall test suite still passes. This makes it quite hard to detect when a test that has previously failed succeeds now as the developer needs to pay close attention to the exact output. Even more importantly, tests that only succeed on _some_ systems are even easier to miss now, as one would have to explicitly take a look at respective CI jobs to notice that those do pass now.

Furthermore, we are about to introduce support for parsing TAP output in Meson. In contrast to prove(1), which treats unexpected passes as a successful test run, Meson treats those as failure. Neither of these tools is wrong in doing so. Quoting the TAP specification [1]:

    Should a todo test point begin succeeding, the harness may report it
    in some way that indicates that whatever was supposed to be done has
    been, and it should be promoted to a normal Test Point.

So it is essentially implementation-defined how exactly the unexpected pass is reported, and whether it should cause the overall test suite to fail or not. It is unarguably a bad thing for us though if these tools interpret these differently, as it would mean that test results now depend on whether the developer uses prove(1) or Meson.

Unify the behaviour by causing a test suite to fail when there are any unexpected passes. As prove(1) does not consider an unexpected pass to be an error this leads to somewhat funky output:

    t1400-update-ref.sh ................................ Dubious, test returned 1 (wstat 256, 0x100)
    All 299 subtests passed
            (1 TODO test unexpectedly succeeded)

    ...

    Test Summary Report
    -------------------
    t1400-update-ref.sh                              (Wstat: 256 (exited 1) Tests: 299 Failed: 0)
      TODO passed:   2
      Non-zero exit status: 1

But as we directly announce that the root cause is an unexpected TODO that has succeeded it's not all that bad.

[1]: https://testanything.org/tap-version-14-specification.html

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 4 ++--
 t/test-lib.sh    | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

Meson has the ability to create a kwargs dictionary that can then be passed to any function call with the `kwargs:` positional argument. This allows one to deduplicate common parameters that one wishes to pass to several different function invocations.

Our tests already have one common parameter that we use everywhere, "timeout", and we're about to add a second common parameter in the next commit. Let's prepare for this by introducing `test_kwargs` so that we can deduplicate these common arguments.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 2 +-
 contrib/subtree/meson.build          | 2 +-
 meson.build                          | 4 ++++
 t/meson.build                        | 6 +++---
 4 files changed, 9 insertions(+), 5 deletions(-)

By default, Meson only knows to pay respect to the exit code of tests to judge whether or not it ran successfully. This can be changed though by specifying the "protocol" parameter. Next to the default "exitcode" protocol, Meson also supports the "tap" output that our tests already know to generate.

Unfortunately, the "tap" protocol was incompatible with `meson test --interactive` and caused a hang. We have upstreamed a fix [1] though, so with the recent release of Meson 1.8 that fix is finally out and we can start using the "tap" protocol when running with a recent-enough version of this build tool.

With this change in place, Meson now properly detects how many subtests ran and whether test suites have been skipped:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed

    Ok:                9
    Fail:              0
    Skipped:           1
    ```

Note that when running `meson test --interactive` the test results will now be marked as "ignored". This is because in interactive mode the file descriptors will remain connected to the user's terminal, and it is expected that the user interacts with the tests (e.g., spawn a debugger or use `test_pause`). As such, the TAP output cannot be parsed reliably by Meson in that case, so the tests are marked as ignored accordingly.

[1]: https://github.com/mesonbuild/meson/pull/13980

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 8 ++++++++
 1 file changed, 8 insertions(+)

Patrick Steinhardt <ps@pks.im> writes:

Apart from some small nits from my side, I think the patch series looks good.

- Karthik

[snip]

Hi,

this patch series starts to parse TAP output generated by our tests when executing them via Meson. This has the benefit that Meson starts to understand skipped tests and reports how many subtests have been executed:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed
    
    Ok:                9
    Fail:              0
    Skipped:           1
    ```

This new feature is only enabled with Meson 1.8 and newer, which contains a bugfix that we have upstreamed [1] to make the TAP parser work in `meson test --interactive` mode.

Despite the changes to Meson itself, this patch series also contains a couple of fixes for our test suite that caused us to not generate proper TAP output.

Changes in v2:
  - Add a patch to fix an unexpectedly passing test on macOS.
  - A couple more fixes for broken TAP output.
  - Link to v1: https://lore.kernel.org/r/20250506-pks-meson-tap-v1-0-5aaab2942a4c@pks.im

Changes in v3:
  - Split up the patch that silences output into multiple patches and
    rework them a bit.
  - Remove redirect that was retained by accident from an earlier
    version.
  - Slight rewording of a commit message.
  - Treat unexpected passes as failure in prove(1) and when executing
    the test directly.
  - Link to v2: https://lore.kernel.org/r/20250527-pks-meson-tap-v2-0-ae360f77786e@pks.im

Changes in v4:
  - Fix references to git-p4(1), which was mistyped as git-b4(1).
  - A couple of commit message improvements.
  - Avoid duplicate test names in t4041.
  - Link to v3: https://lore.kernel.org/r/20250530-pks-meson-tap-v3-0-676f5e41f2e4@pks.im

Thanks!

Patrick

[1]: https://github.com/mesonbuild/meson/pull/13980

---
Patrick Steinhardt (10):
      t: stop announcing prereqs
      t: silence output from `test_create_repo()`
      t9822: use prereq to check for ISO-8859-1 support
      t983*: use prereq to check for Python-specific git-p4(1) support
      t/test-lib: don't print shell traces to stdout
      t/test-lib: fix TAP format for BASH_XTRACEFD warning
      t7815: fix unexpectedly passing test on macOS
      test-lib: fail on unexpectedly passing tests
      meson: introduce kwargs variable for tests
      meson: parse TAP output generated by our tests

 contrib/credential/netrc/meson.build         |  2 +-
 contrib/subtree/meson.build                  |  2 +-
 meson.build                                  | 12 +++++++++
 t/meson.build                                |  6 ++---
 t/t0000-basic.sh                             | 39 +++++++++++++++-------------
 t/t0050-filesystem.sh                        | 30 +++++----------------
 t/t1007-hash-object.sh                       |  2 +-
 t/t3600-rm.sh                                |  5 ----
 t/t4000-diff-format.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             | 22 +++++++++-------
 t/t4060-diff-submodule-option-diff-format.sh |  9 ++++---
 t/t7401-submodule-summary.sh                 | 18 ++++++++-----
 t/t7815-grep-binary.sh                       |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh       | 16 +++++-------
 t/t9822-git-p4-path-encoding.sh              | 13 +++++++---
 t/t9835-git-p4-metadata-encoding-python2.sh  | 24 +++++++++--------
 t/t9836-git-p4-metadata-encoding-python3.sh  | 24 +++++++++--------
 t/t9903-bash-prompt.sh                       |  4 ---
 t/test-lib.sh                                | 18 ++++++++++---
 19 files changed, 133 insertions(+), 117 deletions(-)

Range-diff versus v3:

 1:  0e9aac3c63f =  1:  05d16d9d7ac t: stop announcing prereqs
 2:  2c3bd12eb5d !  2:  ed6a8b205f0 t: silence output from `test_create_repo()`
    @@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(sub
     -test_create_repo sm1 &&
     -head6=$(add_file sm1 foo6 foo7)
     -fullhead6=$(cd sm1; git rev-parse --verify HEAD)
    -+test_expect_success 'setup submodule' '
    ++test_expect_success 'setup submodule anew' '
     +	rm -f sm1 &&
     +	git init sm1 &&
     +	head6=$(add_file sm1 foo6 foo7) &&
 3:  c659a0ce551 =  3:  cf05118aeae t9822: use prereq to check for ISO-8859-1 support
 4:  e7141b15b56 !  4:  4d41989afe6 t983*: use prereq to check for Python-specific git-b4(1) support
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t983*: use prereq to check for Python-specific git-b4(1) support
    +    t983*: use prereq to check for Python-specific git-p4(1) support
     
    -    The tests in t9835 and t9836 verify that git-b4(1) works with both
    +    The tests in t9835 and t9836 verify that git-p4(1) works with both
         Python 2 and 3, respectively. To determine whether we have those Python
         versions in the first place we create a wrapper script that directly
    -    executes the git-b4(1) script with `python2` or `python3` binaries. We
    +    executes the git-p4(1) script with `python2` or `python3` binaries. We
         then condition the execution of tests on whether that wrapper script can
         be executed successfully.
     
 5:  71b76db40e4 =  5:  b3f5f4e0d4d t/test-lib: don't print shell traces to stdout
 6:  b60daf5ac69 !  6:  78ab5b1d331 t/test-lib: fix TAP format for BASH_XTRACEFD warning
    @@ Commit message
         t/test-lib: fix TAP format for BASH_XTRACEFD warning
     
         When the Bash version is too old to support BASH_XTRACEFD we print a
    -    warning to stderr. This warning breaks the TAP format because it is not
    -    prefixed with a "#". Fix this.
    +    warning to stderr. This warning is not prefixed with "#", which causes
    +    TAP parsers to (wrongly) interpret the warning as part of the protocol.
    +
    +    Fix this issue by prefixing the warning with a "#" so that it is treated
    +    as comment.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
 7:  ce55bee9a12 !  7:  61b8b7640b7 t7815: fix unexpectedly passing test on macOS
    @@ Commit message
     
         The test passes if '.' matches a NUL byte, which we expect to only
         happen on Cygwin. The upcoming changes to support parsing TAP output in
    -    Meson surface that this test is also unexpectedly passing on macOS
    -    though.
    +    Meson surface that this test, surprisingly, passes on macOS as well.
     
         It is unclear how long the test has been passing on macOS already.
         064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7,
    -    2025-04-17) mentions that the test started to pass for Cygwin once it
    -    has imported a newer implementation of regcomp(3p) et all, which was
    +    2025-04-17) mentions that the test started to pass for Cygwin. This was
    +    attributed to a new implementation of regcomp(3p) and friends, which was
         inherited from FreeBSD. Given the BSD lineage of macOS it is feasible
         that it also inherited similar code eventually that made the test pass
         now.
    @@ Commit message
         same in this edge case and mark the new behaviour as expected.
     
         This change is outside of the scope of this patch series, which only
    -    introduce support for TAP. So instead of fixing the bigger issue, ignore
    -    the test on Darwin like we already do for Cygwin.
    +    introduces support for TAP. So instead of fixing the bigger issue,
    +    ignore the test on Darwin like we already do for Cygwin.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
 8:  80dcf1d5979 =  8:  02011b7017c test-lib: fail on unexpectedly passing tests
 9:  dca5730ab18 =  9:  540741acc80 meson: introduce kwargs variable for tests
10:  60393aa4af9 = 10:  8417d0ed94c meson: parse TAP output generated by our tests

--- base-commit: 845c48a16a7f7b2c44d8cb137b16a4a1f0140229 change-id: 20250429-pks-meson-tap-1eed604a02a3

We have a couple of cases where our tests end up announcing that a certain prerequisite is or isn't fulfilled. While this is supposed to help the developer it has the downside that it breaks the TAP format.

We could convert these cases to just have a "#" prefix, but it feels rather unlikely that these are generally useful in the first place. We already do announce why a specific test is being skipped, so we should try to use this mechanism to the best extent possible.

Stop announcing these prereqs to fix the TAP format. Where possible, convert the tests to rely on the prerequisites themselves to announce why a test ran or didn't ran.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0050-filesystem.sh                  | 30 ++++++------------------------
 t/t3600-rm.sh                          |  5 -----
 t/t4000-diff-format.sh                 |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 16 +++++++---------
 t/t9903-bash-prompt.sh                 |  4 ----
 5 files changed, 14 insertions(+), 43 deletions(-)

There are a couple users of `test_create_repo()` that use this function outside of any test case. This function is nowadays only a thin wrapper around `git init`, which by default prints a message to stdout that the repository has been initialized. The resulting output may thus confuse TAP parsers.

Refactor these users to instead create the repository in a "setup" test case so that we don't explicitly have to silence them. There's one exception in t1007: we use `push_repo()` and its `pop_repo()` equivalent multiple times, so to reduce the noise introduced by this patch we instead silence this invocation.

While at it, convert callsites to use git-init(1) directly as the `test_create_repo()` function has been deprecated in f0d4d398e28 (test-lib: split up and deprecate test_create_repo(), 2021-05-10).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1007-hash-object.sh                       |  2 +-
 t/t4041-diff-submodule-option.sh             | 22 +++++++++++++---------
 t/t4060-diff-submodule-option-diff-format.sh |  9 ++++++---
 t/t7401-submodule-summary.sh                 | 18 +++++++++++-------
 4 files changed, 31 insertions(+), 20 deletions(-)

Tests in t9822 depend on filesystem support for ISO-8859-1 encoding. We thus have a block of code that acts as a prerequisite -- if we fail to write a file with an ISO-8859-1-encoded file name to disk then we skip all tests.

When the prerequisite fails though we end up printing an error message to stderr, which breaks the TAP format. Fix this by converting the code to a proper prerequisite, which handles output redirection for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9822-git-p4-path-encoding.sh | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

The tests in t9835 and t9836 verify that git-p4(1) works with both Python 2 and 3, respectively. To determine whether we have those Python versions in the first place we create a wrapper script that directly executes the git-p4(1) script with `python2` or `python3` binaries. We then condition the execution of tests on whether that wrapper script can be executed successfully.

The logic that does all of this is not contained in a prerequisite block though, so the output it generates causes us to break the TAP format. Refactor the logic to use `test_lazy_prereq()` to fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9835-git-p4-metadata-encoding-python2.sh | 24 +++++++++++++-----------
 t/t9836-git-p4-metadata-encoding-python3.sh | 24 +++++++++++++-----------
 2 files changed, 26 insertions(+), 22 deletions(-)

When the Bash version is too old to support BASH_XTRACEFD we print a warning to stderr. This warning is not prefixed with "#", which causes TAP parsers to (wrongly) interpret the warning as part of the protocol.

Fix this issue by prefixing the warning with a "#" so that it is treated as comment.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

We have several flags like "--verbose", "--verbose-only" or "-x" that cause us to generate shell traces. The generated tracing output is split up in these cases so that the test's stdout is printed to file descriptor 3 whereas its stderr is printed to file descriptor 4. Depending on which options have been given, we then end up either:

  - Redirecting both file descriptors to a file.

  - Redirecting them to stdout and stderr, respectively.

  - Closing them in case we're running in none-verbose mode.

The second case causes problems though when passing output to a TAP parser. We print the test's stdout to the console's stdout, and that results in broken TAP output.

Fix the issue by instead redirecting the test's stdout to the shell's stderr. This makes it impossible to discern stdout from stderr, but going by my own experience I never came across a usecase where I would have needed this distinction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 35 +++++++++++++++++++----------------
 t/test-lib.sh    |  4 ++--
 2 files changed, 21 insertions(+), 18 deletions(-)

When tests are executed via `test_expect_failure` we rather obviously expect the test itself to fail. If it unexpectedly does not fail then we count the test as a "fixed" test and announce that a known breakage has vanished:

    ok 1 - setup
    ok 2 - create refs/heads/main # TODO known breakage vanished
    ok 3 - create refs/heads/main with oldvalue verification
    ...
    ok 299 - update-ref should also create reflog for HEAD
    # 1 known breakage(s) vanished; please update test(s)
    # passed all remaining 298 test(s)
    1..299

While we announce that tests should be updated, the overall test suite still passes. This makes it quite hard to detect when a test that has previously failed succeeds now as the developer needs to pay close attention to the exact output. Even more importantly, tests that only succeed on _some_ systems are even easier to miss now, as one would have to explicitly take a look at respective CI jobs to notice that those do pass now.

Furthermore, we are about to introduce support for parsing TAP output in Meson. In contrast to prove(1), which treats unexpected passes as a successful test run, Meson treats those as failure. Neither of these tools is wrong in doing so. Quoting the TAP specification [1]:

    Should a todo test point begin succeeding, the harness may report it
    in some way that indicates that whatever was supposed to be done has
    been, and it should be promoted to a normal Test Point.

So it is essentially implementation-defined how exactly the unexpected pass is reported, and whether it should cause the overall test suite to fail or not. It is unarguably a bad thing for us though if these tools interpret these differently, as it would mean that test results now depend on whether the developer uses prove(1) or Meson.

Unify the behaviour by causing a test suite to fail when there are any unexpected passes. As prove(1) does not consider an unexpected pass to be an error this leads to somewhat funky output:

    t1400-update-ref.sh ................................ Dubious, test returned 1 (wstat 256, 0x100)
    All 299 subtests passed
            (1 TODO test unexpectedly succeeded)

    ...

    Test Summary Report
    -------------------
    t1400-update-ref.sh                              (Wstat: 256 (exited 1) Tests: 299 Failed: 0)
      TODO passed:   2
      Non-zero exit status: 1

But as we directly announce that the root cause is an unexpected TODO that has succeeded it's not all that bad.

[1]: https://testanything.org/tap-version-14-specification.html

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0000-basic.sh | 4 ++--
 t/test-lib.sh    | 9 ++++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

In t7815, we have the following test:

    test_expect_failure !CYGWIN 'git grep .fi a' '
        git grep .fi a
    '

The test passes if '.' matches a NUL byte, which we expect to only happen on Cygwin. The upcoming changes to support parsing TAP output in Meson surface that this test, surprisingly, passes on macOS as well.

It is unclear how long the test has been passing on macOS already. 064eed36c7f (config.mak.uname: only set NO_REGEX on cygwin for v1.7, 2025-04-17) mentions that the test started to pass for Cygwin. This was attributed to a new implementation of regcomp(3p) and friends, which was inherited from FreeBSD. Given the BSD lineage of macOS it is feasible that it also inherited similar code eventually that made the test pass now.

It is somewhat dubious what the test actually brings to the table given that it is quite platform specific. Ideally, we would fix this mess by having a configure-time check whether regcomp(3p) works as expected, including NUL bytes, and use our bundled version of the regex library in case it doesn't. Like this, we could ensure that all platforms work the same in this edge case and mark the new behaviour as expected.

This change is outside of the scope of this patch series, which only introduces support for TAP. So instead of fixing the bigger issue, ignore the test on Darwin like we already do for Cygwin.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7815-grep-binary.sh | 2 +-
 t/test-lib.sh          | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Meson has the ability to create a kwargs dictionary that can then be passed to any function call with the `kwargs:` positional argument. This allows one to deduplicate common parameters that one wishes to pass to several different function invocations.

Our tests already have one common parameter that we use everywhere, "timeout", and we're about to add a second common parameter in the next commit. Let's prepare for this by introducing `test_kwargs` so that we can deduplicate these common arguments.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/credential/netrc/meson.build | 2 +-
 contrib/subtree/meson.build          | 2 +-
 meson.build                          | 4 ++++
 t/meson.build                        | 6 +++---
 4 files changed, 9 insertions(+), 5 deletions(-)

By default, Meson only knows to pay respect to the exit code of tests to judge whether or not it ran successfully. This can be changed though by specifying the "protocol" parameter. Next to the default "exitcode" protocol, Meson also supports the "tap" output that our tests already know to generate.

Unfortunately, the "tap" protocol was incompatible with `meson test --interactive` and caused a hang. We have upstreamed a fix [1] though, so with the recent release of Meson 1.8 that fix is finally out and we can start using the "tap" protocol when running with a recent-enough version of this build tool.

With this change in place, Meson now properly detects how many subtests ran and whether test suites have been skipped:

    ```
    $ meson test t002*
    ninja: Entering directory `/home/pks/Development/git/build'
     1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
     2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
     3/10 t0029-core-unsetenvvars             SKIP            0.15s
     4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
     5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
     6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
     7/10 t0020-crlf                          OK              0.81s   36 subtests passed
     8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
     9/10 t0021-conversion                    OK              3.45s   38 subtests passed
    10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed

    Ok:                9
    Fail:              0
    Skipped:           1
    ```

Note that when running `meson test --interactive` the test results will now be marked as "ignored". This is because in interactive mode the file descriptors will remain connected to the user's terminal, and it is expected that the user interacts with the tests (e.g., spawn a debugger or use `test_pause`). As such, the TAP output cannot be parsed reliably by Meson in that case, so the tests are marked as ignored accordingly.

[1]: https://github.com/mesonbuild/meson/pull/13980

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 8 ++++++++
 1 file changed, 8 insertions(+)

Hello

The range-diff looks great. Thanks!