Hi Joel,

On Sun, 25 Mar 2018, Joel Teichroeb wrote:

> Signed-off-by: Joel Teichroeb <j...@teichroeb.net>

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

Reply via email to