Re: [PATCH v6] sparse-checkout: optimize string_list construction and add tests to verify deduplication.

From: Derrick Stolee <stolee@gmail.com>
Date: Jan 20, 2026, 20:37 UTC
Message-ID: <8a4430e9-26d6-4bc5-bb5d-9896c2a2df9f@gmail.com>
In-Reply-To: <20260120153829.48044-1-amishhhaaaa@gmail.com>

On 1/20/2026 10:38 AM, amisha wrote:

> From: Amisha Chhajed <amishhhaaaa@gmail.com>

Code is the same as last time, which is good.

Show 48 quoted lines

> +test_expect_success 'sparse-checkout deduplicates repeated cone patterns' '
> +    rm -f repo/.git/info/sparse-checkout &&
> +    git -C repo sparse-checkout init --cone &&
> +    git -C repo sparse-checkout add --stdin <<-\EOF &&
> +	foo/bar/baz
> +	a/b/c
> +	foo/bar/baz
> +	a/b
> +	EOF
> +    cat >expect <<-\EOF &&
> +	/*
> +	!/*/
> +	/a/
> +	!/a/*/
> +	/foo/
> +	!/foo/*/
> +	/foo/bar/
> +	!/foo/bar/*/
> +	/a/b/
> +	/foo/bar/baz/
> +	EOF
> +    test_cmp expect repo/.git/info/sparse-checkout
> +'
> +
> +test_expect_success 'sparse-checkout list deduplicates repeated cone patterns' '
> +    rm -f repo/.git/info/sparse-checkout &&
> +    git -C repo sparse-checkout init --cone &&
> +    cat <<-\EOF >repo/.git/info/sparse-checkout &&
> +	/*
> +	!/*/
> +	/a/
> +	!/a/*/
> +	/foo/
> +	!/foo/*/
> +	/foo/bar/
> +	!/foo/bar/*/
> +	/a/b/
> +	/foo/bar/baz/
> +	/foo/bar/baz/
> +	EOF
> +    git -C repo sparse-checkout list >actual &&
> +    cat <<-\EOF >expect &&
> +	a/b
> +	foo/bar/baz
> +	EOF
> +    test_cmp expect actual
> +'
> +

These tests have the right structure, but there's a problem: it appears that you've used four spaces for the first level of indent and then use 8-width tabs for the next level. You can see that it disagrees with the last line of the previous test in the diff context. This should be fixed, and likely "git rebase --whitespace=fix" is how you landed on the current use of tab characters.

I think the content between the EOFs shouldn't be indented more than the 'cat' it's a part of, but I could be incorrect there.

Outside of the whitespace issues, I think this test looks good.

Thanks, -Stolee