Re: [PATCH v3 1/8] t7519: rewrite sparse index test

On 8/19/2021 3:45 AM, Johannes Schindelin wrote:> On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote:

quoted

 test_expect_success 'status succeeds with sparse index' '
-	git reset --hard &&
+	git clone . full &&
+	git clone . sparse &&
+	git -C sparse sparse-checkout init --cone --sparse-index &&

Would it make sense to call `git clone --sparse . sparse`? I see that
there is no support for `--sparse=cone`, which makes me wonder whether we
want that at some stage. In any case, cloning with `--sparse` and then
setting up the cone mode should result in a little less work, right?

The amount of work saved is miniscule, but I can understand wanting to
shave what we can from the cost of tests.

quoted

+	git -C sparse sparse-checkout set dir1 dir2 &&

-	test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" &&
-	check_sparse_index_behavior ! &&
-
-	write_script .git/hooks/fsmonitor-test<<-\EOF &&
+	write_script .git/hooks/fsmonitor-test <<-\EOF &&
 		printf "last_update_token\0"

Technically, the backslash needs to be escaped because it is within double
quotes and we do not want the shell to interpolate the `\0`, but `printf`.
Practically, all the shells I tried handle this as expected.

Also, I have a slight preference for:

		printf "%s\\0" last_update_token

and later

		printf "%s\\0" last_update_token dir1/modified

What do your taste buds say about this?

I have no opinions on this one way or another. I will just point out that
the pattern used in this test is also used throughout the test script, so
any change to that format should be applied universally. The test has been
operating without complaint since it was introduced in 5c8cdcf (fsmonitor:
add test cases for fsmonitor extension, 2017-09-22), so compatibility is
likely not a problem.

Thanks,
-Stolee