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

From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Derrick Stolee" <derrickstolee@github.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 1/2] t5516: move plaintext-password tests from t5601 and t5516
Date: Tue, 1 Nov 2022 16:37:41 -0400	[thread overview]
Message-ID: <Y2GDlZBzZqR+lwyU@nand.local> (raw)
In-Reply-To: <Y2DLdsM2CYY3Z2Sc@coredump.intra.peff.net>

On Tue, Nov 01, 2022 at 03:32:06AM -0400, Jeff King wrote:
> 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).

Agreed. Despite some of the niceties you mention, I concur that testing
http(s) is more realistic and worth doing.

Thanks,
Taylor