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

[Junio C Hamano]

Joel Teichroeb  writes:

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

Ah, OK.

Thanks.

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

[Joel Teichroeb]

On Tue, Jun 13, 2017 at 12:45 PM, Junio C Hamano  wrote:
> Joel Teichroeb  writes:
>
>> Signed-off-by: Joel Teichroeb 
>> ---
>>  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.

>
> 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 --  stashes and restores the file' '
>>   >foo &&
>>   >bar &&

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

[Junio C Hamano]

Joel Teichroeb  writes:

> Signed-off-by: Joel Teichroeb 
> ---
>  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?

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?

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

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

[Joel Teichroeb]

Signed-off-by: Joel Teichroeb 
---
 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
+'
+
 test_expect_success 'stash --  stashes and restores the file' '
>foo &&
>bar &&
-- 
2.13.0