From faac9d46e0e2e87d87b0b4e2b9afbf68deaf234d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:41 +0200 Subject: [PATCH 01/10] t: stop announcing prereqs 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 Signed-off-by: Junio C Hamano --- 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 5c9dc90d0b..ca8568067d 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 98259e2ada..1f16e6b522 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 a51f881b1c..32b14e3a71 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 7679780fb8..578d6c8b32 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) && + test -n "$highlight_version" +' -highlight_version=$(highlight --version /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 d667dda654..637a6f13a6 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 From ddfcb9d466a4f4eab6c5bd578c9b13263d4ba610 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:42 +0200 Subject: [PATCH 02/10] t: silence output from `test_create_repo()` 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 Signed-off-by: Junio C Hamano --- 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 b3cf53ff8c..2cd0be7ba7 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 28f9d83d4c..4d4aa1650f 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 76b83101d3..dbfeb7470b 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 9c3cc4cf40..66c3ec2da2 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 && From 844537091d3ab00aaec73bebd6b4c70adfd998fb Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:43 +0200 Subject: [PATCH 03/10] t9822: use prereq to check for ISO-8859-1 support 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 Signed-off-by: Junio C Hamano --- 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 572d395498..e6e07facd4 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 From a1199a23896c674f57f3942358e2f05b3a075e7a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:44 +0200 Subject: [PATCH 04/10] t983*: use prereq to check for Python-specific git-p4(1) support 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 Signed-off-by: Junio C Hamano --- 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 6116f806f6..b969c7e0d5 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 5e5217a66b..da6669bf71 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 From d411d3d837ece4f29b2a23df0bf537795e2dee56 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:45 +0200 Subject: [PATCH 05/10] t/test-lib: don't print shell traces to stdout 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 Signed-off-by: Junio C Hamano --- 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 35c5c2b4f9..16b785f3b9 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 af722d383d..6ce8570226 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 From d4ea24b8a9ea7d4778cb7e5c7938d2a74e74a33c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:46 +0200 Subject: [PATCH 06/10] 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 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 Signed-off-by: Junio C Hamano --- 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 6ce8570226..8c0d76ea5f 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 From d3d8c601fdb8f37eba45fc784ea246107bf88879 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:47 +0200 Subject: [PATCH 07/10] t7815: fix unexpectedly passing test on macOS 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 Signed-off-by: Junio C Hamano --- 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 b7d83f9a5d..55d5e6de17 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 8c0d76ea5f..0a124ffad3 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 () { From 5e0752b071b7e5a702c2008391505c7a39d0bd01 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:48 +0200 Subject: [PATCH 08/10] test-lib: fail on unexpectedly passing tests 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 Signed-off-by: Junio C Hamano --- 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 16b785f3b9..2b63e1c86c 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 0a124ffad3..5352209d3e 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 From b44e63f405a6508652dab2d4ade71e6f0afb9f36 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:49 +0200 Subject: [PATCH 09/10] meson: introduce kwargs variable for tests 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 Signed-off-by: Junio C Hamano --- 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 3d74547c8a..16fa69e317 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 63714166a6..98dd8e0c8e 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 a1476e5b32..6fb898a21d 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 fcfc1c2c2b..3fc8c6c220 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 From c1bc9749234773ce178d86d7d39cd28f2fa4288a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Jun 2025 08:44:50 +0200 Subject: [PATCH 10/10] meson: parse TAP output generated by our tests 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 Signed-off-by: Junio C Hamano --- meson.build | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/meson.build b/meson.build index 6fb898a21d..46c5d068e0 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