Re: [PATCH] t5500: fix mistaken $SERVER reference in helper function

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Jeff King wrote:

> This happens to work out because the "server" directory from the first
> test is still hanging around, and the contents of the two are identical.
> But it was clearly not the intended behavior, and is fragile to cleaning
> up the leftovers from the first test.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t5500-fetch-pack.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Restating to make sure I understand correctly.

fetch_filter_blob_limit_zero is a helper for parameterized tests
taking two parameters.  The first is the path to a repository we will
clone from, and the second is a clone URL to clone from that
repository.  (That way, we can reuse the same logic for multiple URL
schemes.)

Those tests each do the following:

- set up $SERVER containing a test commit and allowing partial clone
- clone from $URL to client
- make a new commit in $SERVER, that client doesn't have
- fetch to catch up, with --filter=blob:none
- assert that the new commit was fetched and new blob wasn't

And in that assertion, we want to get the name of the new commit and
new blob from $SERVER, not client, since we wouldn't want a side
effect of causing them to be fetched in the process.

Alas, in a copy-and-paste gone wrong, 07ef3c6604 gets the name of the
blob (but not the commit) from "server" instead of $SERVER.  And this
happens to work because the first time we call this helper, $SERVER is
"server".  The only reason this happens to work at all is that we're
looking at a blob id; if we looked at the commit id, then the
timestamps wouldn't have matched.

Thanks, the fix is obviously correct.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Particularly telling that the author of 07ef3c6604 introduced this
typo while trying to make the tests _more_ robust.

Once the library code is ready for it, this might be a good candidate
for moving most of the test cases into unit tests and just having one
or two less repetitive integration tests.

Thanks for catching and fixing it!

Jonathan