Re: [PATCH v2 1/2] test: git-stash conflict sets up rerere

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2012, 02:37 UTC
Message-ID: <7v8vetmzgz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1341693962-17090-2-git-send-email-hordp@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> Add a failing test to confirm a conflicted stash apply invokes
> rerere to record the conflicts and resolve the the files it can.

OK.

> In this failing state, mergetool may be confused by a left-over
> state from previous rerere activity.

It is unclear to me what relevance this has to this patch. Does it have this sequence:

    "previous rerere activity" (whatever that is)
    test_must_fail git stash apply &&
    git mergetool

and demonstrate that "git mergetool" fails because there is a wrong rerere state in the repository after "git stash apply" returns?

Or perhaps you are relying on the state that is left by the previous test piece?

> Also, the next test expected us to finish up with a reset, which
> is impossible to do if we fail (as we must) and it's an
> unreasonable expectation anyway.  Begin the next test with a reset
> of his own instead.

Yes, it is always a good discipline to start a new test piece from a known state.

Show 16 quoted lines

> @@ -193,7 +203,37 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' '
>      git reset --hard
>  '
>  
> +test_expect_failure 'conflicted stash sets up rerere'  '
> +    git config rerere.enabled true &&
> +    git checkout stash1 &&
> +    echo "Conflicting stash content" >file11 &&
> +    git stash &&
> +    
> +    git checkout --detach stash2 &&
> +    test_must_fail git stash apply &&
> +    
> +    test -e .git/MERGE_RR &&
> +    test -n "$(git ls-files -u)" &&
> +    conflicts="$(git rerere remaining)" &&

Checking that the index is conflicted with "ls-files -u" and asking the public API "git rerere remaining" to see what paths rerere thinks it did not touch, like you do in the second and third lines, are very sensible, but it is probably not a good idea for this test to check implementation details with "test -f .git/MERGE_RR".

Show 12 quoted lines

> +    test "$conflicts" = "file11" &&
> +    output="$(git mergetool --no-prompt)" &&
> +    test "$output" != "No files need merging" &&
> +
> +    git commit -am "save the stash resolution" &&
> +
> +    git reset --hard stash2 &&
> +    test_must_fail git stash apply &&
> +
> +    test -e .git/MERGE_RR &&
> +    test -n "$(git ls-files -u)" &&
> +    conflicts="$(git rerere remaining)" &&

Likewise.

Show 10 quoted lines

> +    test -z "$conflicts" &&
> +    output="$(git mergetool --no-prompt)" &&
> +    test "$output" = "No files need merging"
> +'
> +
>  test_expect_success 'mergetool takes partial path' '
> +    git reset --hard
>      git config rerere.enabled false &&
>      git checkout -b test12 branch1 &&
>      git submodule update -N &&