On 7/21/2018 3:21 AM, Eric Sunshine wrote:
On Sat, Jul 21, 2018 at 2:34 AM Elijah Newren <new...@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...

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

Reply via email to