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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Oct 31 2022, Jeff King wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>

Whatever our difference of opinion about the usefulness of not asserting
that a thing doesn't get worse, even if it isn't perfect (c.f.:
https://lore.kernel.org/git/221101.86a65b5q9q.gmgdl@evledraar.gmail.com/)
I think that...

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

..this description dosen't match what the patch is doing, okey, so
there's a case where the remote helper swallows the warnings, i.e. we'll
emit fewer than we expected...

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t5551-http-fetch-smart.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index bbe3d5721f..64c6c9f59e 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -597,17 +597,17 @@ test_expect_success 'clone warns or fails when using username:password' '
>  	git -c transfer.credentialsInUrl=warn \
>  		clone $url_userpass attempt2 2>err &&
>  	grep "warning: $message" err >warnings &&
> -	test_line_count = 2 warnings &&
> +	test_line_count -ge 1 warnings &&
>  
>  	test_must_fail git -c transfer.credentialsInUrl=die \
>  		clone $url_userpass attempt3 2>err &&
>  	grep "fatal: $message" err >warnings &&
> -	test_line_count = 1 warnings &&
> +	test_line_count -ge 1 warnings &&
>  
>  	test_must_fail git -c transfer.credentialsInUrl=die \
>  		clone $url_userblank attempt4 2>err &&
>  	grep "fatal: $message" err >warnings &&
> -	test_line_count = 1 warnings
> +	test_line_count -ge 1 warnings
>  '
>  
>  test_expect_success 'clone does not detect username:password when it is https://username@domain:port/' '
> @@ -630,17 +630,17 @@ test_expect_success 'fetch warns or fails when using username:password' '
>  
>  	git -c transfer.credentialsInUrl=warn fetch $url_userpass 2>err &&
>  	grep "warning: $message" err >warnings &&
> -	test_line_count = 3 warnings &&
> +	test_line_count -ge 1 warnings &&
>  
>  	test_must_fail git -c transfer.credentialsInUrl=die \
>  		fetch $url_userpass 2>err &&
>  	grep "fatal: $message" err >warnings &&
> -	test_line_count = 1 warnings &&
> +	test_line_count -ge 1 warnings &&
>  
>  	test_must_fail git -c transfer.credentialsInUrl=die \
>  		fetch $url_userblank 2>err &&
>  	grep "fatal: $message" err >warnings &&
> -	test_line_count = 1 warnings
> +	test_line_count -ge 1 warnings
>  '
>  
>  
> @@ -654,7 +654,7 @@ test_expect_success 'push warns or fails when using username:password' '
>  	test_must_fail git -c transfer.credentialsInUrl=die \
>  		push $url_userpass 2>err &&
>  	grep "fatal: $message" err >warnings &&
> -	test_line_count = 1 warnings
> +	test_line_count -ge 1 warnings
>  '

...but then why are we modifying these codepaths that have nothing to do
with invoking the remote helper, i.e. where we die early before we get
to that?

And even if some of this was invoking that remote helper and we were
modifying it blindly, then presumably the helper swallowing it would
result in 0 some of the time, so "-ge 1" would be wrong.

(That's not the case, but it's why I think the patch doesn't make much
sense).