| Message ID | 20211209051115.52629-14-sunshine@sunshineco.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 77b1d9f355b44b68b4bb08a8143d13330fe4c562 |
| Headers | show |
| Series | tests: fix broken &&-chains & abort loops on error | expand |
On Thu, Dec 09, 2021 at 12:11:09AM -0500, Eric Sunshine wrote: > Simplify the way these tests signal failure by employing the modern > idiom of making the `if` or `case` statement resolve to false when an > error is detected. Yeah, these are pretty non-idiomatic, but... > diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh > index cfde68f193..7e46f4ca85 100755 > --- a/t/t3402-rebase-merge.sh > +++ b/t/t3402-rebase-merge.sh > @@ -68,7 +68,7 @@ test_expect_success 'merge and rebase should match' ' > if test -s difference > then > cat difference > - (exit 1) > + false > else > echo happy > fi ...I'd have said the idiom here is just: git diff-tree -r test-rebase test-merge >difference && test -s difference The extra "cat" and "happy" are verbose output that we usually skip in favor of letting "-x" logging do the talking (and leaving the failed state so you can "cat difference" yourself). That said, I'm OK with this minimal change in the name of keeping creep out of the series. -Peff
On Fri, Dec 10, 2021 at 4:32 AM Jeff King <peff@peff.net> wrote: > On Thu, Dec 09, 2021 at 12:11:09AM -0500, Eric Sunshine wrote: > > if test -s difference > > then > > cat difference > > - (exit 1) > > + false > > else > > echo happy > > fi > > ...I'd have said the idiom here is just: > > git diff-tree -r test-rebase test-merge >difference && > test -s difference > > The extra "cat" and "happy" are verbose output that we usually skip in > favor of letting "-x" logging do the talking (and leaving the failed > state so you can "cat difference" yourself). > > That said, I'm OK with this minimal change in the name of keeping creep > out of the series. Indeed, there's plenty of odd cruft like this in old test scripts which could eventually use good cleanups such as the one you suggest here. But I'm also OK with (indeed prefer) this minimal change for the present in order to reduce likelihood of reviewer fatigue in this already lengthy patch series.
diff --git a/t/t2102-update-index-symlinks.sh b/t/t2102-update-index-symlinks.sh index 22f2c730ae..9b11130ab9 100755 --- a/t/t2102-update-index-symlinks.sh +++ b/t/t2102-update-index-symlinks.sh @@ -25,7 +25,7 @@ test_expect_success \ 'the index entry must still be a symbolic link' ' case "$(git ls-files --stage --cached symlink)" in 120000" "*symlink) echo pass;; -*) echo fail; git ls-files --stage --cached symlink; (exit 1);; +*) echo fail; git ls-files --stage --cached symlink; false;; esac' test_done diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh index cfde68f193..7e46f4ca85 100755 --- a/t/t3402-rebase-merge.sh +++ b/t/t3402-rebase-merge.sh @@ -68,7 +68,7 @@ test_expect_success 'merge and rebase should match' ' if test -s difference then cat difference - (exit 1) + false else echo happy fi @@ -102,7 +102,7 @@ test_expect_success 'merge and rebase should match' ' if test -s difference then cat difference - (exit 1) + false else echo happy fi @@ -117,7 +117,7 @@ test_expect_success 'picking rebase' ' echo happy else git show-branch - (exit 1) + false fi && f=$(git diff-tree --name-only HEAD^ HEAD) && g=$(git diff-tree --name-only HEAD^^ HEAD^) && @@ -127,7 +127,7 @@ test_expect_success 'picking rebase' ' *) echo "$f" echo "$g" - (exit 1) + false esac ' diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 23c3c214c5..6902807ff8 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -177,7 +177,7 @@ test_expect_success 'git add --refresh' ' git read-tree HEAD && case "$(git diff-index HEAD -- foo)" in :100644" "*"M foo") echo pass;; - *) echo fail; (exit 1);; + *) echo fail; false;; esac && git add --refresh -- foo && test -z "$(git diff-index HEAD -- foo)"
Simplify the way these tests signal failure by employing the modern idiom of making the `if` or `case` statement resolve to false when an error is detected. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- t/t2102-update-index-symlinks.sh | 2 +- t/t3402-rebase-merge.sh | 8 ++++---- t/t3700-add.sh | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)