Re: [PATCH 2/2] t5516/t5601: be less strict about the number of credential warnings

From: Jeff King <peff@peff.net>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/2] t5516/t5601: be less strict about the number of credential warnings
Date: Mon, 31 Oct 2022 19:22:48 -0400	[thread overview]
Message-ID: <Y2BYyB3H9n2LDo/E@coredump.intra.peff.net> (raw)
In-Reply-To: <98fa5270cb720f2f038c4bb9571c7d306ff5d759.1667245639.git.gitgitgadget@gmail.com>

On Mon, Oct 31, 2022 at 07:47:18PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> It is unclear as to _why_, but under certain circumstances the warning
> about credentials being passed as part of the URL seems to be swallowed
> by the `git remote-https` helper in the Windows jobs of Git's CI builds.
> 
> Since it is not actually important how many times Git prints the
> warning/error message, as long as it prints it at least once, let's just
> make the test a bit more lenient and test for the latter instead of the
> former, which works around these CI issues.

This makes sense to me. Regardless of whether we actually fix the
multiple outputs (which are user-visible and kind of ugly), I don't
think there's any reason for our tests to assert the somewhat
undesirable output. So this can proceed independently of any fix.

> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 8dd4610a8c2..980c594940b 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1860,15 +1860,15 @@ test_expect_success LIBCURL 'fetch warns or fails when using username:password'
>  
>  	test_must_fail git -c transfer.credentialsInUrl=warn fetch https://username:password@224.0.0.1 2>err &&
>  	grep "warning: $message" err >warnings &&
> -	test_line_count = 3 warnings &&
> +	test_line_count -ge 1 warnings &&

I think this test_line_count (and all the others) is now superfluous;
the exit code of grep will tell us if we found anything.

I don't mind it too much, though, if we're planning to fix the duplicate
messages, at which point it becomes s/-ge/=/.

-Peff