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

From: Jeff King <peff@peff.net>
Date: Oct 31, 2022, 23:22 UTC
Message-ID: <Y2BYyB3H9n2LDo/E@coredump.intra.peff.net>
In-Reply-To: <98fa5270cb720f2f038c4bb9571c7d306ff5d759.1667245639.git.gitgitgadget@gmail.com>

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

Show 10 quoted lines

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

Show 10 quoted lines

> 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