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

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Jun 2017 12:45:03 -0700

Joel Teichroeb <[email-masked]> writes:

> Signed-off-by: Joel Teichroeb <[email-masked]>
> ---
>  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 -- <pathspec> stashes and restores the file' '
>  	>foo &&
>  	>bar &&