[v2,1/3,Outreachy] t3903-stash: test without configured user name
diff mbox series

Message ID a055296c2034a44f02c253ce3194018b21eb4e1f.1540410925.git.slawica92@hotmail.com
State New
Headers show
Series
  • [v2,1/3,Outreachy] t3903-stash: test without configured user name
Related show

Commit Message

Slavica Djukic Oct. 24, 2018, 8:05 p.m. UTC
From: Slavica <slawica92@hotmail.com>

This is part of enhancement request that ask for 'git stash' to work
even if 'user.name' and 'user.email' are not configured.
Due to an implementation detail, git-stash undesirably requires
'user.name' and 'user.email' to be set, but shouldn't.
The issue is discussed here:
https://public-inbox.org/git/87o9debty4.fsf@evledraar.gmail.com/T/#u.

Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
 t/t3903-stash.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Eric Sunshine Oct. 24, 2018, 8:25 p.m. UTC | #1
On Wed, Oct 24, 2018 at 4:06 PM Slavica Djukic
<slavicadj.ip2018@gmail.com> wrote:
> This is part of enhancement request that ask for 'git stash' to work
> even if 'user.name' and 'user.email' are not configured.
> Due to an implementation detail, git-stash undesirably requires
> 'user.name' and 'user.email' to be set, but shouldn't.

Thanks for re-rolling. This version looks better. One comment below...

> Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
> ---
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> @@ -1156,4 +1156,17 @@ test_expect_success 'stash -- <subdir> works with binary files' '
> +test_expect_failure 'stash works when user.name and user.email are not set' '
> +    test_commit 1 &&
> +    test_config user.useconfigonly true &&
> +    test_config stash.usebuiltin true &&
> +    sane_unset GIT_AUTHOR_NAME &&
> +    sane_unset GIT_AUTHOR_EMAIL &&
> +    sane_unset GIT_COMMITTER_NAME &&
> +    sane_unset GIT_COMMITTER_EMAIL &&
> +    test_must_fail git config user.email &&

Instead of simply asserting that 'user.email' is not set here, you
could instead proactively ensure that it is not set. That is, instead
of the test_must_fail(), do this:

    test_unconfig user.email &&
    test_unconfig user.name &&

> +    echo changed >1.t &&
> +    git stash
> +'
> +
>  test_done
> --
> 2.19.1.windows.1
>
Junio C Hamano Oct. 25, 2018, 4:48 a.m. UTC | #2
Eric Sunshine <sunshine@sunshineco.com> writes:

>> +    test_commit 1 &&
>> +    test_config user.useconfigonly true &&
>> +    test_config stash.usebuiltin true &&
>> +    sane_unset GIT_AUTHOR_NAME &&
>> +    sane_unset GIT_AUTHOR_EMAIL &&
>> +    sane_unset GIT_COMMITTER_NAME &&
>> +    sane_unset GIT_COMMITTER_EMAIL &&
>> +    test_must_fail git config user.email &&
>
> Instead of simply asserting that 'user.email' is not set here, you
> could instead proactively ensure that it is not set. That is, instead
> of the test_must_fail(), do this:
>
>     test_unconfig user.email &&
>     test_unconfig user.name &&

Yes, it would be more in line with what is done to the environment
variables and to other configuration variables in the same block.

Not that I think that this inconsistency is end of the world ;-)

Thanks.

>> +    echo changed >1.t &&
>> +    git stash
>> +'
>> +
>>  test_done
>> --
>> 2.19.1.windows.1
>>

Patch
diff mbox series

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 9e06494ba0..048998d5ce 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1156,4 +1156,17 @@  test_expect_success 'stash -- <subdir> works with binary files' '
 	test_path_is_file subdir/untracked
 '
 
+test_expect_failure 'stash works when user.name and user.email are not set' '
+    test_commit 1 &&
+    test_config user.useconfigonly true &&
+    test_config stash.usebuiltin true &&
+    sane_unset GIT_AUTHOR_NAME &&
+    sane_unset GIT_AUTHOR_EMAIL &&
+    sane_unset GIT_COMMITTER_NAME &&
+    sane_unset GIT_COMMITTER_EMAIL &&
+    test_must_fail git config user.email &&
+    echo changed >1.t &&
+    git stash
+'
+
 test_done