[PATCH 0/4] meson: parse TAP output generated by our tests

[PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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

[PATCH 1/4] t: fix cases where output breaks TAP format

[Patrick Steinhardt]

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(-)

diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index b3cf53ff8c9..210cce56ec6 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -30,7 +30,7 @@ setup_repo() {
 
 test_repo=test
 push_repo() {
-	test_create_repo $test_repo
+	test_create_repo $test_repo >/dev/null
 	cd $test_repo
 
 	setup_repo
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 28f9d83d4c1..31f359ddf1e 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -48,7 +48,7 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 add_file . foo >/dev/null
 
 head1=$(add_file sm1 foo1 foo2)
@@ -236,7 +236,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 '
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 fullhead6=$(cd sm1; git rev-parse --verify HEAD)
 test_expect_success 'nonexistent commit' '
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 76b83101d3b..17ef40c0c9f 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -364,7 +364,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 '
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=diff HEAD >actual &&
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 9c3cc4cf404..80bbb1b7b5b 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -38,7 +38,7 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 add_file . foo >/dev/null
 
 head1=$(add_file sm1 foo1 foo2)
@@ -215,7 +215,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
 "
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 test_expect_success 'nonexistent commit' "
 	git submodule summary >actual &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 7679780fb87..84196a10896 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -701,13 +701,13 @@ test_expect_success \
 # syntax highlighting
 
 
-highlight_version=$(highlight --version </dev/null 2>/dev/null)
-if [ $? -eq 127 ]; then
-	say "Skipping syntax highlighting tests: 'highlight' not found"
-elif test -z "$highlight_version"; then
-	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
-else
-	test_set_prereq HIGHLIGHT
+test_lazy_prereq HIGHLIGHT '
+	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
+	test -n "$highlight_version"
+'
+
+if test_have_prereq HIGHLIGHT
+then
 	cat >>gitweb_config.perl <<-\EOF
 	our $highlight_bin = "highlight";
 	$feature{'highlight'}{'override'} = 1;

-- 
2.49.0.1045.g170613ef41.dirty

Re: [PATCH 1/4] t: fix cases where output breaks TAP format

[Phillip Wood]

Hi Patrick

On 06/05/2025 11:59, Patrick Steinhardt wrote:
> 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.

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

> 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(-)
> 
> diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> index b3cf53ff8c9..210cce56ec6 100755
> --- a/t/t1007-hash-object.sh
> +++ b/t/t1007-hash-object.sh
> @@ -30,7 +30,7 @@ setup_repo() {
>   
>   test_repo=test
>   push_repo() {
> -	test_create_repo $test_repo
> +	test_create_repo $test_repo >/dev/null
>   	cd $test_repo
>   
>   	setup_repo
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 28f9d83d4c1..31f359ddf1e 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -48,7 +48,7 @@ commit_file () {
>   	git commit "$@" -m "Commit $*" >/dev/null
>   }
>   
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>   add_file . foo >/dev/null
>   
>   head1=$(add_file sm1 foo1 foo2)
> @@ -236,7 +236,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
>   '
>   
>   rm -f sm1 &&
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>   head6=$(add_file sm1 foo6 foo7)
>   fullhead6=$(cd sm1; git rev-parse --verify HEAD)
>   test_expect_success 'nonexistent commit' '
> diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
> index 76b83101d3b..17ef40c0c9f 100755
> --- a/t/t4060-diff-submodule-option-diff-format.sh
> +++ b/t/t4060-diff-submodule-option-diff-format.sh
> @@ -364,7 +364,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
>   '
>   
>   rm -f sm1 &&
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>   head6=$(add_file sm1 foo6 foo7)
>   test_expect_success 'nonexistent commit' '
>   	git diff-index -p --submodule=diff HEAD >actual &&
> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
> index 9c3cc4cf404..80bbb1b7b5b 100755
> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
> @@ -38,7 +38,7 @@ commit_file () {
>   	git commit "$@" -m "Commit $*" >/dev/null
>   }
>   
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>   add_file . foo >/dev/null
>   
>   head1=$(add_file sm1 foo1 foo2)
> @@ -215,7 +215,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
>   "
>   
>   rm -f sm1 &&
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>   head6=$(add_file sm1 foo6 foo7)
>   test_expect_success 'nonexistent commit' "
>   	git submodule summary >actual &&
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index 7679780fb87..84196a10896 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -701,13 +701,13 @@ test_expect_success \
>   # syntax highlighting
>   
>   
> -highlight_version=$(highlight --version </dev/null 2>/dev/null)
> -if [ $? -eq 127 ]; then
> -	say "Skipping syntax highlighting tests: 'highlight' not found"
> -elif test -z "$highlight_version"; then
> -	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
> -else
> -	test_set_prereq HIGHLIGHT
> +test_lazy_prereq HIGHLIGHT '
> +	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
> +	test -n "$highlight_version"
> +'
> +
> +if test_have_prereq HIGHLIGHT
> +then
>   	cat >>gitweb_config.perl <<-\EOF
>   	our $highlight_bin = "highlight";
>   	$feature{'highlight'}{'override'} = 1;
> 

Re: [PATCH 1/4] t: fix cases where output breaks TAP format

[Patrick Steinhardt]

On Tue, May 06, 2025 at 02:17:09PM +0100, Phillip Wood wrote:
> Hi Patrick
> 
> On 06/05/2025 11:59, Patrick Steinhardt wrote:
> > 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.
> 
> 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.

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

Re: [PATCH 1/4] t: fix cases where output breaks TAP format

[Phillip Wood]

On 07/05/2025 07:52, Patrick Steinhardt wrote:
> On Tue, May 06, 2025 at 02:17:09PM +0100, Phillip Wood 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.
> 
> 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.

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

Re: [PATCH 1/4] t: fix cases where output breaks TAP format

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

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.

> 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.

The changes make sense WRT to the description.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 2/4] t/test-lib: don't print shell traces to stdout

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 35c5c2b4f9b..16b785f3b91 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -219,41 +219,44 @@ test_expect_success 'subtest: --verbose option' '
 	test_expect_success "failing test" false
 	test_done
 	EOF
-	mv t1234-verbose/out t1234-verbose/out+ &&
-	grep -v "^Initialized empty" t1234-verbose/out+ >t1234-verbose/out &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
-	> expecting success of 1234.1 '\''passing test'\'': true
+	mv t1234-verbose/err t1234-verbose/err+ &&
+	grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err &&
+	check_sub_test_lib_test_err t1234-verbose \
+		<<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
+	> ok 2 - test with output
+	> not ok 3 - failing test
+	> #	false
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF_OUT
+	> expecting success of 1234.1 '\''passing test'\'': true
 	> Z
 	> expecting success of 1234.2 '\''test with output'\'': echo foo
 	> foo
-	> ok 2 - test with output
 	> Z
 	> expecting success of 1234.3 '\''failing test'\'': false
-	> not ok 3 - failing test
-	> #	false
 	> Z
-	> # failed 1 among 3 test(s)
-	> 1..3
-	EOF
+	EOF_ERR
 '
 
 test_expect_success 'subtest: --verbose-only option' '
 	run_sub_test_lib_test_err \
 		t1234-verbose \
 		--verbose-only=2 &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
+	check_sub_test_lib_test_err t1234-verbose <<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
-	> Z
-	> expecting success of 1234.2 '\''test with output'\'': echo foo
-	> foo
 	> ok 2 - test with output
-	> Z
 	> not ok 3 - failing test
 	> #	false
 	> # failed 1 among 3 test(s)
 	> 1..3
-	EOF
+	EOF_OUT
+	> Z
+	> expecting success of 1234.2 '\''test with output'\'': echo foo
+	> foo
+	> Z
+	EOF_ERR
 '
 
 test_expect_success 'subtest: skip one with GIT_SKIP_TESTS' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index af722d383d9..6ce8570226c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -707,7 +707,7 @@ then
 	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
 elif test "$verbose" = "t"
 then
-	exec 4>&2 3>&1
+	exec 4>&2 3>&2
 else
 	exec 4>/dev/null 3>/dev/null
 fi
@@ -949,7 +949,7 @@ maybe_setup_verbose () {
 	test -z "$verbose_only" && return
 	if match_pattern_list $test_count "$verbose_only"
 	then
-		exec 4>&2 3>&1
+		exec 4>&2 3>&2
 		# Emit a delimiting blank line when going from
 		# non-verbose to verbose.  Within verbose mode the
 		# delimiter is printed by test_expect_*.  The choice

-- 
2.49.0.1045.g170613ef41.dirty

[PATCH 3/4] meson: introduce kwargs variable for tests

[Patrick Steinhardt]

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(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index 3d74547c8ae..16fa69e317e 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -17,6 +17,6 @@ if get_option('tests')
     workdir: meson.current_source_dir(),
     env: credential_netrc_testenv,
     depends: test_dependencies + bin_wrappers + [credential_netrc],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 63714166a61..98dd8e0c8ea 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -21,7 +21,7 @@ if get_option('tests')
     env: subtree_test_environment,
     workdir: meson.current_source_dir() / 't',
     depends: test_dependencies + bin_wrappers + [ git_subtree ],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
 
diff --git a/meson.build b/meson.build
index 270ce933d0f..94bd525dd7b 100644
--- a/meson.build
+++ b/meson.build
@@ -2027,6 +2027,10 @@ subdir('templates')
 # can properly set up test dependencies. The bin-wrappers themselves are set up
 # at configuration time, so these are fine.
 if get_option('tests')
+  test_kwargs = {
+    'timeout': 0,
+  }
+
   subdir('t')
 endif
 
diff --git a/t/meson.build b/t/meson.build
index b09c0becb8d..1af7111b0f8 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -51,7 +51,7 @@ clar_unit_tests = executable('unit-tests',
   sources: clar_sources + clar_test_suites,
   dependencies: [libgit_commonmain],
 )
-test('unit-tests', clar_unit_tests)
+test('unit-tests', clar_unit_tests, kwargs: test_kwargs)
 
 unit_test_programs = [
   'unit-tests/t-reftable-basics.c',
@@ -76,7 +76,7 @@ foreach unit_test_program : unit_test_programs
   )
   test(unit_test_name, unit_test,
     workdir: meson.current_source_dir(),
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 
@@ -1210,7 +1210,7 @@ foreach integration_test : integration_tests
     workdir: meson.current_source_dir(),
     env: test_environment,
     depends: test_dependencies + bin_wrappers,
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 

-- 
2.49.0.1045.g170613ef41.dirty

Re: [PATCH 3/4] meson: introduce kwargs variable for tests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

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

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 4/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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(+)

diff --git a/meson.build b/meson.build
index 94bd525dd7b..cd8df189d79 100644
--- a/meson.build
+++ b/meson.build
@@ -2031,6 +2031,14 @@ if get_option('tests')
     'timeout': 0,
   }
 
+  # The TAP protocol was already understood by previous versions of Meson, but
+  # it was incompatible with the `meson test --interactive` flag.
+  if meson.version().version_compare('>=1.8.0')
+    test_kwargs += {
+      'protocol': 'tap',
+    }
+  endif
+
   subdir('t')
 endif
 

-- 
2.49.0.1045.g170613ef41.dirty

Re: [PATCH 4/4] meson: parse TAP output generated by our tests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 6112 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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(+)
>
> diff --git a/meson.build b/meson.build
> index 94bd525dd7b..cd8df189d79 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2031,6 +2031,14 @@ if get_option('tests')
>      'timeout': 0,
>    }
>
> +  # The TAP protocol was already understood by previous versions of Meson, but
> +  # it was incompatible with the `meson test --interactive` flag.
> +  if meson.version().version_compare('>=1.8.0')
> +    test_kwargs += {
> +      'protocol': 'tap',
> +    }
> +  endif
> +
>

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.

>    subdir('t')
>  endif
>
>
> --
> 2.49.0.1045.g170613ef41.dirty

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 4/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Thu, May 15, 2025 at 12:48:23AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/meson.build b/meson.build
> > index 94bd525dd7b..cd8df189d79 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2031,6 +2031,14 @@ if get_option('tests')
> >      'timeout': 0,
> >    }
> >
> > +  # The TAP protocol was already understood by previous versions of Meson, but
> > +  # it was incompatible with the `meson test --interactive` flag.
> > +  if meson.version().version_compare('>=1.8.0')
> > +    test_kwargs += {
> > +      'protocol': 'tap',
> > +    }
> > +  endif
> > +
> >
> 
> The change itself looks good. But I do have a question about this:
> 
[snip]
> 
> 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.

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

Re: [PATCH 4/4] meson: parse TAP output generated by our tests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, May 15, 2025 at 12:48:23AM -0700, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> > diff --git a/meson.build b/meson.build
>> > index 94bd525dd7b..cd8df189d79 100644
>> > --- a/meson.build
>> > +++ b/meson.build
>> > @@ -2031,6 +2031,14 @@ if get_option('tests')
>> >      'timeout': 0,
>> >    }
>> >
>> > +  # The TAP protocol was already understood by previous versions of Meson, but
>> > +  # it was incompatible with the `meson test --interactive` flag.
>> > +  if meson.version().version_compare('>=1.8.0')
>> > +    test_kwargs += {
>> > +      'protocol': 'tap',
>> > +    }
>> > +  endif
>> > +
>> >
>>
>> The change itself looks good. But I do have a question about this:
>>
> [snip]
>>
>> 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.
>
> 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.
>

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

> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:
> 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.

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Tue, May 06, 2025 at 02:29:35PM +0200, Patrick Steinhardt wrote:
> On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:
> > 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.
> 
> 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.

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:
> 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

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

Thanks!

Patrick

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Hridoy Ahmed]

Hridoy Ahmed
Hy I am back 

> On 21 May 2025, at 1:57 PM, Patrick Steinhardt <ps@pks.im> wrote:
> 
> On Tue, May 06, 2025 at 12:59:49PM +0200, Patrick Steinhardt wrote:
>> 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
> 
> Junio, I noticed that this series isn't yet part of the "What's cooking"
> report. Is that intentional or an oversight?
> 
> Thanks!
> 
> Patrick
> 

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> 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
>
> Junio, I noticed that this series isn't yet part of the "What's cooking"
> report. Is that intentional or an oversight?

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-<.

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

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

> Patrick Steinhardt <ps@pks.im> writes:
>
>>> 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
>>
>> Junio, I noticed that this series isn't yet part of the "What's cooking"
>> report. Is that intentional or an oversight?
>
> 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-<.

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.

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Wed, May 21, 2025 at 02:26:23PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> >>> 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
> >>
> >> Junio, I noticed that this series isn't yet part of the "What's cooking"
> >> report. Is that intentional or an oversight?
> >
> > 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-<.

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.

> 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.

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Fri, May 23, 2025 at 12:03:24PM +0200, Patrick Steinhardt wrote:
> On Wed, May 21, 2025 at 02:26:23PM -0700, Junio C Hamano wrote:
> > 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.
> 
> 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.

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

diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index b7d83f9a5de..55d5e6de17c 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 !CYGWIN,!MACOS 'git grep .fi a' '
 	git grep .fi a
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6ce8570226c..fef522327f2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1636,6 +1636,9 @@ fi
 # Fix some commands on Windows, and other OS-specific things
 uname_s=$(uname -s)
 case $uname_s in
+Darwin)
+	test_set_prereq MACOS
+	;;
 *MINGW*)
 	# Windows has its own (incompatible) sort and find
 	sort () {

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> ... 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.

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

> 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.

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?

> diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
> index b7d83f9a5de..55d5e6de17c 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 !CYGWIN,!MACOS 'git grep .fi a' '
>  	git grep .fi a
>  '
>  
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 6ce8570226c..fef522327f2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1636,6 +1636,9 @@ fi
>  # Fix some commands on Windows, and other OS-specific things
>  uname_s=$(uname -s)
>  case $uname_s in
> +Darwin)
> +	test_set_prereq MACOS
> +	;;
>  *MINGW*)
>  	# Windows has its own (incompatible) sort and find
>  	sort () {

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Ramsay Jones]

