Re: [PATCH v2 1/6] stash: Add tests for passing in too many refs

[Johannes Schindelin]

Hi Joel,

On Sun, 25 Mar 2018, Joel Teichroeb wrote:

> Signed-off-by: Joel Teichroeb 

I could imagine that the commit message would benefit from this body:

In preparation for converting the stash command incrementally to
a builtin command, this patch improves test coverage of the option
parsing.

> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index aefde7b172..7146e27bb5 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -45,6 +45,12 @@ test_expect_success 'applying bogus stash does nothing' '
>   test_cmp expect file
>  '
>  
> +test_expect_success 'applying with too many agruments does nothing' '
> + test_must_fail git stash apply stash@{0} bar &&
> + echo 1 >expect &&
> + test_cmp expect file
> +'

I suppose you encountered a problem where `stash apply a b` would modify
the file?

And if you really want to verify that the command does nothing, I guess
you will have to use

test-chmtime =123456789 file &&
test_must_fail git stash apply stash@{0} bar &&
test 123456789 = $(test-chmtime -v +0 file | sed 's/[^0-9].*$//')

> @@ -97,6 +103,10 @@ test_expect_success 'stash drop complains of extra 
> options' '
>   test_must_fail git stash drop --foo
>  '
>  
> +test_expect_success 'stash drop complains with too many refs' '
> + test_must_fail git stash drop stash@{1} stash@{2}

I wonder whether you might want to verify that the error message is
printed, e.g. via

test_must_fail git stash drop stash@{1} stash@{2} 2>err &&
test_i18ngrep "Too many" err

Also, since the added tests look very similar, it might make sense to use
a loop (with fixed revision arguments).

Ciao,
Dscho

[PATCH v2 1/6] stash: Add tests for passing in too many refs

[Joel Teichroeb]

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

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aefde7b172..7146e27bb5 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -45,6 +45,12 @@ test_expect_success 'applying bogus stash does nothing' '
test_cmp expect file
 '
 
+test_expect_success 'applying with too many agruments does nothing' '
+   test_must_fail git stash apply stash@{0} bar &&
+   echo 1 >expect &&
+   test_cmp expect file
+'
+
 test_expect_success 'apply does not need clean working directory' '
echo 4 >other-file &&
git stash apply &&
@@ -97,6 +103,10 @@ test_expect_success 'stash drop complains of extra options' 
'
test_must_fail git stash drop --foo
 '
 
+test_expect_success 'stash drop complains with too many refs' '
+   test_must_fail git stash drop stash@{1} stash@{2}
+'
+
 test_expect_success 'drop top stash' '
git reset --hard &&
git stash list > stashlist1 &&
@@ -160,6 +170,10 @@ test_expect_success 'stash pop' '
test 0 = $(git stash list | wc -l)
 '
 
+test_expect_success 'stash pop complains with too many refs' '
+   test_must_fail git stash pop stash@{1} stash@{2}
+'
+
 cat > expect << EOF
 diff --git a/file2 b/file2
 new file mode 100644
@@ -479,6 +493,10 @@ 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 too many refs' '
+   test_must_fail git stash branch stash-branch stash@{1} stash@{2}
+'
+
 test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
@@ -567,6 +585,10 @@ test_expect_success 'stash show -p - no stashes on stack, 
stash-like argument' '
test_cmp expected actual
 '
 
+test_expect_success 'stash show complains with too many refs' '
+   test_must_fail git stash show stash@{1} stash@{2}
+'
+
 test_expect_success 'stash drop - fail early if specified stash is not a stash 
reference' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
-- 
2.16.2