Subject: Re: [PATCH 1/2] t5516: move plaintext-password tests from t5601 and t5516

From: Jeff King <[email-masked]>
Date: Tue, 1 Nov 2022 03:32:06 -0400

On Tue, Nov 01, 2022 at 04:18:32AM +0100, Ævar Arnfjörð Bjarmason wrote:

> Did you consider just using git://; on the WIP branch I linked to where
> I fixed these tests quite a bit already I left them in their own file in
> anticipation of having to test that (but didn't finish that yet). I.e.:

No, I didn't really consider that. Mostly because I was trying to stick
with the original intent of 6dcbdc0d66 that created them.

If we toss that out, then in theory that widens the options. And in some
ways it's nice to use git://, because it has fewer dependencies and so
is run on more platforms. But it strikes me as a pretty unrealistic
test, just because credentials in git:// URLs make no sense and are not
something anybody would do.

As you note, since the error comes from remote.c, it would still
trigger. But it's a bit more "white box" testing than I think makes
sense here. I prefer the original tests' method of trying to create
plausible real-world scenarios and seeing how they behave (and I think
my patch even improves that, since having an actual server on the other
end is the usual case).

-Peff