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

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