Re: [GSOC][PATCH v4] describe: enable sparse index for describe

From: Victoria Dye <vdye@github.com>
Date: Apr 3, 2023, 16:34 UTC
Message-ID: <f8d8ed7f-49cd-d65a-521c-e03d2c552c50@github.com>
In-Reply-To: <20230331182038.224892-1-nanth.raghul@gmail.com>

Raghul Nanth A wrote:

Show 7 quoted lines

> git describe compares the index with the working tree when (and only
> when) it is run with the "--dirty" flag. This is done by the
> run_diff_index() function. The function has been made aware of the
> sparse-index in the series that led to 8d2c3732 (Merge branch
> 'ld/sparse-diff-blame', 2021-12-21). Hence we can just set the
> requires-full-index to false for "describe".
>

This is a good description of the patch and the reasoning behind the changes.

Show 35 quoted lines

> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 801919009e..2b46fb2a48 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1514,6 +1514,36 @@ test_expect_success 'sparse-index is not expanded: stash' '
>  	ensure_not_expanded stash pop
>  '
>  
> +test_expect_success 'describe tested on all' '
> +	init_repos &&
> +
> +	# Add tag to be read by describe
> +
> +	run_on_all git tag -a v1.0 -m "Version 1" &&
> +	test_all_match git describe --dirty &&
> +	run_on_all rm g &&
> +	test_all_match git describe --dirty
> +'
> +
> +
> +test_expect_success 'sparse-index is not expanded: describe' '
> +	init_repos &&
> +
> +	# Add tag to be read by describe
> +
> +	git -C sparse-index tag -a v1.0 -m "Version 1" &&
> +
> +	ensure_not_expanded describe --dirty &&
> +	echo "test" >>sparse-index/g &&
> +	ensure_not_expanded describe --dirty &&
> +	echo "v1.0-dirty" >actual &&
> +
> +	# Check describe on dirty work tree
> +
> +	test_cmp sparse-index-out actual &&

This type of output comparison should be part of 'describe tested on all', not the "ensure not expanded" test; the former tests the correctness of 'git describe', whereas the latter focuses on index expansion. When unit testing, it helps to keep the scope of each test fairly narrow with a specific focus so that they can more easily isolate future regressions.

Show 6 quoted lines

> +	ensure_not_expanded describe
> +'
> +
>  test_expect_success 'sparse index is not expanded: diff' '
>  	init_repos &&
>