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

From: Karthik Nayak <karthik.188@gmail.com>
Date: May 31, 2025, 21:00 UTC
Message-ID: <CAOLa=ZQXQh6+B+gDdV=vT++dscCe6e26NWk4kA_1_aEz4=F+bQ@mail.gmail.com>
In-Reply-To: <20250530-pks-meson-tap-v3-1-676f5e41f2e4@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 38 quoted lines

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

Show 8 quoted lines

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

Show 17 quoted lines

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

Show 13 quoted lines

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

Show 12 quoted lines

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