Subject: Re: [PATCH 1/2] t9902: verify that completion does not print anything

From: Junio C Hamano <[email-masked]>
Date: Thu, 11 Jan 2024 12:29:56 -0800

Patrick Steinhardt <[email-masked]> writes:

> The Bash completion script must not print anything to either stdout or
> stderr. Instead, it is only expected to populate certain variables.
> Tighten our `test_completion ()` test helper to verify this requirement.
>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>  t/t9902-completion.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index aa9a614de3..78cb93bea7 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -87,9 +87,11 @@ test_completion ()
>  	else
>  		sed -e 's/Z$//' |sort >expected
>  	fi &&
> -	run_completion "$1" &&
> +	run_completion "$1" >"$TRASH_DIRECTORY"/output 2>&1 &&

OK, there is a call to test_completion after cd and this is a
reasonable way to make sure we write to a known location.

I would have done the "run_completion should be totally silent"
check immediately after this line, as they logically are more
related to each other than to the two lines that implement "the
'out' file when sorted must match the expectation we just prepared",
but that is not a huge deal.

The filename "output" is something that may tempt folks who add more
tests to this file to use for their own purpose; because we use it
only three times locally here, it might be safer to give it a bit
more specific name, e.g., "run-completion-output" or something.

>  	sort out >out_sorted &&
> -	test_cmp expected out_sorted
> +	test_cmp expected out_sorted &&
> +	test_must_be_empty "$TRASH_DIRECTORY"/output &&
> +	rm "$TRASH_DIRECTORY"/output
>  }
>  
>  # Test __gitcomp.