Re: [PATCH 1/2] t3507: add a testcase showing failure with sparse checkout

From: Ben Peart <peartben@gmail.com>
Date: Jul 23, 2018, 13:12 UTC
Message-ID: <b56598a1-e543-500a-a39b-cee07fe1e533@gmail.com>
In-Reply-To: <CAPig+cTjQ74gOt8gJbYUBaTRsHBvqtcmucoVdiudFt2TrhCn+g@mail.gmail.com>

On 7/21/2018 3:21 AM, Eric Sunshine wrote:

Show 9 quoted lines

> On Sat, Jul 21, 2018 at 2:34 AM Elijah Newren <newren@gmail.com> wrote:
>> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
>> @@ -392,4 +392,17 @@ test_expect_success 'commit --amend -s places the sign-off at the right place' '
>> +test_expect_failure 'failed cherry-pick with sparse-checkout' '
>> +       pristine_detach initial &&
>> +       git config core.sparseCheckout true &&
> 
> Should this be test_config()?
>

I think using test_config() here is fine but...

Show 14 quoted lines

>> +       echo /unrelated >.git/info/sparse-checkout &&
>> +       git read-tree --reset -u HEAD &&
>> +       test_must_fail git cherry-pick -Xours picked>actual &&
>> +       test_i18ngrep ! "Changes not staged for commit:" actual &&
>> +       echo "/*" >.git/info/sparse-checkout &&
>> +       git read-tree --reset -u HEAD &&
>> +       git config core.sparseCheckout false &&
> 
> See question above.
> 
>> +       rm .git/info/sparse-checkout
> 
> Should this cleanup be done by test_when_finished()?
>

I think trying to use test_when_finished() for this really degrades the readability of the test. See below:

test_expect_success 'failed cherry-pick with sparse-checkout' '
	pristine_detach initial &&
	test_config core.sparsecheckout true &&
	echo /unrelated >.git/info/sparse-checkout &&
	git read-tree --reset -u HEAD &&
	test_when_finished "echo \"/*\" >.git/info/sparse-checkout && git 
read-tree --reset -u HEAD && rm .git/info/sparse-checkout" &&
	test_must_fail git cherry-pick -Xours picked>actual &&
	test_i18ngrep ! "Changes not staged for commit:" actual
'

Given it takes multiple commands, I'd prefer to keep the setup and cleanup of the sparse checkout settings symmetrical.