Re: [PATCH v2 1/6] t0300: don't create unused file

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 6, 2023, 08:34 UTC
Message-ID: <230406.86ttxttnir.gmgdl@evledraar.gmail.com>
In-Reply-To: <20230403223338.468025-2-rybak.a.v@gmail.com>

On Tue, Apr 04 2023, Andrei Rybak wrote:

Show 27 quoted lines

> Test 'credential config with partial URLs' in t0300-credentials.sh
> contains three "git credential fill" invocations.  For two of the
> invocations, the test asserts presence or absence of string "yep" in the
> standard output.  For the third test it checks for an error message in
> standard error.
>
> Don't redirect standard output of "git credential" to file "stdout" in
> t0300-credentials.sh to avoid creating an unnecessary file when only
> standard error is checked.
>
> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
> ---
>  t/t0300-credentials.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
> index c66d91e82d..b8612ede95 100755
> --- a/t/t0300-credentials.sh
> +++ b/t/t0300-credentials.sh
> @@ -808,7 +808,7 @@ test_expect_success 'credential config with partial URLs' '
>  
>  	git -c credential.$partial.helper=yep \
>  		-c credential.with%0anewline.username=uh-oh \
> -		credential fill <stdin >stdout 2>stderr &&
> +		credential fill <stdin 2>stderr &&
>  	test_i18ngrep "skipping credential lookup for key" stderr
>  '

This goes for these changes in this series general: You're correct that this is useless now, but I don't think it follows that we should be removing the "redundant" code in all cases, rather than fixing the test to actually check these.

E.g. this will also make this test pass:
	
	diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
	index c66d91e82d8..62c2a0fd50e 100755
	--- a/t/t0300-credentials.sh
	+++ b/t/t0300-credentials.sh
	@@ -806,9 +806,11 @@ test_expect_success 'credential config with partial URLs' '
	 		return 1
	 	done &&
	 
	+	cp stdout stdout.last &&
	 	git -c credential.$partial.helper=yep \
	 		-c credential.with%0anewline.username=uh-oh \
	 		credential fill <stdin >stdout 2>stderr &&
	+	test_cmp stdout.last stdout &&
	 	test_i18ngrep "skipping credential lookup for key" stderr
	 '

Does that make sense? No idea, I don't know the credential system well.

But isn't it worth testing that when we ask for this that we're getting some known output along with the warning?