Re: [PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name

2018-10-24 Thread Junio C Hamano
Eric Sunshine  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
>>


Re: [PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name

2018-10-24 Thread Eric Sunshine
On Wed, Oct 24, 2018 at 4:06 PM Slavica Djukic
 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 
> ---
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> @@ -1156,4 +1156,17 @@ test_expect_success 'stash --  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
>


[PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name

2018-10-24 Thread Slavica Djukic
From: Slavica 

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@evledraar.gmail.com/T/#u.

Signed-off-by: Slavica Djukic 
---
 t/t3903-stash.sh | 13 +
 1 file changed, 13 insertions(+)

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 --  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
-- 
2.19.1.windows.1



[PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name

2018-10-24 Thread Slavica Djukic
Changes since v1:

*changed test title
*removed subshell and HOME override
*fixed weird identation
*unset() replaced with sane_unset()

Slavica (1):
  [Outreachy] t3903-stash: test without configured user name

 t/t3903-stash.sh | 13 +
 1 file changed, 13 insertions(+)

-- 
2.19.1.windows.1