Re: [PATCH v4 1/5] stash: improve option parsing test coverage

2018-03-29 Thread Junio C Hamano
Joel Teichroeb  writes:

> In preparation for converting the stash command incrementally to
> a builtin command, this patch improves test coverage of the option
> parsing. Both for having too many paramerters, or too few.
>
> Signed-off-by: Joel Teichroeb 
> ---
>  t/t3903-stash.sh | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index aefde7b17..8a666c60c 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -84,6 +84,17 @@ test_expect_success 'apply stashed changes (including 
> index)' '
>   test 1 = $(git show HEAD:file)
>  '
>  
> +test_expect_success 'giving too many ref agruments does nothing' '
> +
> + for type in apply drop pop show "branch stash-branch"
> + do
> + test-chmtime =123456789 file &&
> + test_must_fail git stash $type stash@{0} stash@{1} 2>err &&
> + test_i18ngrep "Too many" err &&
> + test 123456789 = $(test-chmtime -v +0 file | sed 
> 's/[^0-9].*$//') || return 1
> + done
> +'

This is done with "file" whose contents are all different in HEAD,
the index and the working tree.  If the command tries to "push" by
mistake, it will touch the timestamp of "file" and fail the test.
That is a reasonable thing to check.

What do stash@{0} and stash{1} record at this point?  Would they
touch that "file" if the command tries to "apply" or "pop" by
mistake?  Perhaps it deserves a bit of in-code comment here.

As "drop" or "show" would not touch the working tree anyway, the
test for timestamp seems pointless, even though grepping for "Too
many" would be a reasonable test.


Re: [PATCH v4 1/5] stash: improve option parsing test coverage

2018-03-29 Thread Eric Sunshine
On Wed, Mar 28, 2018 at 6:21 PM, Joel Teichroeb  wrote:
> In preparation for converting the stash command incrementally to
> a builtin command, this patch improves test coverage of the option
> parsing. Both for having too many paramerters, or too few.

s/paramerters/parameters/

> Signed-off-by: Joel Teichroeb 


[PATCH v4 1/5] stash: improve option parsing test coverage

2018-03-28 Thread Joel Teichroeb
In preparation for converting the stash command incrementally to
a builtin command, this patch improves test coverage of the option
parsing. Both for having too many paramerters, or too few.

Signed-off-by: Joel Teichroeb 
---
 t/t3903-stash.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aefde7b17..8a666c60c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -84,6 +84,17 @@ test_expect_success 'apply stashed changes (including 
index)' '
test 1 = $(git show HEAD:file)
 '
 
+test_expect_success 'giving too many ref agruments does nothing' '
+
+   for type in apply drop pop show "branch stash-branch"
+   do
+   test-chmtime =123456789 file &&
+   test_must_fail git stash $type stash@{0} stash@{1} 2>err &&
+   test_i18ngrep "Too many" err &&
+   test 123456789 = $(test-chmtime -v +0 file | sed 
's/[^0-9].*$//') || return 1
+   done
+'
+
 test_expect_success 'unstashing in a subdirectory' '
git reset --hard HEAD &&
mkdir subdir &&
@@ -479,6 +490,11 @@ test_expect_success 'stash branch - stashes on stack, 
stash-like argument' '
test $(git ls-files --modified | wc -l) -eq 1
 '
 
+test_expect_success 'stash branch complains with no arguments' '
+   test_must_fail git stash branch 2>err &&
+   test_i18ngrep "No branch name specified" err
+'
+
 test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
-- 
2.16.2