Re: [PATCH v10 1/2] t1092: add tests for `git diff-files`

From: Junio C Hamano <gitster@pobox.com>
Date: May 3, 2023, 23:25 UTC
Message-ID: <xmqqpm7hm1yy.fsf@gitster.g>
In-Reply-To: <20230503215549.511999-2-cheskaqiqi@gmail.com>

Shuqi Liang <cheskaqiqi@gmail.com> writes:

Show 5 quoted lines

> +	test_sparse_match git diff-files -- folder2/a &&
> +
> +	write_script edit-contents <<-\EOF &&
> +	echo text >>"$1"
> +	EOF

> +	# Add file to the index but outside of cone for sparse-checkout cases.
> +	# Add file to the index without sparse-checkout cases to ensure all have
> +	# same output.

Are these two sentences supposed to explain the following two commands that are run in all three repositories? As far as I can tell, no command in this test adds anything to the index. Perhaps it is a leftover/stale comment from previous rounds or something?

Show 5 quoted lines

> +	run_on_all mkdir -p folder1 &&
> +	run_on_all cp a folder1/a &&
> +
> +	# file present on-disk with modifications
> +	run_on_all ../edit-contents folder1/a &&

With the above three commands taken together, we have made folder1/a on the working tree different from what is in the index, so we can expect to see differences between the index and the working tree files. "# file present on-disk with modifications" is a good way to summarize a half of what we are trying to achieve, with the other half being that we try to do that to a path outside the cone of interest.

So, perhaps get rid of this comment between the step 2 and 3 of the preparation, and rewrite the comment before the step 1 (i.e. "mkdir -p") of the preparation to explain the whole thing, perhaps like:

	# The directory "folder1" is outside the cone of interest
	# and may not exist in the sparse checkout repositories.
        # Create it as needed, add file "folder1/a" there with
	# contents that is different from the staged version.

to explain what scenario these three run_on_all commands are trying to create?

> +	test_all_match git diff-files &&
> +	test_all_match git diff-files folder1/a &&
> +	test_all_match git diff-files "folder*/a"

I think Victoria suggested to use the double-dash disambiguators for these tests, and it may not be a bad idea to do so, i.e.

	test_all_match git diff-files &&
	test_all_match git diff-files -- folder1/a &&
	test_all_match git diff-files -- folder\*/a

> +'
> +
>  test_done

Thanks.