Re: [PATCH v2 2/5] t6421: add tests checking for excessive object downloads during merge

[Junio C Hamano <gitster@pobox.com>]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +		for i in `test_seq 1 88`; do
> +			echo content $i >dir/unchanged/file_$i
> +		done &&

        for i in $(...)
        do
                ... || return 1
        done &&

> +test_expect_merge_algorithm failure failure 'Objects downloaded for single relevant rename' '
> +	test_setup_repo &&
> +	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-single &&
> +	(
> +		cd objects-single &&
> +
> +		git rev-list --objects --all --missing=print |
> +			grep '\?' >missing-objects-before &&

The closing and reopening of single quote here is somewhat
misleading.  Wouldn't

		grep "?" >... &&

work just as well?

> +		git rev-list --objects --all --missing=print |
> +			grep ^? >missing-objects-after &&
> +		test_cmp missing-objects-before missing-objects-after |
> +			grep "^[-+]?" >found-and-new-objects &&

I do not think we specify what test_cmp's output looks like; we only
guarantee that it is the right tool to use for expecting two paths
have the same contents, and it may give some human readable output.

Do not assume you can grep in the output; use "diff -u" if you mean
you expect things that exist on the left side and missing from the
right hand side are prefixed with '-' and vice versa for '+'.

Or sort "missing-objects-after" and "missing-objects-before" and run 
"comm" on them, which might be more stable.  Depending on the order
two invocations of rev-list spews out the "missing" objects, you may
even see the same object mentioned with "-" and "+" in the diff output
if lines appear to be moved around.

> +		# We should not have any NEW missing objects
> +		! grep ^+ found-and-new-objects &&
> +		# Fetched 2+1=3 objects, so should have 3 fewer missing objects
> +		test_line_count = 3 found-and-new-objects
> +	)
> +'

I think similar comments apply to the tests in the remainder of the
patch.

Thanks.