Re: [PATCH v4 3/5] stash: add test for stashing in a detached state

From: Joel Teichroeb <joel@teichroeb.net>
Date: Jun 13, 2017, 19:48 UTC
Message-ID: <CA+CzEk8U6P58OqruPkP1HePFurNWjgf=Q-h=Hu57zoHpDeenmA@mail.gmail.com>
In-Reply-To: <xmqqa85b65a8.fsf@gitster.mtv.corp.google.com>

On Tue, Jun 13, 2017 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 32 quoted lines

> Joel Teichroeb <joel@teichroeb.net> writes:
>
>> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>
>> ---
>>  t/t3903-stash.sh | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
>> index 5399fb05ca..ce4c8fe3d6 100755
>> --- a/t/t3903-stash.sh
>> +++ b/t/t3903-stash.sh
>> @@ -822,6 +822,18 @@ test_expect_success 'create with multiple arguments for the message' '
>>       test_cmp expect actual
>>  '
>>
>> +test_expect_success 'create in a detached state' '
>> +     test_when_finished "git checkout master" &&
>> +     git checkout HEAD~1 &&
>> +     >foo &&
>> +     git add foo &&
>> +     STASH_ID=$(git stash create) &&
>> +     HEAD_ID=$(git rev-parse --short HEAD) &&
>> +     echo "WIP on (no branch): ${HEAD_ID} initial" >expect &&
>> +     git show --pretty=%s -s ${STASH_ID} >actual &&
>> +     test_cmp expect actual
>> +'
>
> Hmph.  Is the title automatically given to the stash the
> only/primary thing that is of interest to us in this test?  I think
> we care more about that we record the right thing in the resulting
> stash and also after creating the stash the working tree and the
> index becomes clean.  Shouldn't we be testing that?

In this case, the title is really what I wanted to test. There are other tests already to make sure that stash create works, but there were no tests to ensure that a stash was created with the correct title when not on a branch. That being said though, I'll add more validation as more validation is always better.

Show 5 quoted lines

>
> If "git stash create" fails to make the working tree and the index
> clean, then "git checkout master" run by when-finished will carry
> the local modifications with us, which probably is not what you
> meant.  You'd need "reset --hard" there, too, perhaps?

Agreed.

>
>>  test_expect_success 'stash -- <pathspec> stashes and restores the file' '
>>       >foo &&
>>       >bar &&