Re: [PATCH v2 1/6] stash: Add tests for passing in too many refs
From: Johannes Schindelin <hidden>
Date: 2018-03-26 19:21:45
Hi Joel, On Sun, 25 Mar 2018, Joel Teichroeb wrote:
Signed-off-by: Joel Teichroeb <redacted>
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.
quoted hunk ↗ jump to hunk
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].*$//')
quoted hunk ↗ jump to hunk
@@ -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