On 23/05/2025 16:58, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
>> ... 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.
> 
> Yup, I was wondering about the same thing.  Thanks for confirming.
> It is unfortunate that we have blanket USES_BSD_REGEXP prerequisite
> ;-)
> 
>> 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.

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

> 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?

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

 

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Ramsay Jones]

On 23/05/2025 17:40, Ramsay Jones wrote:
> 
> 
> On 23/05/2025 16:58, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>>> ... 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.
>>
>> Yup, I was wondering about the same thing.  Thanks for confirming.
>> It is unfortunate that we have blanket USES_BSD_REGEXP prerequisite
>> ;-)
>>
>>> 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.
> 
> Yep, I very nearly sent that patch with a hunk which deleted that
> test. In the end, I decided to play it more conservatively. :)
> 
>> 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?
> 
> 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
>   $ 

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!

> 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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

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

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

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.

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Fri, May 23, 2025 at 12:33:23PM -0700, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > Question: should meson (or indeed prove) fail the test because of an
> > unexpected _pass_?
> 
> 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

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Phillip Wood]

On 26/05/2025 13:44, Patrick Steinhardt wrote:
> 
> 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.

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Todd Zullinger]

Phillip Wood wrote:
> On 26/05/2025 13:44, Patrick Steinhardt wrote:
>> 
>> 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.
> 
> 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.

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>

-- 
Todd

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 2385 bytes --]

On 5/26/25 8:44 AM, Patrick Steinhardt wrote:
> On Fri, May 23, 2025 at 12:33:23PM -0700, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>>
>>> Question: should meson (or indeed prove) fail the test because of an
>>> unexpected _pass_?
>>
>> 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
> 
> 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 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).
"""



> 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.


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


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Mon, May 26, 2025 at 09:54:40AM -0400, Eli Schwartz wrote:
> On 5/26/25 8:44 AM, Patrick Steinhardt wrote:
> > On Fri, May 23, 2025 at 12:33:23PM -0700, Junio C Hamano wrote:
> >> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> >>
> >>> Question: should meson (or indeed prove) fail the test because of an
> >>> unexpected _pass_?
> >>
> >> 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
> > 
> > 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 see how you can possibly compare this to UB. It is a documented
> variant behavior and thus "implementation-defined", not "undefined".

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

> 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).
> """
> 
> > 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.
> 
> As Phillip noted, treating them as ordinary passes undermines the reason
> for having them.

