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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Stolee,

On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The sparse index is tested with the FS Monitor hook and extension since
> f8fe49e (fsmonitor: integrate with sparse index, 2021-07-14). This test
> was very fragile because it shared an index across sparse and non-sparse
> behavior. Since that expansion and contraction could cause the index to
> lose its FS Monitor bitmap and token, behavior is fragile to changes in
> 'git sparse-checkout set'.
>
> Rewrite the test to use two clones of the original repo: full and
> sparse. This allows us to also keep the test files (actual, expect,
> trace2.txt) out of the repos we are testing with 'git status'.

Makes sense.

It also should accelerate the test, as it does not have to convert between
sparse and full index all the time.

>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t7519-status-fsmonitor.sh | 38 ++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index deea88d4431..6f2cf306f66 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -389,43 +389,47 @@ test_expect_success 'status succeeds after staging/unstaging' '
>  # If "!" is supplied, then we verify that we do not call ensure_full_index
>  # during a call to 'git status'. Otherwise, we verify that we _do_ call it.
>  check_sparse_index_behavior () {
> -	git status --porcelain=v2 >expect &&
> -	git sparse-checkout init --cone --sparse-index &&
> -	git sparse-checkout set dir1 dir2 &&
> +	git -C full status --porcelain=v2 >expect &&
>  	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
> -		git status --porcelain=v2 >actual &&
> +		git -C sparse status --porcelain=v2 >actual &&
>  	test_region $1 index ensure_full_index trace2.txt &&
>  	test_region fsm_hook query trace2.txt &&
>  	test_cmp expect actual &&
> -	rm trace2.txt &&
> -	git sparse-checkout disable
> +	rm trace2.txt
>  }
>
>  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?

> +	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?

These are only minor nits, of course. I do like the overall patch and
would be fine with it as-is.

Ciao,
Dscho

>  	EOF
> -	git config core.fsmonitor .git/hooks/fsmonitor-test &&
> +	git -C full config core.fsmonitor ../.git/hooks/fsmonitor-test &&
> +	git -C sparse config core.fsmonitor ../.git/hooks/fsmonitor-test &&
>  	check_sparse_index_behavior ! &&
>
> -	write_script .git/hooks/fsmonitor-test<<-\EOF &&
> +	write_script .git/hooks/fsmonitor-test <<-\EOF &&
>  		printf "last_update_token\0"
>  		printf "dir1/modified\0"
>  	EOF
>  	check_sparse_index_behavior ! &&
>
> -	cp -r dir1 dir1a &&
> -	git add dir1a &&
> -	git commit -m "add dir1a" &&
> +	git -C sparse sparse-checkout add dir1a &&
> +
> +	for repo in full sparse
> +	do
> +		cp -r $repo/dir1 $repo/dir1a &&
> +		git -C $repo add dir1a &&
> +		git -C $repo commit -m "add dir1a" || return 1
> +	done &&
> +	git -C sparse sparse-checkout set dir1 dir2 &&
>
>  	# This one modifies outside the sparse-checkout definition
>  	# and hence we expect to expand the sparse-index.
> -	write_script .git/hooks/fsmonitor-test<<-\EOF &&
> +	write_script .git/hooks/fsmonitor-test <<-\EOF &&
>  		printf "last_update_token\0"
>  		printf "dir1a/modified\0"
>  	EOF
> --
> gitgitgadget
>
>