Subject: Re: [PATCH v3 2/3] t1092: add tests for `git check-attr`

From: Victoria Dye <[email-masked]>
Date: Tue, 11 Jul 2023 13:47:18 -0700

Shuqi Liang wrote:
> Add smudge/clean filters in .gitattributes files inside the affected
> sparse directories in test 'merge with conflict outside cone', make sure
> it behaves as expected when path is outside of sparse-checkout.
> 
> Add tests for `git check-attr`, make sure it behaves as expected when
> path is both inside or outside of sparse-checkout definition.
> 
> Helped-by: Victoria Dye <[email-masked]>
> Signed-off-by: Shuqi Liang <[email-masked]>
> ---
>  t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 8a95adf4b5..839e08d8dd 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1006,6 +1006,17 @@ test_expect_success 'merge with conflict outside cone' '
>  
>  	test_all_match git checkout -b merge-tip merge-left &&
>  	test_all_match git status --porcelain=v2 &&
> +
> +	echo "a filter=rot13" >>.gitattributes &&
> +	run_on_sparse mkdir folder1 &&
> +	run_on_all cp ../.gitattributes ./folder1 &&
> +	git -C full-checkout add folder1/.gitattributes &&
> +	run_on_sparse git add --sparse folder1/.gitattributes &&
> +	run_on_all git commit -m "add .gitattributes" &&
> +	test_sparse_match git sparse-checkout reapply &&
> +	git config filter.rot13.clean "tr 'A-Za-z' 'N-ZA-Mn-za-m'" &&
> +	git config filter.rot13.smudge "tr 'A-Za-z' 'N-ZA-Mn-za-m'" &&
> +

In general, we try to add tests demonstrating behavior in context with the
implementation of that behavior. Patch 1 [1] contains the update to
attribute reading that's being tested here, so this block should be moved
there accordingly.

Also, does this test fail before patch 1 but succeed after? It's a bit
difficult to tell how this demonstrates that the in-sparse-directory
`.gitattributes` is applied properly now but wasn't before. An
additional comment in the test or commit message would be helpful for
understanding it better.

[1] https://lore.kernel.org/git/[email-masked]/

>  	test_all_match test_must_fail git merge -m merge merge-right &&
>  	test_all_match git status --porcelain=v2 &&
>