Yup, and I tend to agree.

Patrick

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> > 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.
>> 
>> As Phillip noted, treating them as ordinary passes undermines the reason
>> for having them.
>
> Yup, and I tend to agree.

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

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

On Tue, May 27, 2025 at 09:04:55AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> > 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.
> >> 
> >> As Phillip noted, treating them as ordinary passes undermines the reason
> >> for having them.
> >
> > Yup, and I tend to agree.
> 
> 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

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

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 16b785f3b91..2b63e1c86ca 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -130,7 +130,7 @@ test_expect_success 'subtest: a failing TODO test' '
 '
 
 test_expect_success 'subtest: a passing TODO test' '
-	write_and_run_sub_test_lib_test passing-todo <<-\EOF &&
+	write_and_run_sub_test_lib_test_err passing-todo <<-\EOF &&
 	test_expect_failure "pretend we have fixed a known breakage" "true"
 	test_done
 	EOF
@@ -142,7 +142,7 @@ test_expect_success 'subtest: a passing TODO test' '
 '
 
 test_expect_success 'subtest: 2 TODO tests, one passin' '
-	write_and_run_sub_test_lib_test partially-passing-todos <<-\EOF &&
+	write_and_run_sub_test_lib_test_err partially-passing-todos <<-\EOF &&
 	test_expect_failure "pretend we have a known breakage" "false"
 	test_expect_success "pretend we have a passing test" "true"
 	test_expect_failure "pretend we have fixed another known breakage" "true"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0a124ffad38..5352209d3e4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1272,7 +1272,14 @@ test_done () {
 
 		check_test_results_san_file_ "$test_failure"
 
-		if test -z "$skip_all" && test -n "$invert_exit_code"
+		if test "$test_fixed" != 0
+		then
+			if test -z "$invert_exit_code"
+			then
+				GIT_EXIT_OK=t
+				exit 1
+			fi
+		elif test -z "$skip_all" && test -n "$invert_exit_code"
 		then
 			say_color warn "# faking up non-zero exit with --invert-exit-code"
 			GIT_EXIT_OK=t

Re: [PATCH 0/4] meson: parse TAP output generated by our tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> 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
>
> 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 :/

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

[PATCH v2 0/6] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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

[PATCH v2 1/6] t: fix cases where output breaks TAP format

[Patrick Steinhardt]

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(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 5c9dc90d0b0..5f544d0f210 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -12,7 +12,6 @@ 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
@@ -20,15 +19,11 @@ 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" '
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index b3cf53ff8c9..210cce56ec6 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -30,7 +30,7 @@ setup_repo() {
 
 test_repo=test
 push_repo() {
-	test_create_repo $test_repo
+	test_create_repo $test_repo >/dev/null
 	cd $test_repo
 
 	setup_repo
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 98259e2adaa..1f16e6b5228 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -17,11 +17,6 @@ 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" &&
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index a51f881b1c9..32b14e3a714 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -36,7 +36,7 @@ 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
 
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 28f9d83d4c1..31f359ddf1e 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -48,7 +48,7 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 add_file . foo >/dev/null
 
 head1=$(add_file sm1 foo1 foo2)
@@ -236,7 +236,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 '
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 fullhead6=$(cd sm1; git rev-parse --verify HEAD)
 test_expect_success 'nonexistent commit' '
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 76b83101d3b..17ef40c0c9f 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -364,7 +364,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 '
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=diff HEAD >actual &&
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 9c3cc4cf404..80bbb1b7b5b 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -38,7 +38,7 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 add_file . foo >/dev/null
 
 head1=$(add_file sm1 foo1 foo2)
@@ -215,7 +215,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
 "
 
 rm -f sm1 &&
-test_create_repo sm1 &&
+test_create_repo sm1 >/dev/null &&
 head6=$(add_file sm1 foo6 foo7)
 test_expect_success 'nonexistent commit' "
 	git submodule summary >actual &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 7679780fb87..84196a10896 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -701,13 +701,13 @@ test_expect_success \
 # syntax highlighting
 
 
-highlight_version=$(highlight --version </dev/null 2>/dev/null)
-if [ $? -eq 127 ]; then
-	say "Skipping syntax highlighting tests: 'highlight' not found"
-elif test -z "$highlight_version"; then
-	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
-else
-	test_set_prereq HIGHLIGHT
+test_lazy_prereq HIGHLIGHT '
+	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
+	test -n "$highlight_version"
+'
+
+if test_have_prereq HIGHLIGHT
+then
 	cat >>gitweb_config.perl <<-\EOF
 	our $highlight_bin = "highlight";
 	$feature{'highlight'}{'override'} = 1;
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 572d395498e..ddb2052ea7b 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -7,12 +7,17 @@ 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
diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 6116f806f63..83eca4fa658 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -15,7 +15,7 @@ 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
@@ -28,7 +28,7 @@ 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
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 5e5217a66b4..da25edeb546 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -15,7 +15,7 @@ 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
@@ -28,7 +28,7 @@ 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
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index d667dda654e..637a6f13a6d 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -66,10 +66,6 @@ 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.49.0.1266.g31b7d2e469.dirty

Re: [PATCH v2 1/6] t: fix cases where output breaks TAP format

[Eric Sunshine]

On Tue, May 27, 2025 at 10:03 AM Patrick Steinhardt <ps@pks.im> wrote:
> 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.

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."

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> @@ -30,7 +30,7 @@ setup_repo() {
>  test_repo=test
>  push_repo() {
> -       test_create_repo $test_repo
> +       test_create_repo $test_repo >/dev/null
>         cd $test_repo
>         setup_repo

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.

> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> @@ -48,7 +48,7 @@ commit_file () {
> -test_create_repo sm1 &&
> +test_create_repo sm1 >/dev/null &&
>  add_file . foo >/dev/null
>
>  head1=$(add_file sm1 foo1 foo2)

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.

> diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> @@ -7,12 +7,17 @@ test_description='Clone repositories with non ASCII paths'
> -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"
> +'

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.

Re: [PATCH v2 1/6] t: fix cases where output breaks TAP format

[Patrick Steinhardt]

On Tue, May 27, 2025 at 03:47:16PM -0400, Eric Sunshine wrote:
> On Tue, May 27, 2025 at 10:03 AM Patrick Steinhardt <ps@pks.im> wrote:
> > 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.
> 
> 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."

Fair. We can say "adapt" instead.

> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> > @@ -30,7 +30,7 @@ setup_repo() {
> >  test_repo=test
> >  push_repo() {
> > -       test_create_repo $test_repo
> > +       test_create_repo $test_repo >/dev/null
> >         cd $test_repo
> >         setup_repo
> 
> 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.

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

> > diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> > @@ -48,7 +48,7 @@ commit_file () {
> > -test_create_repo sm1 &&
> > +test_create_repo sm1 >/dev/null &&
> >  add_file . foo >/dev/null
> >
> >  head1=$(add_file sm1 foo1 foo2)
> 
> 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.

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

> > diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> > @@ -7,12 +7,17 @@ test_description='Clone repositories with non ASCII paths'
> > -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"
> > +'
> 
> 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?

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.

> 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.

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

Re: [PATCH v2 1/6] t: fix cases where output breaks TAP format

[Eric Sunshine]

On Wed, May 28, 2025 at 11:55 AM Patrick Steinhardt <ps@pks.im> wrote:
> On Tue, May 27, 2025 at 03:47:16PM -0400, Eric Sunshine wrote:
> > On Tue, May 27, 2025 at 10:03 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> > > @@ -48,7 +48,7 @@ commit_file () {
> > > -test_create_repo sm1 &&
> > > +test_create_repo sm1 >/dev/null &&
> > >  add_file . foo >/dev/null
> > >
> > >  head1=$(add_file sm1 foo1 foo2)
> >
> > 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)
> >     '
>
> Yes, it isn't particularly hard. But it does result in a bunch of
> shuffling that makes the patch way harder to read.

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.

> > > diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
> > > @@ -7,12 +7,17 @@ test_description='Clone repositories with non ASCII paths'
> > > -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"
> > > +'
> >
> > 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?
>
> 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.

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.

Re: [PATCH v2 1/6] t: fix cases where output breaks TAP format

[Patrick Steinhardt]

On Wed, May 28, 2025 at 04:14:17PM -0400, Eric Sunshine wrote:
> On Wed, May 28, 2025 at 11:55 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Tue, May 27, 2025 at 03:47:16PM -0400, Eric Sunshine wrote:
> > > On Tue, May 27, 2025 at 10:03 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > > diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> > > > @@ -48,7 +48,7 @@ commit_file () {
> > > > -test_create_repo sm1 &&
> > > > +test_create_repo sm1 >/dev/null &&
> > > >  add_file . foo >/dev/null
> > > >
> > > >  head1=$(add_file sm1 foo1 foo2)
> > >
> > > 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)
> > >     '
> >
> > Yes, it isn't particularly hard. But it does result in a bunch of
> > shuffling that makes the patch way harder to read.
> 
> 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.

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

[PATCH v2 2/6] t/test-lib: don't print shell traces to stdout

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 35c5c2b4f9b..16b785f3b91 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -219,41 +219,44 @@ test_expect_success 'subtest: --verbose option' '
 	test_expect_success "failing test" false
 	test_done
 	EOF
-	mv t1234-verbose/out t1234-verbose/out+ &&
-	grep -v "^Initialized empty" t1234-verbose/out+ >t1234-verbose/out &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
-	> expecting success of 1234.1 '\''passing test'\'': true
+	mv t1234-verbose/err t1234-verbose/err+ &&
+	grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err &&
+	check_sub_test_lib_test_err t1234-verbose \
+		<<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
+	> ok 2 - test with output
+	> not ok 3 - failing test
+	> #	false
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF_OUT
+	> expecting success of 1234.1 '\''passing test'\'': true
 	> Z
 	> expecting success of 1234.2 '\''test with output'\'': echo foo
 	> foo
-	> ok 2 - test with output
 	> Z
 	> expecting success of 1234.3 '\''failing test'\'': false
-	> not ok 3 - failing test
-	> #	false
 	> Z
-	> # failed 1 among 3 test(s)
-	> 1..3
-	EOF
+	EOF_ERR
 '
 
 test_expect_success 'subtest: --verbose-only option' '
 	run_sub_test_lib_test_err \
 		t1234-verbose \
 		--verbose-only=2 &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
+	check_sub_test_lib_test_err t1234-verbose <<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
-	> Z
-	> expecting success of 1234.2 '\''test with output'\'': echo foo
-	> foo
 	> ok 2 - test with output
-	> Z
 	> not ok 3 - failing test
 	> #	false
 	> # failed 1 among 3 test(s)
 	> 1..3
-	EOF
+	EOF_OUT
+	> Z
+	> expecting success of 1234.2 '\''test with output'\'': echo foo
+	> foo
+	> Z
+	EOF_ERR
 '
 
 test_expect_success 'subtest: skip one with GIT_SKIP_TESTS' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index af722d383d9..6ce8570226c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -707,7 +707,7 @@ then
 	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
 elif test "$verbose" = "t"
 then
-	exec 4>&2 3>&1
+	exec 4>&2 3>&2
 else
 	exec 4>/dev/null 3>/dev/null
 fi
@@ -949,7 +949,7 @@ maybe_setup_verbose () {
 	test -z "$verbose_only" && return
 	if match_pattern_list $test_count "$verbose_only"
 	then
-		exec 4>&2 3>&1
+		exec 4>&2 3>&2
 		# Emit a delimiting blank line when going from
 		# non-verbose to verbose.  Within verbose mode the
 		# delimiter is printed by test_expect_*.  The choice

-- 
2.49.0.1266.g31b7d2e469.dirty

Re: [PATCH v2 2/6] t/test-lib: don't print shell traces to stdout

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

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.

> 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(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 35c5c2b4f9b..16b785f3b91 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -219,41 +219,44 @@ test_expect_success 'subtest: --verbose option' '
>  	test_expect_success "failing test" false
>  	test_done
>  	EOF
> -	mv t1234-verbose/out t1234-verbose/out+ &&
> -	grep -v "^Initialized empty" t1234-verbose/out+ >t1234-verbose/out &&
> -	check_sub_test_lib_test t1234-verbose <<-\EOF
> -	> expecting success of 1234.1 '\''passing test'\'': true
> +	mv t1234-verbose/err t1234-verbose/err+ &&
> +	grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err &&
> +	check_sub_test_lib_test_err t1234-verbose \
> +		<<-\EOF_OUT 3<<-\EOF_ERR
>  	> ok 1 - passing test
> +	> ok 2 - test with output
> +	> not ok 3 - failing test
> +	> #	false
> +	> # failed 1 among 3 test(s)
> +	> 1..3
> +	EOF_OUT
> +	> expecting success of 1234.1 '\''passing test'\'': true
>  	> Z
>  	> expecting success of 1234.2 '\''test with output'\'': echo foo
>  	> foo
> -	> ok 2 - test with output
>  	> Z
>  	> expecting success of 1234.3 '\''failing test'\'': false
> -	> not ok 3 - failing test
> -	> #	false
>  	> Z
> -	> # failed 1 among 3 test(s)
> -	> 1..3
> -	EOF
> +	EOF_ERR
>  '
>  
>  test_expect_success 'subtest: --verbose-only option' '
>  	run_sub_test_lib_test_err \
>  		t1234-verbose \
>  		--verbose-only=2 &&
> -	check_sub_test_lib_test t1234-verbose <<-\EOF
> +	check_sub_test_lib_test_err t1234-verbose <<-\EOF_OUT 3<<-\EOF_ERR
>  	> ok 1 - passing test
> -	> Z
> -	> expecting success of 1234.2 '\''test with output'\'': echo foo
> -	> foo
>  	> ok 2 - test with output
> -	> Z
>  	> not ok 3 - failing test
>  	> #	false
>  	> # failed 1 among 3 test(s)
>  	> 1..3
> -	EOF
> +	EOF_OUT
> +	> Z
> +	> expecting success of 1234.2 '\''test with output'\'': echo foo
> +	> foo
> +	> Z
> +	EOF_ERR
>  '
>  
>  test_expect_success 'subtest: skip one with GIT_SKIP_TESTS' '
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index af722d383d9..6ce8570226c 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -707,7 +707,7 @@ then
>  	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
>  elif test "$verbose" = "t"
>  then
> -	exec 4>&2 3>&1
> +	exec 4>&2 3>&2
>  else
>  	exec 4>/dev/null 3>/dev/null
>  fi
> @@ -949,7 +949,7 @@ maybe_setup_verbose () {
>  	test -z "$verbose_only" && return
>  	if match_pattern_list $test_count "$verbose_only"
>  	then
> -		exec 4>&2 3>&1
> +		exec 4>&2 3>&2
>  		# Emit a delimiting blank line when going from
>  		# non-verbose to verbose.  Within verbose mode the
>  		# delimiter is printed by test_expect_*.  The choice

Re: [PATCH v2 2/6] t/test-lib: don't print shell traces to stdout

[Patrick Steinhardt]

On Tue, May 27, 2025 at 12:47:00PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > 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.
> 
> 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?

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.

> 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.

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

[PATCH v2 3/6] t/test-lib: fix TAP format for BASH_XTRACEFD warning

[Patrick Steinhardt]

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(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6ce8570226c..8c0d76ea5f0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -470,7 +470,7 @@ then
 	then
 		: Executed by a Bash version supporting BASH_XTRACEFD.  Good.
 	else
-		echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+		echo >&2 "# warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
 		trace=
 	fi
 fi

-- 
2.49.0.1266.g31b7d2e469.dirty

[PATCH v2 4/6] t7815: fix unexpectedly passing test on macOS

[Patrick Steinhardt]

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(-)

diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index b7d83f9a5de..55d5e6de17c 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 !CYGWIN,!MACOS 'git grep .fi a' '
 	git grep .fi a
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8c0d76ea5f0..0a124ffad38 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1636,6 +1636,9 @@ fi
 # Fix some commands on Windows, and other OS-specific things
 uname_s=$(uname -s)
 case $uname_s in
+Darwin)
+	test_set_prereq MACOS
+	;;
 *MINGW*)
 	# Windows has its own (incompatible) sort and find
 	sort () {

-- 
2.49.0.1266.g31b7d2e469.dirty

[PATCH v2 5/6] meson: introduce kwargs variable for tests

[Patrick Steinhardt]

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(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index 3d74547c8ae..16fa69e317e 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -17,6 +17,6 @@ if get_option('tests')
     workdir: meson.current_source_dir(),
     env: credential_netrc_testenv,
     depends: test_dependencies + bin_wrappers + [credential_netrc],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 63714166a61..98dd8e0c8ea 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -21,7 +21,7 @@ if get_option('tests')
     env: subtree_test_environment,
     workdir: meson.current_source_dir() / 't',
     depends: test_dependencies + bin_wrappers + [ git_subtree ],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
 
diff --git a/meson.build b/meson.build
index a1476e5b322..6fb898a21d1 100644
--- a/meson.build
+++ b/meson.build
@@ -2036,6 +2036,10 @@ subdir('templates')
 # can properly set up test dependencies. The bin-wrappers themselves are set up
 # at configuration time, so these are fine.
 if get_option('tests')
+  test_kwargs = {
+    'timeout': 0,
+  }
+
   subdir('t')
 endif
 
diff --git a/t/meson.build b/t/meson.build
index fcfc1c2c2ba..3fc8c6c2201 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -51,7 +51,7 @@ clar_unit_tests = executable('unit-tests',
   sources: clar_sources + clar_test_suites,
   dependencies: [libgit_commonmain],
 )
-test('unit-tests', clar_unit_tests)
+test('unit-tests', clar_unit_tests, kwargs: test_kwargs)
 
 unit_test_programs = [
   'unit-tests/t-reftable-basics.c',
@@ -76,7 +76,7 @@ foreach unit_test_program : unit_test_programs
   )
   test(unit_test_name, unit_test,
     workdir: meson.current_source_dir(),
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 
@@ -1212,7 +1212,7 @@ foreach integration_test : integration_tests
     workdir: meson.current_source_dir(),
     env: test_environment,
     depends: test_dependencies + bin_wrappers,
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 

-- 
2.49.0.1266.g31b7d2e469.dirty

[PATCH v2 6/6] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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(+)

diff --git a/meson.build b/meson.build
index 6fb898a21d1..46c5d068e05 100644
--- a/meson.build
+++ b/meson.build
@@ -2040,6 +2040,14 @@ if get_option('tests')
     'timeout': 0,
   }
 
+  # The TAP protocol was already understood by previous versions of Meson, but
+  # it was incompatible with the `meson test --interactive` flag.
+  if meson.version().version_compare('>=1.8.0')
+    test_kwargs += {
+      'protocol': 'tap',
+    }
+  endif
+
   subdir('t')
 endif
 

-- 
2.49.0.1266.g31b7d2e469.dirty

[PATCH v3 00/10] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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

[PATCH v3 01/10] t: stop announcing prereqs

[Patrick Steinhardt]

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(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 5c9dc90d0b0..ca8568067d3 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -10,53 +10,35 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 auml=$(printf '\303\244')
 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
-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" '
+test_expect_success CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
 	test $(git config --bool core.ignorecase) = true
 '
-else
-test_expect_success "detection of case insensitive filesystem during repo init" '
+
+test_expect_success !CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
 	{
 		test_must_fail git config --bool core.ignorecase >/dev/null ||
 			test $(git config --bool core.ignorecase) = false
 	}
 '
-fi
 
-if test_have_prereq SYMLINKS
-then
-test_expect_success "detection of filesystem w/o symlink support during repo init" '
+test_expect_success SYMLINKS "detection of filesystem w/o symlink support during repo init" '
 	{
 		test_must_fail git config --bool core.symlinks ||
 		test "$(git config --bool core.symlinks)" = true
 	}
 '
-else
-test_expect_success "detection of filesystem w/o symlink support during repo init" '
+
+test_expect_success !SYMLINKS "detection of filesystem w/o symlink support during repo init" '
 	v=$(git config --bool core.symlinks) &&
 	test "$v" = false
 '
-fi
 
 test_expect_success "setup case tests" '
 	git config core.ignorecase true &&
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 98259e2adaa..1f16e6b5228 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -17,11 +17,6 @@ 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" &&
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index a51f881b1c9..32b14e3a714 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -36,7 +36,7 @@ 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
 
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 7679780fb87..578d6c8b329 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -700,19 +700,17 @@ test_expect_success \
 # ----------------------------------------------------------------------
 # syntax highlighting
 
+test_lazy_prereq HIGHLIGHT '
+	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
+	test -n "$highlight_version"
+'
 
-highlight_version=$(highlight --version </dev/null 2>/dev/null)
-if [ $? -eq 127 ]; then
-	say "Skipping syntax highlighting tests: 'highlight' not found"
-elif test -z "$highlight_version"; then
-	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
-else
-	test_set_prereq HIGHLIGHT
+test_expect_success HIGHLIGHT '
 	cat >>gitweb_config.perl <<-\EOF
 	our $highlight_bin = "highlight";
-	$feature{'highlight'}{'override'} = 1;
+	$feature{"highlight"}{"override"} = 1;
 	EOF
-fi
+'
 
 test_expect_success HIGHLIGHT \
 	'syntax highlighting (no highlight, unknown syntax)' \
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index d667dda654e..637a6f13a6d 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -66,10 +66,6 @@ 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.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 01/10] t: stop announcing prereqs

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4278 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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(-)
>
> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
> index 5c9dc90d0b0..ca8568067d3 100755
> --- a/t/t0050-filesystem.sh
> +++ b/t/t0050-filesystem.sh
> @@ -10,53 +10,35 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  auml=$(printf '\303\244')
>  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
> -fi
> -

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

>  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
>

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`.

> -test_have_prereq SYMLINKS ||
> -	say "will test on a filesystem lacking symbolic links"
> -

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

> -if test_have_prereq CASE_INSENSITIVE_FS
> -then
> -test_expect_success "detection of case insensitive filesystem during repo init" '
> +test_expect_success CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
>  	test $(git config --bool core.ignorecase) = true
>  '
> -else
> -test_expect_success "detection of case insensitive filesystem during repo init" '
> +
> +test_expect_success !CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
>  	{
>  		test_must_fail git config --bool core.ignorecase >/dev/null ||
>  			test $(git config --bool core.ignorecase) = false
>  	}
>  '
> -fi
>

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

[snip]

> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index 7679780fb87..578d6c8b329 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -700,19 +700,17 @@ test_expect_success \
>  # ----------------------------------------------------------------------
>  # syntax highlighting
>
> +test_lazy_prereq HIGHLIGHT '
> +	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
> +	test -n "$highlight_version"
> +'
>

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.

> -highlight_version=$(highlight --version </dev/null 2>/dev/null)
> -if [ $? -eq 127 ]; then
> -	say "Skipping syntax highlighting tests: 'highlight' not found"
> -elif test -z "$highlight_version"; then
> -	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
> -else
> -	test_set_prereq HIGHLIGHT
> +test_expect_success HIGHLIGHT '
>  	cat >>gitweb_config.perl <<-\EOF
>  	our $highlight_bin = "highlight";
> -	$feature{'highlight'}{'override'} = 1;
> +	$feature{"highlight"}{"override"} = 1;

Nice ;)

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 02/10] t: silence output from `test_create_repo()`

[Patrick Steinhardt]

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(-)

diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index b3cf53ff8c9..2cd0be7ba76 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -30,7 +30,7 @@ setup_repo() {
 
 test_repo=test
 push_repo() {
-	test_create_repo $test_repo
+	git init --quiet $test_repo
 	cd $test_repo
 
 	setup_repo
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 28f9d83d4c1..e4289a1b71b 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -48,11 +48,12 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
-fullhead1=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup submodule' '
+	git init sm1 &&
+	add_file . foo &&
+	head1=$(add_file sm1 foo1 foo2) &&
+	fullhead1=$(cd sm1 && git rev-parse --verify HEAD)
+'
 
 test_expect_success 'added submodule' '
 	git add sm1 &&
@@ -235,10 +236,13 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 	test_cmp expected actual
 '
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
-fullhead6=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup submodule' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7) &&
+	fullhead6=$(cd sm1 && git rev-parse --verify HEAD)
+'
+
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 76b83101d3b..dbfeb7470bc 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -363,9 +363,12 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 	diff_cmp expected actual
 '
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
+test_expect_success 'setup' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7)
+'
+
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 9c3cc4cf404..66c3ec2da22 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -38,10 +38,11 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
+test_expect_success 'setup submodule' '
+	git init sm1 &&
+	add_file . foo &&
+	head1=$(add_file sm1 foo1 foo2)
+'
 
 test_expect_success 'added submodule' "
 	git add sm1 &&
@@ -214,9 +215,12 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
 	test_cmp expected actual
 "
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
+test_expect_success 'setup submodule' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7)
+'
+
 test_expect_success 'nonexistent commit' "
 	git submodule summary >actual &&
 	cat >expected <<-EOF &&

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 02/10] t: silence output from `test_create_repo()`

[Eric Sunshine]

On Fri, May 30, 2025 at 9:31 AM Patrick Steinhardt <ps@pks.im> wrote:
> 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>
> ---
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> @@ -48,11 +48,12 @@ commit_file () {
> +test_expect_success 'setup submodule' '
> +       git init sm1 &&
> +       add_file . foo &&
> +       head1=$(add_file sm1 foo1 foo2) &&
> +       fullhead1=$(cd sm1 && git rev-parse --verify HEAD)
> +'
> @@ -235,10 +236,13 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
> +test_expect_success 'setup submodule' '
> +       rm -f sm1 &&
> +       git init sm1 &&
> +       head6=$(add_file sm1 foo6 foo7) &&
> +       fullhead6=$(cd sm1 && git rev-parse --verify HEAD)
> +'

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/

[PATCH v3 03/10] t9822: use prereq to check for ISO-8859-1 support

[Patrick Steinhardt]

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(-)

diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 572d395498e..e6e07facd4b 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -7,12 +7,17 @@ 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" &&
+	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

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

[PATCH v3 04/10] t983*: use prereq to check for Python-specific git-b4(1) support

[Patrick Steinhardt]

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(-)

diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 6116f806f63..b969c7e0d5a 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -12,23 +12,25 @@ failing, and produces maximally sane output in git.'
 ## SECTION REPEATED IN t9836 ##
 ###############################
 
+EXTRA_PATH="$(pwd)/temp_python"
+mkdir "$EXTRA_PATH"
+PATH="$EXTRA_PATH:$PATH"
+export PATH
+
 # 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)
-if test -n "$python_target_binary"
-then
-	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH"
-	export PATH
-
-	write_script temp_python/git-p4-python2 <<-EOF
+test_lazy_prereq P4_PYTHON2 '
+	python_target_binary=$(which python2) &&
+	test -n "$python_target_binary" &&
+	write_script "$EXTRA_PATH"/git-p4-python2 <<-EOF &&
 	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
 	EOF
-fi
+	( git p4-python2 || true ) >err &&
+	test_grep "valid commands" err
+'
 
-git p4-python2 >err
-if ! grep 'valid commands' err
+if ! test_have_prereq P4_PYTHON2
 then
 	skip_all="skipping python2 git p4 tests; python2 not available"
 	test_done
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 5e5217a66b4..da6669bf711 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -12,23 +12,25 @@ failing, and produces maximally sane output in git.'
 ## SECTION REPEATED IN t9835 ##
 ###############################
 
+EXTRA_PATH="$(pwd)/temp_python"
+mkdir "$EXTRA_PATH"
+PATH="$EXTRA_PATH:$PATH"
+export PATH
+
 # 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)
-if test -n "$python_target_binary"
-then
-	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH"
-	export PATH
-
-	write_script temp_python/git-p4-python3 <<-EOF
+test_lazy_prereq P4_PYTHON3 '
+	python_target_binary=$(which python3) &&
+	test -n "$python_target_binary" &&
+	write_script "$EXTRA_PATH"/git-p4-python3 <<-EOF &&
 	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
 	EOF
-fi
+	( git p4-python3 || true ) >err &&
+	test_grep "valid commands" err
+'
 
-git p4-python3 >err
-if ! grep 'valid commands' err
+if ! test_have_prereq P4_PYTHON3
 then
 	skip_all="skipping python3 git p4 tests; python3 not available"
 	test_done

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 04/10] t983*: use prereq to check for Python-specific git-b4(1) support

[Todd Zullinger]

Patrick Steinhardt wrote:
> 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.

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)

-- 
Todd

Re: [PATCH v3 04/10] t983*: use prereq to check for Python-specific git-b4(1) support

[Patrick Steinhardt]

On Fri, May 30, 2025 at 10:08:15AM -0400, Todd Zullinger wrote:
> Patrick Steinhardt wrote:
> > 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.
> 
> 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. :)

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

> 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?

I don't think so.

> 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.

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

[PATCH v3 05/10] t/test-lib: don't print shell traces to stdout

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 35c5c2b4f9b..16b785f3b91 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -219,41 +219,44 @@ test_expect_success 'subtest: --verbose option' '
 	test_expect_success "failing test" false
 	test_done
 	EOF
-	mv t1234-verbose/out t1234-verbose/out+ &&
-	grep -v "^Initialized empty" t1234-verbose/out+ >t1234-verbose/out &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
-	> expecting success of 1234.1 '\''passing test'\'': true
+	mv t1234-verbose/err t1234-verbose/err+ &&
+	grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err &&
+	check_sub_test_lib_test_err t1234-verbose \
+		<<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
+	> ok 2 - test with output
+	> not ok 3 - failing test
+	> #	false
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF_OUT
+	> expecting success of 1234.1 '\''passing test'\'': true
 	> Z
 	> expecting success of 1234.2 '\''test with output'\'': echo foo
 	> foo
-	> ok 2 - test with output
 	> Z
 	> expecting success of 1234.3 '\''failing test'\'': false
-	> not ok 3 - failing test
-	> #	false
 	> Z
-	> # failed 1 among 3 test(s)
-	> 1..3
-	EOF
+	EOF_ERR
 '
 
 test_expect_success 'subtest: --verbose-only option' '
 	run_sub_test_lib_test_err \
 		t1234-verbose \
 		--verbose-only=2 &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
+	check_sub_test_lib_test_err t1234-verbose <<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
-	> Z
-	> expecting success of 1234.2 '\''test with output'\'': echo foo
-	> foo
 	> ok 2 - test with output
-	> Z
 	> not ok 3 - failing test
 	> #	false
 	> # failed 1 among 3 test(s)
 	> 1..3
-	EOF
+	EOF_OUT
+	> Z
+	> expecting success of 1234.2 '\''test with output'\'': echo foo
+	> foo
+	> Z
+	EOF_ERR
 '
 
 test_expect_success 'subtest: skip one with GIT_SKIP_TESTS' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index af722d383d9..6ce8570226c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -707,7 +707,7 @@ then
 	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
 elif test "$verbose" = "t"
 then
-	exec 4>&2 3>&1
+	exec 4>&2 3>&2
 else
 	exec 4>/dev/null 3>/dev/null
 fi
@@ -949,7 +949,7 @@ maybe_setup_verbose () {
 	test -z "$verbose_only" && return
 	if match_pattern_list $test_count "$verbose_only"
 	then
-		exec 4>&2 3>&1
+		exec 4>&2 3>&2
 		# Emit a delimiting blank line when going from
 		# non-verbose to verbose.  Within verbose mode the
 		# delimiter is printed by test_expect_*.  The choice

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 05/10] t/test-lib: don't print shell traces to stdout

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

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]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 06/10] t/test-lib: fix TAP format for BASH_XTRACEFD warning

[Patrick Steinhardt]

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(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6ce8570226c..8c0d76ea5f0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -470,7 +470,7 @@ then
 	then
 		: Executed by a Bash version supporting BASH_XTRACEFD.  Good.
 	else
-		echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+		echo >&2 "# warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
 		trace=
 	fi
 fi

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 06/10] t/test-lib: fix TAP format for BASH_XTRACEFD warning

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

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]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 07/10] t7815: fix unexpectedly passing test on macOS

[Patrick Steinhardt]

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(-)

diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index b7d83f9a5de..55d5e6de17c 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 !CYGWIN,!MACOS 'git grep .fi a' '
 	git grep .fi a
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8c0d76ea5f0..0a124ffad38 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1636,6 +1636,9 @@ fi
 # Fix some commands on Windows, and other OS-specific things
 uname_s=$(uname -s)
 case $uname_s in
+Darwin)
+	test_set_prereq MACOS
+	;;
 *MINGW*)
 	# Windows has its own (incompatible) sort and find
 	sort () {

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 07/10] t7815: fix unexpectedly passing test on macOS

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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

Nit: s/introduce/introduces

> the test on Darwin like we already do for Cygwin.
>

The change makes sense to me.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 07/10] t7815: fix unexpectedly passing test on macOS

[Kristoffer Haugsbakk]

On Fri, May 30, 2025, at 15:31, Patrick Steinhardt wrote:
> 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.

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.

> 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

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

> regcomp(3p) et all, which was

s/et all,/et al.,/

Re: [PATCH v3 07/10] t7815: fix unexpectedly passing test on macOS

[Patrick Steinhardt]

On Sun, Jun 01, 2025 at 11:19:21AM +0200, Kristoffer Haugsbakk wrote:
> On Fri, May 30, 2025, at 15:31, Patrick Steinhardt wrote:
> > 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
> 
> “started to pass” followed by “has imported” doesn’t sound right.

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

[PATCH v3 08/10] test-lib: fail on unexpectedly passing tests

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 16b785f3b91..2b63e1c86ca 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -130,7 +130,7 @@ test_expect_success 'subtest: a failing TODO test' '
 '
 
 test_expect_success 'subtest: a passing TODO test' '
-	write_and_run_sub_test_lib_test passing-todo <<-\EOF &&
+	write_and_run_sub_test_lib_test_err passing-todo <<-\EOF &&
 	test_expect_failure "pretend we have fixed a known breakage" "true"
 	test_done
 	EOF
@@ -142,7 +142,7 @@ test_expect_success 'subtest: a passing TODO test' '
 '
 
 test_expect_success 'subtest: 2 TODO tests, one passin' '
-	write_and_run_sub_test_lib_test partially-passing-todos <<-\EOF &&
+	write_and_run_sub_test_lib_test_err partially-passing-todos <<-\EOF &&
 	test_expect_failure "pretend we have a known breakage" "false"
 	test_expect_success "pretend we have a passing test" "true"
 	test_expect_failure "pretend we have fixed another known breakage" "true"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0a124ffad38..5352209d3e4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1272,7 +1272,14 @@ test_done () {
 
 		check_test_results_san_file_ "$test_failure"
 
-		if test -z "$skip_all" && test -n "$invert_exit_code"
+		if test "$test_fixed" != 0
+		then
+			if test -z "$invert_exit_code"
+			then
+				GIT_EXIT_OK=t
+				exit 1
+			fi
+		elif test -z "$skip_all" && test -n "$invert_exit_code"
 		then
 			say_color warn "# faking up non-zero exit with --invert-exit-code"
 			GIT_EXIT_OK=t

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

[PATCH v3 09/10] meson: introduce kwargs variable for tests

[Patrick Steinhardt]

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(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index 3d74547c8ae..16fa69e317e 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -17,6 +17,6 @@ if get_option('tests')
     workdir: meson.current_source_dir(),
     env: credential_netrc_testenv,
     depends: test_dependencies + bin_wrappers + [credential_netrc],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 63714166a61..98dd8e0c8ea 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -21,7 +21,7 @@ if get_option('tests')
     env: subtree_test_environment,
     workdir: meson.current_source_dir() / 't',
     depends: test_dependencies + bin_wrappers + [ git_subtree ],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
 
diff --git a/meson.build b/meson.build
index a1476e5b322..6fb898a21d1 100644
--- a/meson.build
+++ b/meson.build
@@ -2036,6 +2036,10 @@ subdir('templates')
 # can properly set up test dependencies. The bin-wrappers themselves are set up
 # at configuration time, so these are fine.
 if get_option('tests')
+  test_kwargs = {
+    'timeout': 0,
+  }
+
   subdir('t')
 endif
 
diff --git a/t/meson.build b/t/meson.build
index fcfc1c2c2ba..3fc8c6c2201 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -51,7 +51,7 @@ clar_unit_tests = executable('unit-tests',
   sources: clar_sources + clar_test_suites,
   dependencies: [libgit_commonmain],
 )
-test('unit-tests', clar_unit_tests)
+test('unit-tests', clar_unit_tests, kwargs: test_kwargs)
 
 unit_test_programs = [
   'unit-tests/t-reftable-basics.c',
@@ -76,7 +76,7 @@ foreach unit_test_program : unit_test_programs
   )
   test(unit_test_name, unit_test,
     workdir: meson.current_source_dir(),
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 
@@ -1212,7 +1212,7 @@ foreach integration_test : integration_tests
     workdir: meson.current_source_dir(),
     env: test_environment,
     depends: test_dependencies + bin_wrappers,
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

[PATCH v3 10/10] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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(+)

diff --git a/meson.build b/meson.build
index 6fb898a21d1..46c5d068e05 100644
--- a/meson.build
+++ b/meson.build
@@ -2040,6 +2040,14 @@ if get_option('tests')
     'timeout': 0,
   }
 
+  # The TAP protocol was already understood by previous versions of Meson, but
+  # it was incompatible with the `meson test --interactive` flag.
+  if meson.version().version_compare('>=1.8.0')
+    test_kwargs += {
+      'protocol': 'tap',
+    }
+  endif
+
   subdir('t')
 endif
 

-- 
2.50.0.rc0.604.gd4ff7b7c86.dirty

Re: [PATCH v3 00/10] meson: parse TAP output generated by our tests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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
>

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

- Karthik

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v4 00/10] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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

[PATCH v4 01/10] t: stop announcing prereqs

[Patrick Steinhardt]

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(-)

diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 5c9dc90d0b0..ca8568067d3 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -10,53 +10,35 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 auml=$(printf '\303\244')
 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
-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" '
+test_expect_success CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
 	test $(git config --bool core.ignorecase) = true
 '
-else
-test_expect_success "detection of case insensitive filesystem during repo init" '
+
+test_expect_success !CASE_INSENSITIVE_FS "detection of case insensitive filesystem during repo init" '
 	{
 		test_must_fail git config --bool core.ignorecase >/dev/null ||
 			test $(git config --bool core.ignorecase) = false
 	}
 '
-fi
 
-if test_have_prereq SYMLINKS
-then
-test_expect_success "detection of filesystem w/o symlink support during repo init" '
+test_expect_success SYMLINKS "detection of filesystem w/o symlink support during repo init" '
 	{
 		test_must_fail git config --bool core.symlinks ||
 		test "$(git config --bool core.symlinks)" = true
 	}
 '
-else
-test_expect_success "detection of filesystem w/o symlink support during repo init" '
+
+test_expect_success !SYMLINKS "detection of filesystem w/o symlink support during repo init" '
 	v=$(git config --bool core.symlinks) &&
 	test "$v" = false
 '
-fi
 
 test_expect_success "setup case tests" '
 	git config core.ignorecase true &&
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 98259e2adaa..1f16e6b5228 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -17,11 +17,6 @@ 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" &&
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index a51f881b1c9..32b14e3a714 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -36,7 +36,7 @@ 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
 
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 7679780fb87..578d6c8b329 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -700,19 +700,17 @@ test_expect_success \
 # ----------------------------------------------------------------------
 # syntax highlighting
 
+test_lazy_prereq HIGHLIGHT '
+	highlight_version=$(highlight --version </dev/null 2>/dev/null) &&
+	test -n "$highlight_version"
+'
 
-highlight_version=$(highlight --version </dev/null 2>/dev/null)
-if [ $? -eq 127 ]; then
-	say "Skipping syntax highlighting tests: 'highlight' not found"
-elif test -z "$highlight_version"; then
-	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
-else
-	test_set_prereq HIGHLIGHT
+test_expect_success HIGHLIGHT '
 	cat >>gitweb_config.perl <<-\EOF
 	our $highlight_bin = "highlight";
-	$feature{'highlight'}{'override'} = 1;
+	$feature{"highlight"}{"override"} = 1;
 	EOF
-fi
+'
 
 test_expect_success HIGHLIGHT \
 	'syntax highlighting (no highlight, unknown syntax)' \
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index d667dda654e..637a6f13a6d 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -66,10 +66,6 @@ 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.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 02/10] t: silence output from `test_create_repo()`

[Patrick Steinhardt]

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(-)

diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index b3cf53ff8c9..2cd0be7ba76 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -30,7 +30,7 @@ setup_repo() {
 
 test_repo=test
 push_repo() {
-	test_create_repo $test_repo
+	git init --quiet $test_repo
 	cd $test_repo
 
 	setup_repo
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 28f9d83d4c1..4d4aa1650fe 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -48,11 +48,12 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
-fullhead1=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup submodule' '
+	git init sm1 &&
+	add_file . foo &&
+	head1=$(add_file sm1 foo1 foo2) &&
+	fullhead1=$(cd sm1 && git rev-parse --verify HEAD)
+'
 
 test_expect_success 'added submodule' '
 	git add sm1 &&
@@ -235,10 +236,13 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 	test_cmp expected actual
 '
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
-fullhead6=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup submodule anew' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7) &&
+	fullhead6=$(cd sm1 && git rev-parse --verify HEAD)
+'
+
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=log HEAD >actual &&
 	cat >expected <<-EOF &&
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 76b83101d3b..dbfeb7470bc 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -363,9 +363,12 @@ test_expect_success 'typechanged submodule(submodule->blob)' '
 	diff_cmp expected actual
 '
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
+test_expect_success 'setup' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7)
+'
+
 test_expect_success 'nonexistent commit' '
 	git diff-index -p --submodule=diff HEAD >actual &&
 	cat >expected <<-EOF &&
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 9c3cc4cf404..66c3ec2da22 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -38,10 +38,11 @@ commit_file () {
 	git commit "$@" -m "Commit $*" >/dev/null
 }
 
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
+test_expect_success 'setup submodule' '
+	git init sm1 &&
+	add_file . foo &&
+	head1=$(add_file sm1 foo1 foo2)
+'
 
 test_expect_success 'added submodule' "
 	git add sm1 &&
@@ -214,9 +215,12 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
 	test_cmp expected actual
 "
 
-rm -f sm1 &&
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
+test_expect_success 'setup submodule' '
+	rm -f sm1 &&
+	git init sm1 &&
+	head6=$(add_file sm1 foo6 foo7)
+'
+
 test_expect_success 'nonexistent commit' "
 	git submodule summary >actual &&
 	cat >expected <<-EOF &&

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 03/10] t9822: use prereq to check for ISO-8859-1 support

[Patrick Steinhardt]

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(-)

diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 572d395498e..e6e07facd4b 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -7,12 +7,17 @@ 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" &&
+	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

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 04/10] t983*: use prereq to check for Python-specific git-p4(1) support

[Patrick Steinhardt]

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(-)

diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 6116f806f63..b969c7e0d5a 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -12,23 +12,25 @@ failing, and produces maximally sane output in git.'
 ## SECTION REPEATED IN t9836 ##
 ###############################
 
+EXTRA_PATH="$(pwd)/temp_python"
+mkdir "$EXTRA_PATH"
+PATH="$EXTRA_PATH:$PATH"
+export PATH
+
 # 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)
-if test -n "$python_target_binary"
-then
-	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH"
-	export PATH
-
-	write_script temp_python/git-p4-python2 <<-EOF
+test_lazy_prereq P4_PYTHON2 '
+	python_target_binary=$(which python2) &&
+	test -n "$python_target_binary" &&
+	write_script "$EXTRA_PATH"/git-p4-python2 <<-EOF &&
 	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
 	EOF
-fi
+	( git p4-python2 || true ) >err &&
+	test_grep "valid commands" err
+'
 
-git p4-python2 >err
-if ! grep 'valid commands' err
+if ! test_have_prereq P4_PYTHON2
 then
 	skip_all="skipping python2 git p4 tests; python2 not available"
 	test_done
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 5e5217a66b4..da6669bf711 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -12,23 +12,25 @@ failing, and produces maximally sane output in git.'
 ## SECTION REPEATED IN t9835 ##
 ###############################
 
+EXTRA_PATH="$(pwd)/temp_python"
+mkdir "$EXTRA_PATH"
+PATH="$EXTRA_PATH:$PATH"
+export PATH
+
 # 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)
-if test -n "$python_target_binary"
-then
-	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH"
-	export PATH
-
-	write_script temp_python/git-p4-python3 <<-EOF
+test_lazy_prereq P4_PYTHON3 '
+	python_target_binary=$(which python3) &&
+	test -n "$python_target_binary" &&
+	write_script "$EXTRA_PATH"/git-p4-python3 <<-EOF &&
 	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
 	EOF
-fi
+	( git p4-python3 || true ) >err &&
+	test_grep "valid commands" err
+'
 
-git p4-python3 >err
-if ! grep 'valid commands' err
+if ! test_have_prereq P4_PYTHON3
 then
 	skip_all="skipping python3 git p4 tests; python3 not available"
 	test_done

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 05/10] t/test-lib: don't print shell traces to stdout

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 35c5c2b4f9b..16b785f3b91 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -219,41 +219,44 @@ test_expect_success 'subtest: --verbose option' '
 	test_expect_success "failing test" false
 	test_done
 	EOF
-	mv t1234-verbose/out t1234-verbose/out+ &&
-	grep -v "^Initialized empty" t1234-verbose/out+ >t1234-verbose/out &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
-	> expecting success of 1234.1 '\''passing test'\'': true
+	mv t1234-verbose/err t1234-verbose/err+ &&
+	grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err &&
+	check_sub_test_lib_test_err t1234-verbose \
+		<<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
+	> ok 2 - test with output
+	> not ok 3 - failing test
+	> #	false
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF_OUT
+	> expecting success of 1234.1 '\''passing test'\'': true
 	> Z
 	> expecting success of 1234.2 '\''test with output'\'': echo foo
 	> foo
-	> ok 2 - test with output
 	> Z
 	> expecting success of 1234.3 '\''failing test'\'': false
-	> not ok 3 - failing test
-	> #	false
 	> Z
-	> # failed 1 among 3 test(s)
-	> 1..3
-	EOF
+	EOF_ERR
 '
 
 test_expect_success 'subtest: --verbose-only option' '
 	run_sub_test_lib_test_err \
 		t1234-verbose \
 		--verbose-only=2 &&
-	check_sub_test_lib_test t1234-verbose <<-\EOF
+	check_sub_test_lib_test_err t1234-verbose <<-\EOF_OUT 3<<-\EOF_ERR
 	> ok 1 - passing test
-	> Z
-	> expecting success of 1234.2 '\''test with output'\'': echo foo
-	> foo
 	> ok 2 - test with output
-	> Z
 	> not ok 3 - failing test
 	> #	false
 	> # failed 1 among 3 test(s)
 	> 1..3
-	EOF
+	EOF_OUT
+	> Z
+	> expecting success of 1234.2 '\''test with output'\'': echo foo
+	> foo
+	> Z
+	EOF_ERR
 '
 
 test_expect_success 'subtest: skip one with GIT_SKIP_TESTS' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index af722d383d9..6ce8570226c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -707,7 +707,7 @@ then
 	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
 elif test "$verbose" = "t"
 then
-	exec 4>&2 3>&1
+	exec 4>&2 3>&2
 else
 	exec 4>/dev/null 3>/dev/null
 fi
@@ -949,7 +949,7 @@ maybe_setup_verbose () {
 	test -z "$verbose_only" && return
 	if match_pattern_list $test_count "$verbose_only"
 	then
-		exec 4>&2 3>&1
+		exec 4>&2 3>&2
 		# Emit a delimiting blank line when going from
 		# non-verbose to verbose.  Within verbose mode the
 		# delimiter is printed by test_expect_*.  The choice

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 06/10] t/test-lib: fix TAP format for BASH_XTRACEFD warning

[Patrick Steinhardt]

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(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 6ce8570226c..8c0d76ea5f0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -470,7 +470,7 @@ then
 	then
 		: Executed by a Bash version supporting BASH_XTRACEFD.  Good.
 	else
-		echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+		echo >&2 "# warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
 		trace=
 	fi
 fi

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 07/10] t7815: fix unexpectedly passing test on macOS

[Patrick Steinhardt]

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(-)

diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index b7d83f9a5de..55d5e6de17c 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 !CYGWIN,!MACOS 'git grep .fi a' '
 	git grep .fi a
 '
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8c0d76ea5f0..0a124ffad38 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1636,6 +1636,9 @@ fi
 # Fix some commands on Windows, and other OS-specific things
 uname_s=$(uname -s)
 case $uname_s in
+Darwin)
+	test_set_prereq MACOS
+	;;
 *MINGW*)
 	# Windows has its own (incompatible) sort and find
 	sort () {

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 08/10] test-lib: fail on unexpectedly passing tests

[Patrick Steinhardt]

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(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 16b785f3b91..2b63e1c86ca 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -130,7 +130,7 @@ test_expect_success 'subtest: a failing TODO test' '
 '
 
 test_expect_success 'subtest: a passing TODO test' '
-	write_and_run_sub_test_lib_test passing-todo <<-\EOF &&
+	write_and_run_sub_test_lib_test_err passing-todo <<-\EOF &&
 	test_expect_failure "pretend we have fixed a known breakage" "true"
 	test_done
 	EOF
@@ -142,7 +142,7 @@ test_expect_success 'subtest: a passing TODO test' '
 '
 
 test_expect_success 'subtest: 2 TODO tests, one passin' '
-	write_and_run_sub_test_lib_test partially-passing-todos <<-\EOF &&
+	write_and_run_sub_test_lib_test_err partially-passing-todos <<-\EOF &&
 	test_expect_failure "pretend we have a known breakage" "false"
 	test_expect_success "pretend we have a passing test" "true"
 	test_expect_failure "pretend we have fixed another known breakage" "true"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0a124ffad38..5352209d3e4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1272,7 +1272,14 @@ test_done () {
 
 		check_test_results_san_file_ "$test_failure"
 
-		if test -z "$skip_all" && test -n "$invert_exit_code"
+		if test "$test_fixed" != 0
+		then
+			if test -z "$invert_exit_code"
+			then
+				GIT_EXIT_OK=t
+				exit 1
+			fi
+		elif test -z "$skip_all" && test -n "$invert_exit_code"
 		then
 			say_color warn "# faking up non-zero exit with --invert-exit-code"
 			GIT_EXIT_OK=t

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 09/10] meson: introduce kwargs variable for tests

[Patrick Steinhardt]

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(-)

diff --git a/contrib/credential/netrc/meson.build b/contrib/credential/netrc/meson.build
index 3d74547c8ae..16fa69e317e 100644
--- a/contrib/credential/netrc/meson.build
+++ b/contrib/credential/netrc/meson.build
@@ -17,6 +17,6 @@ if get_option('tests')
     workdir: meson.current_source_dir(),
     env: credential_netrc_testenv,
     depends: test_dependencies + bin_wrappers + [credential_netrc],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
diff --git a/contrib/subtree/meson.build b/contrib/subtree/meson.build
index 63714166a61..98dd8e0c8ea 100644
--- a/contrib/subtree/meson.build
+++ b/contrib/subtree/meson.build
@@ -21,7 +21,7 @@ if get_option('tests')
     env: subtree_test_environment,
     workdir: meson.current_source_dir() / 't',
     depends: test_dependencies + bin_wrappers + [ git_subtree ],
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endif
 
diff --git a/meson.build b/meson.build
index a1476e5b322..6fb898a21d1 100644
--- a/meson.build
+++ b/meson.build
@@ -2036,6 +2036,10 @@ subdir('templates')
 # can properly set up test dependencies. The bin-wrappers themselves are set up
 # at configuration time, so these are fine.
 if get_option('tests')
+  test_kwargs = {
+    'timeout': 0,
+  }
+
   subdir('t')
 endif
 
diff --git a/t/meson.build b/t/meson.build
index fcfc1c2c2ba..3fc8c6c2201 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -51,7 +51,7 @@ clar_unit_tests = executable('unit-tests',
   sources: clar_sources + clar_test_suites,
   dependencies: [libgit_commonmain],
 )
-test('unit-tests', clar_unit_tests)
+test('unit-tests', clar_unit_tests, kwargs: test_kwargs)
 
 unit_test_programs = [
   'unit-tests/t-reftable-basics.c',
@@ -76,7 +76,7 @@ foreach unit_test_program : unit_test_programs
   )
   test(unit_test_name, unit_test,
     workdir: meson.current_source_dir(),
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 
@@ -1212,7 +1212,7 @@ foreach integration_test : integration_tests
     workdir: meson.current_source_dir(),
     env: test_environment,
     depends: test_dependencies + bin_wrappers,
-    timeout: 0,
+    kwargs: test_kwargs,
   )
 endforeach
 

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

[PATCH v4 10/10] meson: parse TAP output generated by our tests

[Patrick Steinhardt]

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(+)

diff --git a/meson.build b/meson.build
index 6fb898a21d1..46c5d068e05 100644
--- a/meson.build
+++ b/meson.build
@@ -2040,6 +2040,14 @@ if get_option('tests')
     'timeout': 0,
   }
 
+  # The TAP protocol was already understood by previous versions of Meson, but
+  # it was incompatible with the `meson test --interactive` flag.
+  if meson.version().version_compare('>=1.8.0')
+    test_kwargs += {
+      'protocol': 'tap',
+    }
+  endif
+
   subdir('t')
 endif
 

-- 
2.50.0.rc0.629.g846fc57c9e.dirty

Re: [PATCH v4 00/10] meson: parse TAP output generated by our tests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4495 bytes --]

Hello

> 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

The range-diff looks great. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]