* [PATCH 0/25] detecting &&-chain breakage
@ 2015-03-20 10:04 Jeff King
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
` (28 more replies)
0 siblings, 29 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:04 UTC (permalink / raw)
To: git
This is a cleanup of the &&-chain lint I posted earlier:
http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859
I don't know who came up with the idea for it originally, but the
concept certainly was floating in the back of my mind from Jonathan's
earlier version that is referenced in that thread.
The general idea is to detect &&-chain breakage that can lead to our
tests yielding false success. The first patch implements and discusses
the lint-check itself, which is quite simple. The bulk of the work was
fixing all of the issues in the existing tests. :)
That didn't all need to happen immediately. I mainly wanted to start on
it to answer two questions:
1. Are &&-chain breakages actually preventing us from seeing any test
failures? Or is it mostly just pedantry, and we miss out only on
knowing whether "cat >expect <<-\EOF" failed (which presumably it
never does).
2. How bad are the false positives? Both how common, and how bad to
work around.
But after a few hours, I reached a zen state and just kept going. So at
the end of this series, the whole test suite is --chain-lint clean
(modulo any tests that are skipped on my platform). We could even switch
the checks on by default at the end of the series, but I didn't do that
here. I think it would be sane to run them all the time, though; in the
normal success case, they don't add any forks (the shell just runs
"(exit) && ...", and realizes that the whole thing is one big &&-chain).
I couldn't measure any time difference running the suite with and
without it.
Anyway, to answer the questions: Yes, there were definitely tests whose
values were being thrown away, and we would not have noticed if they
failed. The good news is that all of them did pass once we started
checking their results. Hooray.
There were a number of false positives, though as a percentage of the
test suite, probably not many (it's just that we have quite a lot of
tests). Most of them were in rather old tests, and IMHO the fixes I did
actually improved the readability of the result. So overall I think this
is a very positive change; I doubt it will get in people's way very
often, and I look forward to having one less thing to worry about
handling manually in review. The biggest downside is that I may have
automated Eric Sunshine out of a job. :)
The patches are:
[01/25]: t/test-lib: introduce --chain-lint option
[02/25]: t: fix severe &&-chain breakage
[03/25]: t: fix moderate &&-chain breakage
[04/25]: t: fix trivial &&-chain breakage
[05/25]: t: assume test_cmp produces verbose output
[06/25]: t: use verbose instead of hand-rolled errors
[07/25]: t: use test_must_fail instead of hand-rolled blocks
[08/25]: t: fix &&-chaining issues around setup which might fail
[09/25]: t: use test_might_fail for diff and grep
[10/25]: t: use test_expect_code instead of hand-rolled comparison
[11/25]: t: wrap complicated expect_code users in a block
[12/25]: t: avoid using ":" for comments
[13/25]: t3600: fix &&-chain breakage for setup commands
[14/25]: t7201: fix &&-chain breakage
[15/25]: t9502: fix &&-chain breakage
[16/25]: t6030: use modern test_* helpers
[17/25]: t0020: use modern test_* helpers
[18/25]: t1301: use modern test_* helpers
[19/25]: t6034: use modern test_* helpers
[20/25]: t4117: use modern test_* helpers
[21/25]: t9001: use test_when_finished
[22/25]: t0050: appease --chain-lint
[23/25]: t7004: fix embedded single-quotes
[24/25]: t0005: fix broken &&-chains
[25/25]: t4104: drop hand-rolled error reporting
It's a lot of patches, and a few of them are long. I tried to group
them by functionality rather than file (though as you can see, some of
the tests were unique enough snowflakes that it made sense to discuss
their issues separately). I'm hoping it should be an easy review, if
not a short one.
I'll spare you the full per-file diffstat, but the total is a very
satisfying:
84 files changed, 397 insertions(+), 647 deletions(-)
That's a lot of changes in a lot of hunks, but most of them are in files
that haven't been touched in a while. The series merges cleanly with
"pu", so I don't think I've stepped on anyone's topics in flight.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 01/25] t/test-lib: introduce --chain-lint option
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
@ 2015-03-20 10:05 ` Jeff King
2015-03-25 2:53 ` SZEDER Gábor
2015-03-20 10:06 ` [PATCH 02/25] t: fix severe &&-chain breakage Jeff King
` (27 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:05 UTC (permalink / raw)
To: git
It's easy to miss an "&&"-chain in a test script, like:
test_expect_success 'check something important' '
cmd1 &&
cmd2
cmd3
'
The test harness will notice if cmd3 fails, but a failure of
cmd1 or cmd2 will go unnoticed, as their exit status is lost
after cmd3 runs.
The toy example above is easy to spot because the "cmds" are
all the same length, but real code is much more complicated.
It's also difficult to detect these situations by statically
analyzing the shell code with regexps (like the
check-non-portable-shell script does); there's too much
context required to know whether a &&-chain is appropriate
on a given line or not.
This patch instead lets the shell check each test by
sticking a command with a specific and unusual return code
at the top of each test, like:
(exit 117) &&
cmd1 &&
cmd2
cmd3
In a well-formed test, the non-zero exit from the first
command prevents any of the rest from being run, and the
test's exit code is 117. In a bad test (like the one above),
the 117 is lost, and cmd3 is run.
When we encounter a failure of this check, we abort the test
script entirely. For one thing, we have no clue which subset
of the commands in the test snippet were actually run.
Running further tests would be pointless, because we're now
in an unknown state. And two, this is not a "test failure"
in the traditional sense. The test script is buggy, not the
code it is testing. We should be able to fix these problems
in the script once, and not have them come back later as a
regression in git's code.
After checking a test snippet for --chain-lint, we do still
run the test itself. We could actually have a pure-lint
mode which just checks each test, but there are a few
reasons not to. One, because the tests are executing
arbitrary code, which could impact the later environment
(e.g., that could impact which set of tests we run at all).
And two, because a pure-lint mode would still be expensive
to run, because a significant amount of code runs outside of
the test_expect_* blocks. Instead, this option is designed
to be used as part of a normal test suite run, where it adds
very little overhead.
Turning on this option detects quite a few problems in
existing tests, which will be fixed in subsequent patches.
However, there are a number of places it cannot reach:
- it cannot find a failure to break out of loops on error,
like:
cmd1 &&
for i in a b c; do
cmd2 $i
done &&
cmd3
which will not notice failures of "cmd2 a" or "cmd b"
- it cannot find a missing &&-chain inside a block or
subfunction, like:
foo () {
cmd1
cmd2
}
foo &&
bar
which will not notice a failure of cmd1.
- it only checks tests that you run; every platform will
have some tests skipped due to missing prequisites,
so it's impossible to say from one run that the test
suite is free of broken &&-chains. However, all tests get
run by _somebody_, so eventually we will notice problems.
- it does not operate on test_when_finished or prerequisite
blocks. It could, but these tends to be much shorter and
less of a problem, so I punted on them in this patch.
This patch was inspired by an earlier patch by Jonathan
Nieder:
http://article.gmane.org/gmane.comp.version-control.git/235913
This implementation and all bugs are mine.
Signed-off-by: Jeff King <peff@peff.net>
---
t/README | 10 ++++++++++
t/test-lib.sh | 16 ++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/t/README b/t/README
index d5bb0c9..35438bc 100644
--- a/t/README
+++ b/t/README
@@ -168,6 +168,16 @@ appropriately before running "make".
Using this option with a RAM-based filesystem (such as tmpfs)
can massively speed up the test suite.
+--chain-lint::
+--no-chain-lint::
+ If --chain-lint is enabled, the test harness will check each
+ test to make sure that it properly "&&-chains" all commands (so
+ that a failure in the middle does not go unnoticed by the final
+ exit code of the test). This check is performed in addition to
+ running the tests themselves. You may also enable or disable
+ this feature by setting the GIT_TEST_CHAIN_LINT environment
+ variable to "1" or "0", respectively.
+
You can also set the GIT_TEST_INSTALLED environment variable to
the bindir of an existing git installation to test that installation.
You still need to have built this git sandbox, from which various
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c096778..50b3d3f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -232,6 +232,12 @@ do
--root=*)
root=$(expr "z$1" : 'z[^=]*=\(.*\)')
shift ;;
+ --chain-lint)
+ GIT_TEST_CHAIN_LINT=1
+ shift ;;
+ --no-chain-lint)
+ GIT_TEST_CHAIN_LINT=0
+ shift ;;
-x)
trace=t
verbose=t
@@ -524,6 +530,16 @@ test_eval_ () {
test_run_ () {
test_cleanup=:
expecting_failure=$2
+
+ if test "${GIT_TEST_CHAIN_LINT:-0}" != 0; then
+ # 117 is magic because it is unlikely to match the exit
+ # code of other programs
+ test_eval_ "(exit 117) && $1"
+ if test "$?" != 117; then
+ error "bug in the test script: broken &&-chain: $1"
+ fi
+ fi
+
setup_malloc_check
test_eval_ "$1"
eval_ret=$?
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 02/25] t: fix severe &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
@ 2015-03-20 10:06 ` Jeff King
2015-03-20 10:06 ` [PATCH 03/25] t: fix moderate " Jeff King
` (26 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:06 UTC (permalink / raw)
To: git
These are tests which are missing a link in their &&-chain,
in a location which causes a significant portion of the test
to be missed (e.g., the test effectively does nothing, or
consists of a long string of actions and output comparisons,
and we throw away the exit code of at least one part of the
string).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1300-repo-config.sh | 2 +-
t/t1403-show-ref.sh | 10 +++++-----
t/t1700-split-index.sh | 2 +-
t/t3600-rm.sh | 2 +-
t/t4047-diff-dirstat.sh | 2 +-
t/t4104-apply-boundary.sh | 4 ++--
t/t4202-log.sh | 2 +-
t/t5100-mailinfo.sh | 2 +-
t/t5510-fetch.sh | 2 +-
t/t5540-http-push-webdav.sh | 2 +-
t/t5551-http-fetch-smart.sh | 2 +-
t/t6036-recursive-corner-cases.sh | 2 +-
t/t9300-fast-import.sh | 2 +-
t/t9902-completion.sh | 2 +-
14 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 938fc8b..bc0b392 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -677,7 +677,7 @@ test_expect_success 'invalid unit' '
echo 1auto >expect &&
git config aninvalid.unit >actual &&
test_cmp expect actual &&
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
fatal: bad numeric config value '\''1auto'\'' for '\''aninvalid.unit'\'' in .git/config: invalid unit
EOF
test_must_fail git config --int --get aninvalid.unit 2>actual &&
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 3e500ed..7e10bcf 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -28,7 +28,7 @@ test_expect_success 'show-ref' '
>expect &&
- test_must_fail git show-ref D >actual
+ test_must_fail git show-ref D >actual &&
test_cmp expect actual
'
@@ -62,7 +62,7 @@ test_expect_success 'show-ref --verify' '
test_must_fail git show-ref --verify tags/A >actual &&
test_cmp expect actual &&
- test_must_fail git show-ref --verify D >actual
+ test_must_fail git show-ref --verify D >actual &&
test_cmp expect actual
'
@@ -78,7 +78,7 @@ test_expect_success 'show-ref --verify -q' '
test_must_fail git show-ref --verify -q tags/A >actual &&
test_cmp expect actual &&
- test_must_fail git show-ref --verify -q D >actual
+ test_must_fail git show-ref --verify -q D >actual &&
test_cmp expect actual
'
@@ -105,10 +105,10 @@ test_expect_success 'show-ref -d' '
test_cmp expect actual &&
git show-ref -d refs/heads/master >actual &&
- test_cmp expect actual
+ test_cmp expect actual &&
git show-ref -d --verify refs/heads/master >actual &&
- test_cmp expect actual
+ test_cmp expect actual &&
>expect &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 94fb473..a55f5bc 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -182,7 +182,7 @@ test_expect_success 'unify index, two files remain' '
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two
EOF
- test_cmp ls-files.expect ls-files.actual
+ test_cmp ls-files.expect ls-files.actual &&
test-dump-split-index .git/index | sed "/^own/d" >actual &&
cat >expect <<EOF &&
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index e00d7d2..b0db89b 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -170,7 +170,7 @@ test_expect_success 'but with -f it should work.' '
git rm -f foo baz &&
test ! -f foo &&
test ! -f baz &&
- test_must_fail git ls-files --error-unmatch foo
+ test_must_fail git ls-files --error-unmatch foo &&
test_must_fail git ls-files --error-unmatch baz
'
diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh
index 0d50dce..3b8b792 100755
--- a/t/t4047-diff-dirstat.sh
+++ b/t/t4047-diff-dirstat.sh
@@ -374,7 +374,7 @@ test_expect_success 'later options override earlier options:' '
git diff --dirstat=files,10,cumulative,changes,noncumulative,3 -M HEAD^..HEAD >actual_diff_dirstat_M &&
test_cmp expect_diff_dirstat_M actual_diff_dirstat_M &&
git diff --dirstat=files,10,cumulative,changes,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC &&
- test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC
+ test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC &&
git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 HEAD^..HEAD >actual_diff_dirstat &&
test_cmp expect_diff_dirstat actual_diff_dirstat &&
git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 -M HEAD^..HEAD >actual_diff_dirstat_M &&
diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh
index c617c2a..c97aad1 100755
--- a/t/t4104-apply-boundary.sh
+++ b/t/t4104-apply-boundary.sh
@@ -78,8 +78,8 @@ test_expect_success setup '
echo $i
done >victim &&
cat victim >del-z-expect &&
- git diff victim >del-z-patch.with
- git diff --unified=0 >del-z-patch.without &&
+ git diff victim >del-z-patch.with &&
+ git diff --unified=0 >del-z-patch.without
: done
'
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 5f2b290..a22ac7c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -481,7 +481,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --no-decorate >actual &&
test_cmp expect.none actual &&
git log --oneline --decorate >actual &&
- test_cmp expect.short actual
+ test_cmp expect.short actual &&
test_unconfig log.decorate &&
git log --pretty=raw >expect.raw &&
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 60df10f..e97cfb2 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -34,7 +34,7 @@ do
if test -f "$TEST_DIRECTORY"/t5100/msg$mail--no-inbody-headers
then
check_mailinfo $mail --no-inbody-headers
- fi
+ fi &&
if test -f "$TEST_DIRECTORY"/t5100/msg$mail--message-id
then
check_mailinfo $mail --message-id
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d78f320..d3fa2c9 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -124,7 +124,7 @@ test_expect_success 'fetch --prune handles overlapping refspecs' '
git rev-parse origin/master &&
git rev-parse origin/pr/42 &&
- git config --unset-all remote.origin.fetch
+ git config --unset-all remote.origin.fetch &&
git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&
diff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh
index 8d7b3c5..88ff5a4 100755
--- a/t/t5540-http-push-webdav.sh
+++ b/t/t5540-http-push-webdav.sh
@@ -169,7 +169,7 @@ test_expect_failure 'push to password-protected repository (no user in URL)' '
test_commit pw-nouser &&
set_askpass user@host pass@host &&
git push "$HTTPD_URL/auth/dumb/test_repo.git" HEAD &&
- expect_askpass both user@host
+ expect_askpass both user@host &&
git rev-parse --verify HEAD >expect &&
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git" \
rev-parse --verify HEAD >actual &&
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 6cbc12d..eae20d9 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -169,7 +169,7 @@ test_expect_success 'GIT_SMART_HTTP can disable smart http' '
'
test_expect_success 'invalid Content-Type rejected' '
- test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual
+ test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual &&
grep "not valid:" actual
'
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index a86087b..b43d031 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -533,7 +533,7 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '
test $(git rev-parse :3:a) = $(git rev-parse B:a) &&
test $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) &&
- test $(git rev-parse :1:a/file) = $(git rev-parse C:a/file)
+ test $(git rev-parse :1:a/file) = $(git rev-parse C:a/file) &&
test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&
test -f a~D^0
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index c538e0a..6bd5a00 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2228,7 +2228,7 @@ test_expect_success 'R: feature import-marks-if-exists' '
>expect &&
git fast-import --import-marks-if-exists=not_io.marks \
- --export-marks=io.marks <<-\EOF
+ --export-marks=io.marks <<-\EOF &&
feature import-marks-if-exists=io.marks
EOF
test_cmp expect io.marks
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7a883d1..4a14a58 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -568,7 +568,7 @@ test_expect_success 'complete files' '
test_completion "git commit " "modified" &&
: TODO .gitignore should not be here &&
- test_completion "git ls-files " <<-\EOF
+ test_completion "git ls-files " <<-\EOF &&
.gitignore
dir
modified
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 03/25] t: fix moderate &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
2015-03-20 10:06 ` [PATCH 02/25] t: fix severe &&-chain breakage Jeff King
@ 2015-03-20 10:06 ` Jeff King
2015-03-20 10:07 ` [PATCH 04/25] t: fix trivial " Jeff King
` (25 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:06 UTC (permalink / raw)
To: git
These are tests which are missing a link in their &&-chain,
but in a way that probably does not effect the outcome of
the test. Most of these are of the form:
some_cmd >actual
test_cmp expect actual
The main point of the test is to verify the output, and a
failure in some_cmd would probably be noticed by bogus
output. But it is good for the tests to also confirm that
"some_cmd" does not die unexpectedly after producing its
output.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0062-revision-walking.sh | 2 +-
t/t0201-gettext-fallbacks.sh | 4 ++--
t/t1300-repo-config.sh | 2 +-
t/t3001-ls-files-others-exclude.sh | 2 +-
t/t3010-ls-files-killed-modified.sh | 4 ++--
t/t3404-rebase-interactive.sh | 2 +-
t/t4041-diff-submodule-option.sh | 2 +-
t/t4052-stat-output.sh | 34 +++++++++++++++++-----------------
t/t4212-log-corrupt.sh | 2 +-
t/t5551-http-fetch-smart.sh | 2 +-
t/t5709-clone-refspec.sh | 2 +-
t/t6028-merge-up-to-date.sh | 2 +-
t/t6132-pathspec-exclude.sh | 28 ++++++++++++++--------------
13 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/t/t0062-revision-walking.sh b/t/t0062-revision-walking.sh
index 3d98eb8..113c728 100755
--- a/t/t0062-revision-walking.sh
+++ b/t/t0062-revision-walking.sh
@@ -26,7 +26,7 @@ test_expect_success 'setup' '
'
test_expect_success 'revision walking can be done twice' '
- test-revision-walking run-twice > run_twice_actual
+ test-revision-walking run-twice >run_twice_actual &&
test_cmp run_twice_expected run_twice_actual
'
diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh
index 5d80a98..00c6d3d 100755
--- a/t/t0201-gettext-fallbacks.sh
+++ b/t/t0201-gettext-fallbacks.sh
@@ -52,7 +52,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v
cmdline="git am" &&
export cmdline;
printf "When you have resolved this problem, run git am --resolved." >expect &&
- eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual
+ eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual &&
test_i18ncmp expect actual
'
@@ -60,7 +60,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v
cmdline="git am" &&
export cmdline;
printf "When you have resolved this problem, run \"git am --resolved\"." >expect &&
- eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual
+ eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual &&
test_i18ncmp expect actual
'
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index bc0b392..66dd286 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1165,7 +1165,7 @@ test_expect_failure 'adding a key into an empty section reuses header' '
Qkey = value
EOF
- git config section.key value
+ git config section.key value &&
test_cmp expect .git/config
'
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index b2798fe..3fc484e 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -294,7 +294,7 @@ one/a.1
one/two/a.1
three/a.1
EOF
- git ls-files -o -i --exclude "**/a.1" >actual
+ git ls-files -o -i --exclude "**/a.1" >actual &&
test_cmp expect actual
'
diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
index 6d3b828..ea6a678 100755
--- a/t/t3010-ls-files-killed-modified.sh
+++ b/t/t3010-ls-files-killed-modified.sh
@@ -99,12 +99,12 @@ test_expect_success 'git ls-files -k to show killed files.' '
'
test_expect_success 'git ls-files -k output (w/o icase)' '
- git ls-files -k >.output
+ git ls-files -k >.output &&
test_cmp .expected .output
'
test_expect_success 'git ls-files -k output (w/ icase)' '
- git -c core.ignorecase=true ls-files -k >.output
+ git -c core.ignorecase=true ls-files -k >.output &&
test_cmp .expected .output
'
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 034eb35..edad20d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -950,7 +950,7 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
set_fake_editor &&
FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 &&
FAKE_LINES="2 1" git rebase --edit-todo &&
- git rebase --continue
+ git rebase --continue &&
test M = $(git cat-file commit HEAD^ | sed -ne \$p) &&
test L = $(git cat-file commit HEAD | sed -ne \$p)
'
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index e432896..ff61341 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -539,7 +539,7 @@ test_expect_success 'diff --submodule with objects referenced by alternates' '
git checkout origin/master
) &&
git diff --submodule > ../actual
- )
+ ) &&
test_cmp expected actual
'
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index b68afef..4a02b17 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -94,7 +94,7 @@ EOF
while read verb expect cmd args
do
test_expect_success "$cmd $verb COLUMNS (big change)" '
- COLUMNS=200 git $cmd $args >output
+ COLUMNS=200 git $cmd $args >output &&
grep " | " output >actual &&
test_cmp "$expect" actual
'
@@ -102,7 +102,7 @@ do
test "$cmd" != diff || continue
test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
- COLUMNS=200 git $cmd $args --graph >output
+ COLUMNS=200 git $cmd $args --graph >output &&
grep " | " output >actual &&
test_cmp "$expect-graph" actual
'
@@ -122,7 +122,7 @@ EOF
while read verb expect cmd args
do
test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
- COLUMNS=40 git $cmd $args >output
+ COLUMNS=40 git $cmd $args >output &&
grep " | " output >actual &&
test_cmp "$expect" actual
'
@@ -130,7 +130,7 @@ do
test "$cmd" != diff || continue
test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
- COLUMNS=40 git $cmd $args --graph >output
+ COLUMNS=40 git $cmd $args --graph >output &&
grep " | " output >actual &&
test_cmp "$expect-graph" actual
'
@@ -150,7 +150,7 @@ EOF
while read verb expect cmd args
do
test_expect_success "$cmd $verb statGraphWidth config" '
- git -c diff.statGraphWidth=26 $cmd $args >output
+ git -c diff.statGraphWidth=26 $cmd $args >output &&
grep " | " output >actual &&
test_cmp "$expect" actual
'
@@ -158,7 +158,7 @@ do
test "$cmd" != diff || continue
test_expect_success "$cmd --graph $verb statGraphWidth config" '
- git -c diff.statGraphWidth=26 $cmd $args --graph >output
+ git -c diff.statGraphWidth=26 $cmd $args --graph >output &&
grep " | " output >actual &&
test_cmp "$expect-graph" actual
'
@@ -179,19 +179,19 @@ EOF
while read cmd args
do
test_expect_success "$cmd --stat=width with big change" '
- git $cmd $args --stat=40 >output
+ git $cmd $args --stat=40 >output &&
grep " | " output >actual &&
test_cmp expect actual
'
test_expect_success "$cmd --stat-width=width with big change" '
- git $cmd $args --stat-width=40 >output
+ git $cmd $args --stat-width=40 >output &&
grep " | " output >actual &&
test_cmp expect actual
'
test_expect_success "$cmd --stat-graph-width with big change" '
- git $cmd $args --stat-graph-width=26 >output
+ git $cmd $args --stat-graph-width=26 >output &&
grep " | " output >actual &&
test_cmp expect actual
'
@@ -199,13 +199,13 @@ do
test "$cmd" != diff || continue
test_expect_success "$cmd --stat-width=width --graph with big change" '
- git $cmd $args --stat-width=40 --graph >output
+ git $cmd $args --stat-width=40 --graph >output &&
grep " | " output >actual &&
test_cmp expect-graph actual
'
test_expect_success "$cmd --stat-graph-width --graph with big change" '
- git $cmd $args --stat-graph-width=26 --graph >output
+ git $cmd $args --stat-graph-width=26 --graph >output &&
grep " | " output >actual &&
test_cmp expect-graph actual
'
@@ -265,7 +265,7 @@ EOF
while read verb expect cmd args
do
test_expect_success "$cmd $verb COLUMNS (long filename)" '
- COLUMNS=200 git $cmd $args >output
+ COLUMNS=200 git $cmd $args >output &&
grep " | " output >actual &&
test_cmp "$expect" actual
'
@@ -273,7 +273,7 @@ do
test "$cmd" != diff || continue
test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
- COLUMNS=200 git $cmd $args --graph >output
+ COLUMNS=200 git $cmd $args --graph >output &&
grep " | " output >actual &&
test_cmp "$expect-graph" actual
'
@@ -294,7 +294,7 @@ while read verb expect cmd args
do
test_expect_success COLUMNS_CAN_BE_1 \
"$cmd $verb prefix greater than COLUMNS (big change)" '
- COLUMNS=1 git $cmd $args >output
+ COLUMNS=1 git $cmd $args >output &&
grep " | " output >actual &&
test_cmp "$expect" actual
'
@@ -303,7 +303,7 @@ do
test_expect_success COLUMNS_CAN_BE_1 \
"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
- COLUMNS=1 git $cmd $args --graph >output
+ COLUMNS=1 git $cmd $args --graph >output &&
grep " | " output >actual &&
test_cmp "$expect-graph" actual
'
@@ -320,7 +320,7 @@ EOF
test_expect_success 'merge --stat respects COLUMNS (big change)' '
git checkout -b branch HEAD^^ &&
COLUMNS=100 git merge --stat --no-ff master^ >output &&
- grep " | " output >actual
+ grep " | " output >actual &&
test_cmp expect actual
'
@@ -329,7 +329,7 @@ cat >expect <<'EOF'
EOF
test_expect_success 'merge --stat respects COLUMNS (long filename)' '
COLUMNS=100 git merge --stat --no-ff master >output &&
- grep " | " output >actual
+ grep " | " output >actual &&
test_cmp expect actual
'
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index 67bd8ec..22aa8b7 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -61,7 +61,7 @@ test_expect_success 'unparsable dates produce sentinel value' '
test_expect_success 'unparsable dates produce sentinel value (%ad)' '
commit=$(munge_author_date HEAD totally_bogus) &&
echo >expect &&
- git log -1 --format=%ad $commit >actual
+ git log -1 --format=%ad $commit >actual &&
test_cmp expect actual
'
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index eae20d9..ef2feea 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -209,7 +209,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set
git config http.cookiefile cookies.txt &&
git config http.savecookies true &&
git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
- tail -3 cookies.txt > cookies_tail.txt
+ tail -3 cookies.txt >cookies_tail.txt &&
test_cmp expect_cookies.txt cookies_tail.txt
'
diff --git a/t/t5709-clone-refspec.sh b/t/t5709-clone-refspec.sh
index 6f1ea98..7ace253 100755
--- a/t/t5709-clone-refspec.sh
+++ b/t/t5709-clone-refspec.sh
@@ -147,7 +147,7 @@ test_expect_success '--single-branch with detached' '
git for-each-ref refs/remotes/origin |
sed -e "/HEAD$/d" \
-e "s|/remotes/origin/|/heads/|" >../actual
- )
+ ) &&
# nothing
>expect &&
test_cmp expect actual
diff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh
index c518e9c..7763c1b 100755
--- a/t/t6028-merge-up-to-date.sh
+++ b/t/t6028-merge-up-to-date.sh
@@ -83,7 +83,7 @@ test_expect_success 'merge fast-forward octopus' '
git reset --hard c0 &&
test_tick &&
- git merge c1 c2
+ git merge c1 c2 &&
expect=$(git rev-parse c2) &&
current=$(git rev-parse HEAD) &&
test "$expect" = "$current"
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index 62049be..e1e1b1f 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -30,7 +30,7 @@ test_expect_success 'exclude only should error out' '
'
test_expect_success 't_e_i() exclude sub' '
- git log --oneline --format=%s -- . ":(exclude)sub" >actual
+ git log --oneline --format=%s -- . ":(exclude)sub" >actual &&
cat <<EOF >expect &&
sub2/file
file
@@ -39,7 +39,7 @@ EOF
'
test_expect_success 't_e_i() exclude sub/sub/file' '
- git log --oneline --format=%s -- . ":(exclude)sub/sub/file" >actual
+ git log --oneline --format=%s -- . ":(exclude)sub/sub/file" >actual &&
cat <<EOF >expect &&
sub2/file
sub/sub/sub/file
@@ -51,7 +51,7 @@ EOF
'
test_expect_success 't_e_i() exclude sub using mnemonic' '
- git log --oneline --format=%s -- . ":!sub" >actual
+ git log --oneline --format=%s -- . ":!sub" >actual &&
cat <<EOF >expect &&
sub2/file
file
@@ -60,7 +60,7 @@ EOF
'
test_expect_success 't_e_i() exclude :(icase)SUB' '
- git log --oneline --format=%s -- . ":(exclude,icase)SUB" >actual
+ git log --oneline --format=%s -- . ":(exclude,icase)SUB" >actual &&
cat <<EOF >expect &&
sub2/file
file
@@ -71,7 +71,7 @@ EOF
test_expect_success 't_e_i() exclude sub2 from sub' '
(
cd sub &&
- git log --oneline --format=%s -- :/ ":/!sub2" >actual
+ git log --oneline --format=%s -- :/ ":/!sub2" >actual &&
cat <<EOF >expect &&
sub/sub/sub/file
sub/file2
@@ -84,7 +84,7 @@ EOF
'
test_expect_success 't_e_i() exclude sub/*file' '
- git log --oneline --format=%s -- . ":(exclude)sub/*file" >actual
+ git log --oneline --format=%s -- . ":(exclude)sub/*file" >actual &&
cat <<EOF >expect &&
sub2/file
sub/file2
@@ -94,7 +94,7 @@ EOF
'
test_expect_success 't_e_i() exclude :(glob)sub/*/file' '
- git log --oneline --format=%s -- . ":(exclude,glob)sub/*/file" >actual
+ git log --oneline --format=%s -- . ":(exclude,glob)sub/*/file" >actual &&
cat <<EOF >expect &&
sub2/file
sub/sub/sub/file
@@ -106,7 +106,7 @@ EOF
'
test_expect_success 'm_p_d() exclude sub' '
- git ls-files -- . ":(exclude)sub" >actual
+ git ls-files -- . ":(exclude)sub" >actual &&
cat <<EOF >expect &&
file
sub2/file
@@ -115,7 +115,7 @@ EOF
'
test_expect_success 'm_p_d() exclude sub/sub/file' '
- git ls-files -- . ":(exclude)sub/sub/file" >actual
+ git ls-files -- . ":(exclude)sub/sub/file" >actual &&
cat <<EOF >expect &&
file
sub/file
@@ -127,7 +127,7 @@ EOF
'
test_expect_success 'm_p_d() exclude sub using mnemonic' '
- git ls-files -- . ":!sub" >actual
+ git ls-files -- . ":!sub" >actual &&
cat <<EOF >expect &&
file
sub2/file
@@ -136,7 +136,7 @@ EOF
'
test_expect_success 'm_p_d() exclude :(icase)SUB' '
- git ls-files -- . ":(exclude,icase)SUB" >actual
+ git ls-files -- . ":(exclude,icase)SUB" >actual &&
cat <<EOF >expect &&
file
sub2/file
@@ -147,7 +147,7 @@ EOF
test_expect_success 'm_p_d() exclude sub2 from sub' '
(
cd sub &&
- git ls-files -- :/ ":/!sub2" >actual
+ git ls-files -- :/ ":/!sub2" >actual &&
cat <<EOF >expect &&
../file
file
@@ -160,7 +160,7 @@ EOF
'
test_expect_success 'm_p_d() exclude sub/*file' '
- git ls-files -- . ":(exclude)sub/*file" >actual
+ git ls-files -- . ":(exclude)sub/*file" >actual &&
cat <<EOF >expect &&
file
sub/file2
@@ -170,7 +170,7 @@ EOF
'
test_expect_success 'm_p_d() exclude :(glob)sub/*/file' '
- git ls-files -- . ":(exclude,glob)sub/*/file" >actual
+ git ls-files -- . ":(exclude,glob)sub/*/file" >actual &&
cat <<EOF >expect &&
file
sub/file
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 04/25] t: fix trivial &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (2 preceding siblings ...)
2015-03-20 10:06 ` [PATCH 03/25] t: fix moderate " Jeff King
@ 2015-03-20 10:07 ` Jeff King
2015-03-20 10:07 ` [PATCH 05/25] t: assume test_cmp produces verbose output Jeff King
` (24 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:07 UTC (permalink / raw)
To: git
These are tests which are missing a link in their &&-chain,
but during a setup phase. We may fail to notice failure in
commands that build the test environment, but these are
typically not expected to fail at all (but it's still good
to double-check that our test environment is what we
expect).
Signed-off-by: Jeff King <peff@peff.net>
---
t/annotate-tests.sh | 2 +-
t/t0000-basic.sh | 4 ++--
t/t0011-hashmap.sh | 2 +-
t/t0201-gettext-fallbacks.sh | 4 ++--
t/t1006-cat-file.sh | 2 +-
t/t3010-ls-files-killed-modified.sh | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
t/t3405-rebase-malformed.sh | 4 ++--
t/t3425-rebase-topology-merges.sh | 4 ++--
t/t3600-rm.sh | 4 ++--
t/t4014-format-patch.sh | 2 +-
t/t4041-diff-submodule-option.sh | 10 ++++++----
t/t4049-diff-stat-count.sh | 6 +++---
t/t4054-diff-bogus-tree.sh | 2 +-
t/t5500-fetch-pack.sh | 2 +-
t/t5510-fetch.sh | 2 +-
t/t5526-fetch-submodules.sh | 10 +++++-----
t/t5531-deep-submodule-push.sh | 2 +-
t/t5533-push-cas.sh | 4 ++--
t/t5541-http-push-smart.sh | 2 +-
t/t5550-http-fetch-dumb.sh | 2 +-
t/t5551-http-fetch-smart.sh | 2 +-
t/t6000-rev-list-misc.sh | 4 ++--
t/t6006-rev-list-format.sh | 2 +-
t/t6022-merge-rename.sh | 6 +++---
t/t6036-recursive-corner-cases.sh | 2 +-
t/t6111-rev-list-treesame.sh | 2 +-
t/t6200-fmt-merge-msg.sh | 2 +-
t/t7004-tag.sh | 2 +-
| 2 +-
t/t7009-filter-branch-null-sha1.sh | 2 +-
t/t7400-submodule-basic.sh | 4 ++--
t/t7406-submodule-update.sh | 2 +-
t/t7508-status.sh | 8 ++++----
t/t7510-signed-commit.sh | 2 +-
t/t7600-merge.sh | 2 +-
t/t7612-merge-verify-signatures.sh | 2 +-
t/t8003-blame-corner-cases.sh | 8 ++++----
t/t8008-blame-formats.sh | 2 +-
t/t9001-send-email.sh | 2 +-
t/t9300-fast-import.sh | 2 +-
t/t9500-gitweb-standalone-no-errors.sh | 2 +-
t/t9903-bash-prompt.sh | 2 +-
43 files changed, 70 insertions(+), 68 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 071e4d7..f5c0175 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -405,7 +405,7 @@ test_expect_success 'setup -L :regex' '
mv hello.c hello.orig &&
echo "#include <stdio.h>" >hello.c &&
cat hello.orig >>hello.c &&
- tr Q "\\t" >>hello.c <<-\EOF
+ tr Q "\\t" >>hello.c <<-\EOF &&
void mail()
{
Qputs("mail");
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index f10ba4a..79b9074 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -253,7 +253,7 @@ test_expect_success 'test --verbose' '
test_expect_success "failing test" false
test_done
EOF
- mv test-verbose/out test-verbose/out+
+ mv test-verbose/out test-verbose/out+ &&
grep -v "^Initialized empty" test-verbose/out+ >test-verbose/out &&
check_sub_test_lib_test test-verbose <<-\EOF
> expecting success: true
@@ -974,7 +974,7 @@ test_expect_success 'writing this tree with --missing-ok' '
################################################################
test_expect_success 'git read-tree followed by write-tree should be idempotent' '
- rm -f .git/index
+ rm -f .git/index &&
git read-tree $tree &&
test -f .git/index &&
newtree=$(git write-tree) &&
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index f97c805..9c217d9 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -218,7 +218,7 @@ test_expect_success 'grow / shrink' '
echo size >> in &&
echo 64 51 >> expect &&
echo put key52 value52 >> in &&
- echo NULL >> expect
+ echo NULL >> expect &&
echo size >> in &&
echo 256 52 >> expect &&
for n in $(test_seq 12)
diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh
index 00c6d3d..90da1c7 100755
--- a/t/t0201-gettext-fallbacks.sh
+++ b/t/t0201-gettext-fallbacks.sh
@@ -50,7 +50,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v
test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces' '
cmdline="git am" &&
- export cmdline;
+ export cmdline &&
printf "When you have resolved this problem, run git am --resolved." >expect &&
eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual &&
test_i18ncmp expect actual
@@ -58,7 +58,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v
test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces and quotes' '
cmdline="git am" &&
- export cmdline;
+ export cmdline &&
printf "When you have resolved this problem, run \"git am --resolved\"." >expect &&
eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual &&
test_i18ncmp expect actual
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index a72e700..ab36b1e 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -274,7 +274,7 @@ test_expect_success 'setup blobs which are likely to delta' '
'
test_expect_success 'confirm that neither loose blob is a delta' '
- cat >expect <<-EOF
+ cat >expect <<-EOF &&
$_z40
$_z40
EOF
diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
index ea6a678..62fce10 100755
--- a/t/t3010-ls-files-killed-modified.sh
+++ b/t/t3010-ls-files-killed-modified.sh
@@ -62,7 +62,7 @@ test_expect_success 'git update-index --add to add various paths.' '
cd submod$i && git commit --allow-empty -m "empty $i"
) || break
done &&
- git update-index --add submod[12]
+ git update-index --add submod[12] &&
(
cd submod1 &&
git commit --allow-empty -m "empty 1 (updated)"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index edad20d..eed76cc 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1007,7 +1007,7 @@ test_expect_success 'rebase -i with --strategy and -X' '
'
test_expect_success 'rebase -i error on commits with \ in message' '
- current_head=$(git rev-parse HEAD)
+ current_head=$(git rev-parse HEAD) &&
test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" &&
test_commit TO-REMOVE will-conflict old-content &&
test_commit "\temp" will-conflict new-content dummy &&
diff --git a/t/t3405-rebase-malformed.sh b/t/t3405-rebase-malformed.sh
index 19eddad..ff8c360 100755
--- a/t/t3405-rebase-malformed.sh
+++ b/t/t3405-rebase-malformed.sh
@@ -24,7 +24,7 @@ test_expect_success setup '
git add file1 file2 &&
test_tick &&
git commit -m "Initial commit" &&
- git branch diff-in-message
+ git branch diff-in-message &&
git checkout -b multi-line-subject &&
cat F >file2 &&
@@ -36,7 +36,7 @@ test_expect_success setup '
git checkout diff-in-message &&
echo "commit log message containing a diff" >G &&
- echo "" >>G
+ echo "" >>G &&
cat G >file2 &&
git add file2 &&
git diff --cached >>G &&
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
index 1d195fb..846f85c 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -24,7 +24,7 @@ test_expect_success 'setup of non-linear-history' '
test_commit c &&
git checkout b &&
test_commit d &&
- test_commit e
+ test_commit e &&
git checkout c &&
test_commit g &&
@@ -33,7 +33,7 @@ test_expect_success 'setup of non-linear-history' '
cherry_pick gp g &&
test_commit i &&
git checkout b &&
- test_commit f
+ test_commit f &&
git checkout d &&
test_commit n &&
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index b0db89b..1962989 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -183,7 +183,7 @@ test_expect_success 'refuse to remove cached empty file with modifications' '
test_expect_success 'remove intent-to-add file without --force' '
echo content >intent-to-add &&
- git add -N intent-to-add
+ git add -N intent-to-add &&
git rm --cached intent-to-add
'
@@ -201,7 +201,7 @@ test_expect_success 'Recursive without -r fails' '
'
test_expect_success 'Recursive with -r but dirty' '
- echo qfwfq >>frotz/nitfol
+ echo qfwfq >>frotz/nitfol &&
test_must_fail git rm -r frotz &&
test -d frotz &&
test -f frotz/nitfol
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 256affc..c39e500 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -802,7 +802,7 @@ test_expect_success '--no-signature suppresses format.signaturefile ' '
'
test_expect_success '--signature-file overrides format.signaturefile' '
- cat >other-mail-signature <<-\EOF
+ cat >other-mail-signature <<-\EOF &&
Use this other signature instead of mail-signature.
EOF
test_config format.signaturefile mail-signature &&
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index ff61341..2d9731b 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -528,10 +528,12 @@ test_expect_success 'diff --submodule with objects referenced by alternates' '
sha1_before=$(git rev-parse --short HEAD)
echo b >b &&
git add b &&
- git commit -m b
- sha1_after=$(git rev-parse --short HEAD)
- echo "Submodule sub $sha1_before..$sha1_after:
- > b" >../expected
+ git commit -m b &&
+ sha1_after=$(git rev-parse --short HEAD) &&
+ {
+ echo "Submodule sub $sha1_before..$sha1_after:" &&
+ echo " > b"
+ } >../expected
) &&
(cd super &&
(cd sub &&
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 5b594e8..a341217 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -18,7 +18,7 @@ test_expect_success 'mode-only change show as a 0-line change' '
test_chmod +x b d &&
echo a >a &&
echo c >c &&
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
a | 1 +
b | 0
...
@@ -33,7 +33,7 @@ test_expect_success 'binary changes do not count in lines' '
echo a >a &&
echo c >c &&
cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
a | 1 +
c | 1 +
...
@@ -55,7 +55,7 @@ test_expect_success 'exclude unmerged entries from total file count' '
done |
git update-index --index-info &&
echo d >d &&
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
a | 1 +
b | 1 +
...
diff --git a/t/t4054-diff-bogus-tree.sh b/t/t4054-diff-bogus-tree.sh
index 0843c87..1d6efab 100755
--- a/t/t4054-diff-bogus-tree.sh
+++ b/t/t4054-diff-bogus-tree.sh
@@ -16,7 +16,7 @@ test_expect_success 'create bogus tree' '
test_expect_success 'create tree with matching file' '
echo bar >foo &&
git add foo &&
- good_tree=$(git write-tree)
+ good_tree=$(git write-tree) &&
blob=$(git rev-parse :foo)
'
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index bd37f04..692d717 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -414,7 +414,7 @@ test_expect_success 'setup tests for the --stdin parameter' '
do
git tag $head $head
done &&
- cat >input <<-\EOF
+ cat >input <<-\EOF &&
refs/heads/C
refs/heads/A
refs/heads/D
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d3fa2c9..0ba9db0 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -596,7 +596,7 @@ test_configured_prune () {
test_unconfig remote.origin.prune &&
git fetch &&
git rev-parse --verify refs/remotes/origin/newbranch
- )
+ ) &&
# now remove it
git branch -d newbranch &&
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index ca5b027..a4532b0 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -216,7 +216,7 @@ test_expect_success "Recursion stops when no new submodule commits are fetched"
head2=$(git rev-parse --short HEAD) &&
echo "Fetching submodule submodule" > expect.out.sub &&
echo "From $pwd/." > expect.err.sub &&
- echo " $head1..$head2 master -> origin/master" >> expect.err.sub
+ echo " $head1..$head2 master -> origin/master" >>expect.err.sub &&
head -2 expect.err >> expect.err.sub &&
(
cd downstream &&
@@ -315,7 +315,7 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne
) &&
head1=$(git rev-parse --short HEAD^) &&
git add subdir/deepsubmodule &&
- git commit -m "new deepsubmodule"
+ git commit -m "new deepsubmodule" &&
head2=$(git rev-parse --short HEAD) &&
echo "From $pwd/submodule" > ../expect.err.sub &&
echo " $head1..$head2 master -> origin/master" >> ../expect.err.sub
@@ -337,7 +337,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess
head2=$(git rev-parse --short HEAD) &&
tail -2 expect.err > expect.err.deepsub &&
echo "From $pwd/." > expect.err &&
- echo " $head1..$head2 master -> origin/master" >> expect.err
+ echo " $head1..$head2 master -> origin/master" >>expect.err &&
cat expect.err.sub >> expect.err &&
cat expect.err.deepsub >> expect.err &&
(
@@ -387,7 +387,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config
git commit -m "new submodule" &&
head2=$(git rev-parse --short HEAD) &&
echo "From $pwd/." > expect.err.2 &&
- echo " $head1..$head2 master -> origin/master" >> expect.err.2
+ echo " $head1..$head2 master -> origin/master" >>expect.err.2 &&
head -2 expect.err >> expect.err.2 &&
(
cd downstream &&
@@ -415,7 +415,7 @@ test_expect_success "'submodule.<sub>.fetchRecurseSubmodules=on-demand' override
git commit -m "new submodule" &&
head2=$(git rev-parse --short HEAD) &&
echo "From $pwd/." > expect.err.2 &&
- echo " $head1..$head2 master -> origin/master" >> expect.err.2
+ echo " $head1..$head2 master -> origin/master" >>expect.err.2 &&
head -2 expect.err >> expect.err.2 &&
(
cd downstream &&
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 445bb5f..6507487 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -104,7 +104,7 @@ test_expect_success 'push fails when commit on multiple branches if one branch h
'
test_expect_success 'push succeeds if submodule has no remote and is on the first superproject commit' '
- git init --bare a
+ git init --bare a &&
git clone a a1 &&
(
cd a1 &&
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index ba20d83..dccf8a6 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -159,7 +159,7 @@ test_expect_success 'cover everything with default force-with-lease (protected)'
(
cd src &&
git branch naster master^
- )
+ ) &&
git ls-remote src refs/heads/\* >expect &&
(
cd dst &&
@@ -174,7 +174,7 @@ test_expect_success 'cover everything with default force-with-lease (allowed)' '
(
cd src &&
git branch naster master^
- )
+ ) &&
(
cd dst &&
git fetch &&
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index d2c681e..47cee53 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -158,7 +158,7 @@ test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '
# create a dissimilarly-named remote ref so that git is unable to match the
# two refs (viz. local, remote) unless an explicit refspec is provided.
- git push origin master:retsam
+ git push origin master:retsam &&
echo "change changed" > path2 &&
git commit -a -m path2 --amend &&
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 2731ad4..61e087e 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup repository' '
git config push.default matching &&
echo content1 >file &&
git add file &&
- git commit -m one
+ git commit -m one &&
echo content2 >file &&
git add file &&
git commit -m two
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index ef2feea..cf0a6ea 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -83,7 +83,7 @@ test_expect_success 'clone http repository' '
test_expect_success 'fetch changes via http' '
echo content >>file &&
git commit -a -m two &&
- git push public
+ git push public &&
(cd clone && git pull) &&
test_cmp file clone/file
'
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 2602086..7911ed9 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -42,7 +42,7 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '
test_tick &&
git commit -m that &&
- ONE=$(git rev-parse HEAD:one)
+ ONE=$(git rev-parse HEAD:one) &&
git rev-list --objects HEAD two >output &&
grep "$ONE two/three" output &&
! grep one output
@@ -85,7 +85,7 @@ test_expect_success 'rev-list can show index objects' '
# - we do not show the root tree; since we updated the index, it
# does not have a valid cache tree
#
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
8e4020bb5a8d8c873b25de15933e75cc0fc275df one
d9d3a7417b9605cfd88ee6306b28dadc29e6ab08 only-in-index
9200b628cf9dc883a85a7abc8d6e6730baee589c two
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index a02a45a..2b7c0f0 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -45,7 +45,7 @@ test_expect_success 'setup' '
head2=$(git rev-parse --verify HEAD) &&
head2_short=$(git rev-parse --verify --short $head2) &&
tree2=$(git rev-parse --verify HEAD:) &&
- tree2_short=$(git rev-parse --verify --short $tree2)
+ tree2_short=$(git rev-parse --verify --short $tree2) &&
git config --unset i18n.commitEncoding
'
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index a89dfbe..05ebba7 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -685,7 +685,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
git add -A &&
git commit -mA &&
- git checkout -b side
+ git checkout -b side &&
git rm -rf df &&
git commit -mB &&
@@ -716,7 +716,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '
git add -A &&
git commit -mA &&
- git checkout -b side
+ git checkout -b side &&
git rm -f file &&
git commit -m "Delete file" &&
@@ -745,7 +745,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
git add -A &&
git commit -mA &&
- git checkout -b side
+ git checkout -b side &&
cp file newfile &&
git add -A &&
git commit -m "Add file copy" &&
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index b43d031..8923b04 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -86,7 +86,7 @@ test_expect_success 'setup criss-cross + rename merges with basic modification'
rm -rf .git &&
git init &&
- ten="0 1 2 3 4 5 6 7 8 9"
+ ten="0 1 2 3 4 5 6 7 8 9" &&
for i in $ten
do
echo line $i in a sample file
diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
index 88b84df..45e3673 100755
--- a/t/t6111-rev-list-treesame.sh
+++ b/t/t6111-rev-list-treesame.sh
@@ -58,7 +58,7 @@ test_expect_success setup '
git checkout master &&
test_tick && git merge --no-ff fiddler-branch &&
- note K
+ note K &&
test_commit "file=Part 1" file "Part 1" L &&
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 54b5744..2e2fb0e 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -194,7 +194,7 @@ test_expect_success '--log=5 with custom comment character' '
'
test_expect_success 'merge.log=0 disables shortlog' '
- echo "Merge branch ${apos}left${apos}" >expected
+ echo "Merge branch ${apos}left${apos}" >expected &&
git -c merge.log=0 fmt-merge-msg <.git/FETCH_HEAD >actual &&
test_cmp expected actual
'
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 35c805a..347d3be 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1180,7 +1180,7 @@ test_expect_success 'message in editor has initial comment: first line' '
test_expect_success \
'message in editor has initial comment: remainder' '
# remove commented lines from the remainder -- should be empty
- >rest.expect
+ >rest.expect &&
sed -e 1d -e '/^#/d' <actual >rest.actual &&
test_cmp rest.expect rest.actual
'
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index da958a8..947b690 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -396,7 +396,7 @@ test_expect_success TTY 'command-specific pager overrides core.pager' '
sane_unset PAGER GIT_PAGER &&
echo "foo:initial" >expect &&
>actual &&
- test_config core.pager "exit 1"
+ test_config core.pager "exit 1" &&
test_config pager.log "sed s/^/foo:/ >actual" &&
test_terminal git log --format=%s -1 &&
test_cmp expect actual
diff --git a/t/t7009-filter-branch-null-sha1.sh b/t/t7009-filter-branch-null-sha1.sh
index a997f7a..c27f90f 100755
--- a/t/t7009-filter-branch-null-sha1.sh
+++ b/t/t7009-filter-branch-null-sha1.sh
@@ -13,7 +13,7 @@ test_expect_success 'setup: a commit with a bogus null sha1 in the tree' '
{
git ls-tree HEAD &&
printf "160000 commit $_z40\\tbroken\\n"
- } >broken-tree
+ } >broken-tree &&
echo "add broken entry" >msg &&
tree=$(git mktree <broken-tree) &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5811a98..540771c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -766,7 +766,7 @@ test_expect_success 'moving the superproject does not break submodules' '
(
cd addtest &&
git submodule status >expect
- )
+ ) &&
mv addtest addtest2 &&
(
cd addtest2 &&
@@ -987,7 +987,7 @@ test_expect_success 'submodule with UTF-8 name' '
test_expect_success 'submodule add clone shallow submodule' '
mkdir super &&
- pwd=$(pwd)
+ pwd=$(pwd) &&
(
cd super &&
git init &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 29d3d2c..dda3929 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -754,7 +754,7 @@ test_expect_success SYMLINKS 'submodule update can handle symbolic links in pwd'
test_expect_success 'submodule update clone shallow submodule' '
git clone cloned super3 &&
- pwd=$(pwd)
+ pwd=$(pwd) &&
(cd super3 &&
sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
mv -f .gitmodules.tmp .gitmodules &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 6b16bcb..0f9ad4c 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -431,7 +431,7 @@ test_expect_success 'status -s -uno' '
'
test_expect_success 'status -s (status.showUntrackedFiles no)' '
- git config status.showuntrackedfiles no
+ git config status.showuntrackedfiles no &&
git status -s >output &&
test_cmp expect output
'
@@ -465,7 +465,7 @@ EOF
'
test_expect_success 'status (status.showUntrackedFiles normal)' '
- test_config status.showuntrackedfiles normal
+ test_config status.showuntrackedfiles normal &&
git status >output &&
test_i18ncmp expect output
'
@@ -485,7 +485,7 @@ test_expect_success 'status -s -unormal' '
'
test_expect_success 'status -s (status.showUntrackedFiles normal)' '
- git config status.showuntrackedfiles normal
+ git config status.showuntrackedfiles normal &&
git status -s >output &&
test_cmp expect output
'
@@ -520,7 +520,7 @@ EOF
'
test_expect_success 'status (status.showUntrackedFiles all)' '
- test_config status.showuntrackedfiles all
+ test_config status.showuntrackedfiles all &&
git status >output &&
test_i18ncmp expect output
'
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 474dab3..fc1ff45 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -42,7 +42,7 @@ test_expect_success GPG 'create signed commits' '
git tag seventh-unsigned &&
test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&
- git tag seventh-signed
+ git tag seventh-signed &&
echo 8 >file && test_tick && git commit -a -m eighth -SB7227189 &&
git tag eighth-signed-alt
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index b164621..75c50ee 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -133,7 +133,7 @@ test_expect_success 'setup' '
test_tick &&
git commit -m "commit 3" &&
git tag c3 &&
- c3=$(git rev-parse HEAD)
+ c3=$(git rev-parse HEAD) &&
git reset --hard "$c0" &&
create_merge_msgs
'
diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh
index 21a0bf8..8ae69a6 100755
--- a/t/t7612-merge-verify-signatures.sh
+++ b/t/t7612-merge-verify-signatures.sh
@@ -29,7 +29,7 @@ test_expect_success GPG 'create signed commits' '
git checkout -b side-untrusted &&
echo 3 >baz && git add baz &&
- test_tick && git commit -SB7227189 -m "untrusted on side"
+ test_tick && git commit -SB7227189 -m "untrusted on side" &&
git checkout master
'
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 2a3469b..32895e5 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -26,7 +26,7 @@ test_expect_success setup '
cat one >uno &&
mv two dos &&
cat one >>tres &&
- echo DEF >>mouse
+ echo DEF >>mouse &&
git add uno dos tres mouse &&
test_tick &&
GIT_AUTHOR_NAME=Second git commit -a -m Second &&
@@ -153,15 +153,15 @@ test_expect_success 'blame path that used to be a directory' '
'
test_expect_success 'blame to a commit with no author name' '
- TREE=`git rev-parse HEAD:`
- cat >badcommit <<EOF
+ TREE=`git rev-parse HEAD:` &&
+ cat >badcommit <<EOF &&
tree $TREE
author <noname> 1234567890 +0000
committer David Reiss <dreiss@facebook.com> 1234567890 +0000
some message
EOF
- COMMIT=`git hash-object -t commit -w badcommit`
+ COMMIT=`git hash-object -t commit -w badcommit` &&
git --no-pager blame $COMMIT -- uno >/dev/null
'
diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
index d15f8b3..29f84a6 100755
--- a/t/t8008-blame-formats.sh
+++ b/t/t8008-blame-formats.sh
@@ -5,7 +5,7 @@ test_description='blame output in various formats on a simple case'
test_expect_success 'setup' '
echo a >file &&
- git add file
+ git add file &&
test_tick &&
git commit -m one &&
echo b >>file &&
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 0513055..37caa18 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1325,7 +1325,7 @@ test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data'
test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' '
clean_fake_sendmail &&
- git config sendemail.transferEncoding 8bit
+ git config sendemail.transferEncoding 8bit &&
test_must_fail git send-email \
--transfer-encoding=7bit \
--smtp-server="$(pwd)/fake.sendmail" \
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 6bd5a00..584b538 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1132,7 +1132,7 @@ test_expect_success \
compare_diff_raw expect actual'
test_expect_success PIPE 'N: read and copy directory' '
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf file3/newf
:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100 file2/oldf file3/oldf
EOF
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index e74b9ab..f9f078e 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -297,7 +297,7 @@ test_expect_success 'setup incomplete lines' '
echo "Dominus regit me," >file &&
echo "incomplete line" | tr -d "\\012" >>file &&
git commit -a -m "Change incomplete line" &&
- git tag incomplete_lines_chg
+ git tag incomplete_lines_chg &&
echo "Dominus regit me," >file &&
git commit -a -m "Remove incomplete line" &&
git tag incomplete_lines_rem
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 51ecd3e..0c6acdd 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -166,7 +166,7 @@ test_expect_success 'prompt - inside bare repository' '
'
test_expect_success 'prompt - interactive rebase' '
- printf " (b1|REBASE-i 2/3)" >expected
+ printf " (b1|REBASE-i 2/3)" >expected &&
write_script fake_editor.sh <<-\EOF &&
echo "exec echo" >"$1"
echo "edit $(git log -1 --format="%h")" >>"$1"
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 05/25] t: assume test_cmp produces verbose output
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (3 preceding siblings ...)
2015-03-20 10:07 ` [PATCH 04/25] t: fix trivial " Jeff King
@ 2015-03-20 10:07 ` Jeff King
2015-03-20 10:09 ` [PATCH 06/25] t: use verbose instead of hand-rolled errors Jeff King
` (23 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:07 UTC (permalink / raw)
To: git
Some tests call test_cmp, and if it fails show the actual
output generated. This is mostly pointless, as test_cmp will
already show a diff between the expected and actual output.
It also fools --chain-lint by putting an "||" in the middle
of the chain, so we'd rather not use this construct.
Note that these cases actually show a pre-processed version
of the data, rather than exactly what test_cmp would show.
However, test_cmp's output is generally good for pointing
the user in the right direction, and they can then dig in
the trash directory themselves if they want to see more
details.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t6012-rev-list-simplify.sh | 10 ++--------
t/t6111-rev-list-treesame.sh | 5 +----
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index fde5e71..b89cd6b 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -95,10 +95,7 @@ check_outcome () {
git log --pretty="$FMT" --parents $param |
unnote >actual &&
sed -e "s/^.* \([^ ]*\) .*/\1/" >check <actual &&
- test_cmp expect check || {
- cat actual
- false
- }
+ test_cmp expect check
'
}
@@ -121,10 +118,7 @@ test_expect_success 'full history simplification without parent' '
git log --pretty="$FMT" --full-history E -- lost |
unnote >actual &&
sed -e "s/^.* \([^ ]*\) .*/\1/" >check <actual &&
- test_cmp expect check || {
- cat actual
- false
- }
+ test_cmp expect check
'
test_expect_success '--full-diff is not affected by --parents' '
diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
index 45e3673..32474c2 100755
--- a/t/t6111-rev-list-treesame.sh
+++ b/t/t6111-rev-list-treesame.sh
@@ -92,10 +92,7 @@ check_outcome () {
git log --format="$FMT" $param |
unnote >actual &&
sed -e "$munge_actual" <actual >check &&
- test_cmp expect check || {
- cat actual
- false
- }
+ test_cmp expect check
'
}
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 06/25] t: use verbose instead of hand-rolled errors
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (4 preceding siblings ...)
2015-03-20 10:07 ` [PATCH 05/25] t: assume test_cmp produces verbose output Jeff King
@ 2015-03-20 10:09 ` Jeff King
2015-03-20 10:09 ` [PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks Jeff King
` (22 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:09 UTC (permalink / raw)
To: git
Many tests that predate the "verbose" helper function use a
pattern like:
test ... || {
echo ...
false
}
to give more verbose output. Using the helper, we can do
this with a single line, and avoid a || which interacts
badly with &&-chaining (besides fooling --chain-lint, we hit
the error block no matter which command in the chain failed,
so we may often show useless results).
In most cases, the messages printed by "verbose" are equally
good (in some cases better; t6006 accidentally redirects the
message to a file!). The exception is t7001, whose output
suffers slightly. However, it's still enough to show the
user which part failed, given that we will have just printed
the test script to stderr.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4022-diff-rewrite.sh | 5 +----
t/t4202-log.sh | 30 +++++-------------------------
t/t6006-rev-list-format.sh | 5 +----
t/t7001-mv.sh | 5 +----
t/t7300-clean.sh | 10 ++--------
5 files changed, 10 insertions(+), 45 deletions(-)
diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh
index 2d030a4..cb51d9f 100755
--- a/t/t4022-diff-rewrite.sh
+++ b/t/t4022-diff-rewrite.sh
@@ -20,10 +20,7 @@ test_expect_success setup '
test_expect_success 'detect rewrite' '
actual=$(git diff-files -B --summary test) &&
- expr "$actual" : " rewrite test ([0-9]*%)$" || {
- echo "Eh? <<$actual>>"
- false
- }
+ verbose expr "$actual" : " rewrite test ([0-9]*%)$"
'
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index a22ac7c..85230a8 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -113,11 +113,7 @@ test_expect_success 'diff-filter=M' '
actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
expect=$(echo second) &&
- test "$actual" = "$expect" || {
- echo Oops
- echo "Actual: $actual"
- false
- }
+ verbose test "$actual" = "$expect"
'
@@ -125,11 +121,7 @@ test_expect_success 'diff-filter=D' '
actual=$(git log --pretty="format:%s" --diff-filter=D HEAD) &&
expect=$(echo sixth ; echo third) &&
- test "$actual" = "$expect" || {
- echo Oops
- echo "Actual: $actual"
- false
- }
+ verbose test "$actual" = "$expect"
'
@@ -137,11 +129,7 @@ test_expect_success 'diff-filter=R' '
actual=$(git log -M --pretty="format:%s" --diff-filter=R HEAD) &&
expect=$(echo third) &&
- test "$actual" = "$expect" || {
- echo Oops
- echo "Actual: $actual"
- false
- }
+ verbose test "$actual" = "$expect"
'
@@ -149,11 +137,7 @@ test_expect_success 'diff-filter=C' '
actual=$(git log -C -C --pretty="format:%s" --diff-filter=C HEAD) &&
expect=$(echo fourth) &&
- test "$actual" = "$expect" || {
- echo Oops
- echo "Actual: $actual"
- false
- }
+ verbose test "$actual" = "$expect"
'
@@ -161,11 +145,7 @@ test_expect_success 'git log --follow' '
actual=$(git log --follow --pretty="format:%s" ichi) &&
expect=$(echo third ; echo second ; echo initial) &&
- test "$actual" = "$expect" || {
- echo Oops
- echo "Actual: $actual"
- false
- }
+ verbose test "$actual" = "$expect"
'
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 2b7c0f0..b77d4c9 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -358,10 +358,7 @@ test_expect_success 'empty email' '
test_tick &&
C=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} </dev/null) &&
A=$(git show --pretty=format:%an,%ae,%ad%n -s $C) &&
- test "$A" = "A U Thor,,Thu Apr 7 15:14:13 2005 -0700" || {
- echo "Eh? $A" >failure
- false
- }
+ verbose test "$A" = "A U Thor,,Thu Apr 7 15:14:13 2005 -0700"
'
test_expect_success 'del LF before empty (1)' '
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 69f11bd..7b56081 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -161,10 +161,7 @@ test_expect_success "Michael Cassar's test case" '
git mv papers/unsorted/Thesis.pdf papers/all-papers/moo-blah.pdf &&
T=`git write-tree` &&
- git ls-tree -r $T | grep partA/outline.txt || {
- git ls-tree -r $T
- (exit 1)
- }
+ git ls-tree -r $T | verbose grep partA/outline.txt
'
rm -fr papers partA path?
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 04118ad..99be5d9 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -119,10 +119,7 @@ test_expect_success C_LOCALE_OUTPUT 'git clean with relative prefix' '
git clean -n ../src |
sed -n -e "s|^Would remove ||p"
) &&
- test "$would_clean" = ../src/part3.c || {
- echo "OOps <$would_clean>"
- false
- }
+ verbose test "$would_clean" = ../src/part3.c
'
test_expect_success C_LOCALE_OUTPUT 'git clean with absolute path' '
@@ -134,10 +131,7 @@ test_expect_success C_LOCALE_OUTPUT 'git clean with absolute path' '
git clean -n "$(pwd)/../src" |
sed -n -e "s|^Would remove ||p"
) &&
- test "$would_clean" = ../src/part3.c || {
- echo "OOps <$would_clean>"
- false
- }
+ verbose test "$would_clean" = ../src/part3.c
'
test_expect_success 'git clean with out of work tree relative path' '
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (5 preceding siblings ...)
2015-03-20 10:09 ` [PATCH 06/25] t: use verbose instead of hand-rolled errors Jeff King
@ 2015-03-20 10:09 ` Jeff King
2015-03-20 10:10 ` [PATCH 08/25] t: fix &&-chaining issues around setup which might fail Jeff King
` (21 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:09 UTC (permalink / raw)
To: git
These test scripts likely predate test_must_fail, and can be
made simpler by using it (in addition to making them pass
--chain-lint).
The case in t6036 loses some verbosity in the failure case,
but it is so tied to a specific failure mode that it is not
worth keeping around (and the outcome of the test is not
affected at all).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4124-apply-ws-rule.sh | 5 ++---
t/t6036-recursive-corner-cases.sh | 7 +------
t/t9300-fast-import.sh | 7 ++-----
3 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index c6474de..d350065 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -99,9 +99,8 @@ test_expect_success 'whitespace=warn, default rule' '
test_expect_success 'whitespace=error-all, default rule' '
- apply_patch --whitespace=error-all && return 1
- test -s target && return 1
- : happy
+ test_must_fail apply_patch --whitespace=error-all &&
+ ! test -s target
'
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 8923b04..9d6621c 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -195,12 +195,7 @@ test_expect_success 'git detects differently handled merges conflict' '
git reset --hard &&
git checkout D^0 &&
- git merge -s recursive E^0 && {
- echo "BAD: should have conflicted"
- test "Incorrectly merged content" = "$(cat new_a)" &&
- echo "BAD: Silently accepted wrong content"
- return 1
- }
+ test_must_fail git merge -s recursive E^0 &&
test 3 = $(git ls-files -s | wc -l) &&
test 3 = $(git ls-files -u | wc -l) &&
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 584b538..aac126f 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2853,8 +2853,8 @@ test_expect_success 'S: notemodify with garbage after mark commit-ish must fail'
# from
#
test_expect_success 'S: from with garbage after mark must fail' '
- # no &&
- git fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err
+ test_must_fail \
+ git fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err &&
commit refs/heads/S2
mark :303
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
@@ -2865,9 +2865,6 @@ test_expect_success 'S: from with garbage after mark must fail' '
M 100644 :403 hello.c
EOF
- ret=$? &&
- echo returned $ret &&
- test $ret -ne 0 && # failed, but it created the commit
# go create the commit, need it for merge test
git fast-import --import-marks=marks --export-marks=marks <<-EOF &&
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 08/25] t: fix &&-chaining issues around setup which might fail
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (6 preceding siblings ...)
2015-03-20 10:09 ` [PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks Jeff King
@ 2015-03-20 10:10 ` Jeff King
2015-03-20 10:11 ` [PATCH 09/25] t: use test_might_fail for diff and grep Jeff King
` (20 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:10 UTC (permalink / raw)
To: git
Many tests have an initial setup step that might fail based
on whether earlier tests in the script have succeeded or
not. Using a trick like "|| true" breaks the &&-chain,
missing earlier failures (and fooling --chain-lint).
We can use test_might_fail in some cases, which is correct
and makes the intent more obvious. We can also use
test_unconfig for unsetting config (and which is more
robust, as well).
The case in t9500 is an oddball. It wants to run cmd1 _or_
cmd2, and does it like:
cmd1 || cmd2 &&
other_stuff
It's not wrong in this case, but it's a bad habit to get
into, because it breaks the &&-chain if used anywhere except
at the beginning of the test (and we use the correct
solution here, putting it inside a block for precedence).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t5503-tagfollow.sh | 4 ++--
t/t6032-merge-large-rename.sh | 6 +++---
t/t7201-co.sh | 2 +-
t/t7508-status.sh | 2 +-
t/t9500-gitweb-standalone-no-errors.sh | 5 ++++-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
index f30c038..4ca48f0 100755
--- a/t/t5503-tagfollow.sh
+++ b/t/t5503-tagfollow.sh
@@ -139,8 +139,8 @@ EOF
'
test_expect_success 'new clone fetch master and tags' '
- git branch -D cat
- rm -f $U
+ test_might_fail git branch -D cat &&
+ rm -f $U &&
(
mkdir clone2 &&
cd clone2 &&
diff --git a/t/t6032-merge-large-rename.sh b/t/t6032-merge-large-rename.sh
index 15beecc..0f79268 100755
--- a/t/t6032-merge-large-rename.sh
+++ b/t/t6032-merge-large-rename.sh
@@ -28,10 +28,10 @@ make_text() {
test_rename() {
test_expect_success "rename ($1, $2)" '
- n='$1'
- expect='$2'
+ n='$1' &&
+ expect='$2' &&
git checkout -f master &&
- git branch -D test$n || true &&
+ test_might_fail git branch -D test$n &&
git reset --hard initial &&
for i in $(count $n); do
make_text $i initial initial >$i
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index eae9e5a..a7fe4e6 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -591,7 +591,7 @@ test_expect_success 'checkout --conflict=merge, overriding config' '
'
test_expect_success 'checkout --conflict=diff3' '
- git config --unset merge.conflictstyle
+ test_unconfig merge.conflictstyle &&
setup_conflicting_index &&
echo "none of the above" >sample &&
echo ourside >expect &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 0f9ad4c..c3ed7cb 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -538,7 +538,7 @@ A dir2/added
?? untracked
EOF
test_expect_success 'status -s -uall' '
- git config --unset status.showuntrackedfiles
+ test_unconfig status.showuntrackedfiles &&
git status -s -uall >output &&
test_cmp expect output
'
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index f9f078e..e94b2f1 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -779,7 +779,10 @@ test_expect_success \
test_expect_success \
'unborn HEAD: "summary" page (with "heads" subview)' \
- 'git checkout orphan_branch || git checkout --orphan orphan_branch &&
+ '{
+ git checkout orphan_branch ||
+ git checkout --orphan orphan_branch
+ } &&
test_when_finished "git checkout master" &&
gitweb_run "p=.git;a=summary"'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 09/25] t: use test_might_fail for diff and grep
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (7 preceding siblings ...)
2015-03-20 10:10 ` [PATCH 08/25] t: fix &&-chaining issues around setup which might fail Jeff King
@ 2015-03-20 10:11 ` Jeff King
2015-03-20 10:11 ` [PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison Jeff King
` (19 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:11 UTC (permalink / raw)
To: git
Some tests run diff or grep to produce an output, and then
compare the output to an expected value. We know the exit
code we expect these processes to have (e.g., grep yields 0
if it produced output and 1 otherwise), so it would not make
the test wrong to look for it. But the difference between
their output and the expected output (e.g., shown by
test_cmp) is much more useful to somebody debugging the test
than the test just bailing out.
These tests break the &&-chain to skip the exit-code check
of the process. However, we can get the same effect by using
test_might_fail. Note that in some cases the test did use
"|| return 1", which meant the test was not wrong, but it
did fool --chain-lint.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1002-read-tree-m-u-2way.sh | 8 ++++----
t/t5550-http-fetch-dumb.sh | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh
index fed877b..e3bf821 100755
--- a/t/t1002-read-tree-m-u-2way.sh
+++ b/t/t1002-read-tree-m-u-2way.sh
@@ -75,8 +75,8 @@ test_expect_success \
echo yomin >yomin &&
git update-index --add yomin &&
read_tree_u_must_succeed -m -u $treeH $treeM &&
- git ls-files --stage >4.out || return 1
- git diff -U0 --no-index M.out 4.out >4diff.out
+ git ls-files --stage >4.out &&
+ test_might_fail git diff -U0 --no-index M.out 4.out >4diff.out &&
compare_change 4diff.out expected &&
check_cache_at yomin clean &&
sum bozbar frotz nitfol >actual4.sum &&
@@ -94,8 +94,8 @@ test_expect_success \
git update-index --add yomin &&
echo yomin yomin >yomin &&
read_tree_u_must_succeed -m -u $treeH $treeM &&
- git ls-files --stage >5.out || return 1
- git diff -U0 --no-index M.out 5.out >5diff.out
+ git ls-files --stage >5.out &&
+ test_might_fail git diff -U0 --no-index M.out 5.out >5diff.out &&
compare_change 5diff.out expected &&
check_cache_at yomin dirty &&
sum bozbar frotz nitfol >actual5.sum &&
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 61e087e..3d11b7a 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -184,8 +184,8 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '
'
test_expect_success 'did not use upload-pack service' '
- grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act
- : >exp
+ test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act &&
+ : >exp &&
test_cmp exp act
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (8 preceding siblings ...)
2015-03-20 10:11 ` [PATCH 09/25] t: use test_might_fail for diff and grep Jeff King
@ 2015-03-20 10:11 ` Jeff King
2015-03-20 10:12 ` [PATCH 11/25] t: wrap complicated expect_code users in a block Jeff King
` (18 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:11 UTC (permalink / raw)
To: git
This makes our output in the event of a failure slightly
nicer, and it means that we do not break the &&-chain.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0040-parse-options.sh | 12 +++------
t/t4035-diff-quiet.sh | 66 +++++++++++++++++++++++-------------------------
t/t4053-diff-no-index.sh | 4 +--
3 files changed, 38 insertions(+), 44 deletions(-)
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index a90c86b..b044785 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -172,12 +172,9 @@ test_expect_success 'long options' '
'
test_expect_success 'missing required value' '
- test-parse-options -s;
- test $? = 129 &&
- test-parse-options --string;
- test $? = 129 &&
- test-parse-options --file;
- test $? = 129
+ test_expect_code 129 test-parse-options -s &&
+ test_expect_code 129 test-parse-options --string &&
+ test_expect_code 129 test-parse-options --file
'
cat > expect << EOF
@@ -227,8 +224,7 @@ test_expect_success 'unambiguously abbreviated option with "="' '
'
test_expect_success 'ambiguously abbreviated option' '
- test-parse-options --strin 123;
- test $? = 129
+ test_expect_code 129 test-parse-options --strin 123
'
cat > expect << EOF
diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
index e8ae2a0..461f4bb 100755
--- a/t/t4035-diff-quiet.sh
+++ b/t/t4035-diff-quiet.sh
@@ -29,67 +29,65 @@ test_expect_success 'setup' '
'
test_expect_success 'git diff-tree HEAD^ HEAD' '
- git diff-tree --quiet HEAD^ HEAD >cnt
- test $? = 1 && test_line_count = 0 cnt
+ test_expect_code 1 git diff-tree --quiet HEAD^ HEAD >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree HEAD^ HEAD -- a' '
- git diff-tree --quiet HEAD^ HEAD -- a >cnt
- test $? = 0 && test_line_count = 0 cnt
+ test_expect_code 0 git diff-tree --quiet HEAD^ HEAD -- a >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree HEAD^ HEAD -- b' '
- git diff-tree --quiet HEAD^ HEAD -- b >cnt
- test $? = 1 && test_line_count = 0 cnt
+ test_expect_code 1 git diff-tree --quiet HEAD^ HEAD -- b >cnt &&
+ test_line_count = 0 cnt
'
# this diff outputs one line: sha1 of the given head
test_expect_success 'echo HEAD | git diff-tree --stdin' '
- echo $(git rev-parse HEAD) | git diff-tree --quiet --stdin >cnt
- test $? = 1 && test_line_count = 1 cnt
+ echo $(git rev-parse HEAD) |
+ test_expect_code 1 git diff-tree --quiet --stdin >cnt &&
+ test_line_count = 1 cnt
'
test_expect_success 'git diff-tree HEAD HEAD' '
- git diff-tree --quiet HEAD HEAD >cnt
- test $? = 0 && test_line_count = 0 cnt
+ test_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-files' '
- git diff-files --quiet >cnt
- test $? = 0 && test_line_count = 0 cnt
+ test_expect_code 0 git diff-files --quiet >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-index --cached HEAD' '
- git diff-index --quiet --cached HEAD >cnt
- test $? = 0 && test_line_count = 0 cnt
+ test_expect_code 0 git diff-index --quiet --cached HEAD >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-index --cached HEAD^' '
- git diff-index --quiet --cached HEAD^ >cnt
- test $? = 1 && test_line_count = 0 cnt
+ test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-index --cached HEAD^' '
echo text >>b &&
echo 3 >c &&
- git add . && {
- git diff-index --quiet --cached HEAD^ >cnt
- test $? = 1 && test_line_count = 0 cnt
- }
+ git add . &&
+ test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree -Stext HEAD^ HEAD -- b' '
- git commit -m "text in b" && {
- git diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt
- test $? = 1 && test_line_count = 0 cnt
- }
+ git commit -m "text in b" &&
+ test_expect_code 1 git diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree -Snot-found HEAD^ HEAD -- b' '
- git diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt
- test $? = 0 && test_line_count = 0 cnt
+ test_expect_code 0 git diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-files' '
- echo 3 >>c && {
- git diff-files --quiet >cnt
- test $? = 1 && test_line_count = 0 cnt
- }
+ echo 3 >>c &&
+ test_expect_code 1 git diff-files --quiet >cnt &&
+ test_line_count = 0 cnt
'
+
test_expect_success 'git diff-index --cached HEAD' '
- git update-index c && {
- git diff-index --quiet --cached HEAD >cnt
- test $? = 1 && test_line_count = 0 cnt
- }
+ git update-index c &&
+ test_expect_code 1 git diff-index --quiet --cached HEAD >cnt &&
+ test_line_count = 0 cnt
'
test_expect_success 'git diff, one file outside repo' '
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 2ab3c48..075ece6 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -17,8 +17,8 @@ test_expect_success 'setup' '
'
test_expect_success 'git diff --no-index directories' '
- git diff --no-index a b >cnt
- test $? = 1 && test_line_count = 14 cnt
+ test_expect_code 1 git diff --no-index a b >cnt &&
+ test_line_count = 14 cnt
'
test_expect_success 'git diff --no-index relative path outside repo' '
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 11/25] t: wrap complicated expect_code users in a block
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (9 preceding siblings ...)
2015-03-20 10:11 ` [PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison Jeff King
@ 2015-03-20 10:12 ` Jeff King
2015-03-20 10:12 ` [PATCH 12/25] t: avoid using ":" for comments Jeff King
` (17 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:12 UTC (permalink / raw)
To: git
If we are expecting a command to produce a particular exit
code, we can use test_expect_code. However, some cases are
more complicated, and want to accept one of a range of exit
codes. For these, we end up with something like:
cmd;
case "$?" in
...
That unfortunately breaks the &&-chain and fools
--chain-lint. Since these special cases are so few, we can
wrap them in a block, like this:
{ cmd; ret=$?; } &&
case "$ret" in
...
This accomplishes the same thing, and retains the &&-chain
(the exit status fed to the && is that of the assignment,
which should always be true). It's technically longer, but
it is probably a good thing for unusual code like this to
stand out.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0005-signals.sh | 4 ++--
t/t4026-color.sh | 6 +++---
t/t5004-archive-corner-cases.sh | 6 ++++--
t/t5512-ls-remote.sh | 6 ++++--
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh
index aeea50c..5c5707d 100755
--- a/t/t0005-signals.sh
+++ b/t/t0005-signals.sh
@@ -10,8 +10,8 @@ one
EOF
test_expect_success 'sigchain works' '
- test-sigchain >actual
- case "$?" in
+ { test-sigchain >actual; ret=$?; } &&
+ case "$ret" in
143) true ;; # POSIX w/ SIGTERM=15
271) true ;; # ksh w/ SIGTERM=15
3) true ;; # Windows
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index 4d20fea..2b32c4f 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -111,9 +111,9 @@ test_expect_success 'unknown color slots are ignored (branch)' '
'
test_expect_success 'unknown color slots are ignored (status)' '
- git config color.status.nosuchslotwilleverbedefined white || exit
- git status
- case $? in 0|1) : ok ;; *) false ;; esac
+ git config color.status.nosuchslotwilleverbedefined white &&
+ { git status; ret=$?; } &&
+ case $ret in 0|1) : ok ;; *) false ;; esac
'
test_done
diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index 305bcac..654adda 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -66,8 +66,10 @@ test_expect_success UNZIP 'zip archive of empty tree is empty' '
# handle the empty repo at all, making our later check of its exit code
# a no-op). But we cannot do anything reasonable except skip the test
# on such platforms anyway, and this is the moral equivalent.
- "$GIT_UNZIP" "$TEST_DIRECTORY"/t5004/empty.zip
- expect_code=$?
+ {
+ "$GIT_UNZIP" "$TEST_DIRECTORY"/t5004/empty.zip
+ expect_code=$?
+ } &&
git archive --format=zip HEAD >empty.zip &&
make_dir extract &&
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 321c3e5..3bd9759 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -103,8 +103,10 @@ test_expect_success 'confuses pattern as remote when no remote specified' '
'
test_expect_success 'die with non-2 for wrong repository even with --exit-code' '
- git ls-remote --exit-code ./no-such-repository ;# not &&
- status=$? &&
+ {
+ git ls-remote --exit-code ./no-such-repository
+ status=$?
+ } &&
test $status != 2 && test $status != 0
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 12/25] t: avoid using ":" for comments
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (10 preceding siblings ...)
2015-03-20 10:12 ` [PATCH 11/25] t: wrap complicated expect_code users in a block Jeff King
@ 2015-03-20 10:12 ` Jeff King
2015-03-20 10:12 ` [PATCH 13/25] t3600: fix &&-chain breakage for setup commands Jeff King
` (16 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:12 UTC (permalink / raw)
To: git
The ":" is not a comment marker, but rather a noop command.
Using it as a comment like:
: do something
cmd1 &&
: something else
cmd2
breaks the &&-chain, and we would fail to notice if "cmd1"
failed in this instance. We can just use regular "#"
comments instead.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4104-apply-boundary.sh | 18 +++++++++---------
t/t5533-push-cas.sh | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh
index c97aad1..497afdc 100755
--- a/t/t4104-apply-boundary.sh
+++ b/t/t4104-apply-boundary.sh
@@ -18,7 +18,7 @@ test_expect_success setup '
cat victim >original &&
git update-index --add victim &&
- : add to the head
+ # add to the head
for i in a b '"$L"' y
do
echo $i
@@ -27,7 +27,7 @@ test_expect_success setup '
git diff victim >add-a-patch.with &&
git diff --unified=0 >add-a-patch.without &&
- : insert at line two
+ # insert at line two
for i in b a '"$L"' y
do
echo $i
@@ -36,7 +36,7 @@ test_expect_success setup '
git diff victim >insert-a-patch.with &&
git diff --unified=0 >insert-a-patch.without &&
- : modify at the head
+ # modify at the head
for i in a '"$L"' y
do
echo $i
@@ -45,16 +45,16 @@ test_expect_success setup '
git diff victim >mod-a-patch.with &&
git diff --unified=0 >mod-a-patch.without &&
- : remove from the head
+ # remove from the head
for i in '"$L"' y
do
echo $i
done >victim &&
cat victim >del-a-expect &&
- git diff victim >del-a-patch.with
+ git diff victim >del-a-patch.with &&
git diff --unified=0 >del-a-patch.without &&
- : add to the tail
+ # add to the tail
for i in b '"$L"' y z
do
echo $i
@@ -63,7 +63,7 @@ test_expect_success setup '
git diff victim >add-z-patch.with &&
git diff --unified=0 >add-z-patch.without &&
- : modify at the tail
+ # modify at the tail
for i in b '"$L"' z
do
echo $i
@@ -72,7 +72,7 @@ test_expect_success setup '
git diff victim >mod-z-patch.with &&
git diff --unified=0 >mod-z-patch.without &&
- : remove from the tail
+ # remove from the tail
for i in b '"$L"'
do
echo $i
@@ -81,7 +81,7 @@ test_expect_success setup '
git diff victim >del-z-patch.with &&
git diff --unified=0 >del-z-patch.without
- : done
+ # done
'
for with in with without
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index dccf8a6..c402d8d 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -14,7 +14,7 @@ setup_srcdst_basic () {
}
test_expect_success setup '
- : create template repository
+ # create template repository
test_commit A &&
test_commit B &&
test_commit C
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 13/25] t3600: fix &&-chain breakage for setup commands
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (11 preceding siblings ...)
2015-03-20 10:12 ` [PATCH 12/25] t: avoid using ":" for comments Jeff King
@ 2015-03-20 10:12 ` Jeff King
2015-03-20 10:12 ` [PATCH 14/25] t7201: fix &&-chain breakage Jeff King
` (15 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:12 UTC (permalink / raw)
To: git
As with the earlier patch to fix "trivial" &&-chain
breakage, these missing "&&" operators are not a serious
problem (e.g., we do not expect "echo" to fail).
Ironically, however, inserting them shows that some of the
commands _do_ fail. Specifically, some of the tests start by
making sure we are at a commit with the string "content" in
the file "foo". However, running "git commit" may fail
because the previous test left us in that state already, and
there is nothing to commit.
We could remove these commands entirely, but they serve to
document the test's assumptions, as well as make it robust
when an earlier test has failed. We could use test_might_fail
to handle all cases, but that would miss an unrelated
failure to make the commit. Instead, we can just pass the
--allow-empty flag to git-commit, which means that it will
not complain if our setup is a noop.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t3600-rm.sh | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 1962989..9d90d2c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -38,37 +38,37 @@ test_expect_success \
test_expect_success \
'Test that git rm --cached foo succeeds if the index matches the file' \
- 'echo content > foo
- git add foo
+ 'echo content >foo &&
+ git add foo &&
git rm --cached foo'
test_expect_success \
'Test that git rm --cached foo succeeds if the index matches the file' \
- 'echo content > foo
- git add foo
- git commit -m foo
- echo "other content" > foo
+ 'echo content >foo &&
+ git add foo &&
+ git commit -m foo &&
+ echo "other content" >foo &&
git rm --cached foo'
test_expect_success \
'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' '
- echo content > foo
- git add foo
- git commit -m foo
- echo "other content" > foo
- git add foo
- echo "yet another content" > foo
+ echo content >foo &&
+ git add foo &&
+ git commit -m foo --allow-empty &&
+ echo "other content" >foo &&
+ git add foo &&
+ echo "yet another content" >foo &&
test_must_fail git rm --cached foo
'
test_expect_success \
'Test that git rm --cached -f foo works in case where --cached only did not' \
- 'echo content > foo
- git add foo
- git commit -m foo
- echo "other content" > foo
- git add foo
- echo "yet another content" > foo
+ 'echo content >foo &&
+ git add foo &&
+ git commit -m foo --allow-empty &&
+ echo "other content" >foo &&
+ git add foo &&
+ echo "yet another content" >foo &&
git rm --cached -f foo'
test_expect_success \
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 14/25] t7201: fix &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (12 preceding siblings ...)
2015-03-20 10:12 ` [PATCH 13/25] t3600: fix &&-chain breakage for setup commands Jeff King
@ 2015-03-20 10:12 ` Jeff King
2015-03-20 10:13 ` [PATCH 15/25] t9502: " Jeff King
` (14 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:12 UTC (permalink / raw)
To: git
One of these breakages is in setup, but one is more severe
and may miss a real test failure. These are pulled out from
the rest, though, because we also clean up a few other
anachronisms. The most interesting is the use of this
here-doc construct:
(cat >... <<EOF
...
EOF
) &&
It looks like an attempt to make the &&-chaining more
natural by letting it come at the end of the here-doc. But
the extra sub-shell is so non-idiomatic (plus the lack of
"<<-") that it ends up confusing.
Since these are just using a single line, we can accomplish
the same thing with a single printf (which also makes the
use of tab more obvious than the verbatim whitespace).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7201-co.sh | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index a7fe4e6..8859236 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -88,14 +88,10 @@ test_expect_success "checkout with unrelated dirty tree without -m" '
git checkout -f master &&
fill 0 1 2 3 4 5 6 7 8 >same &&
- cp same kept
+ cp same kept &&
git checkout side >messages &&
- test_cmp same kept
- (cat > messages.expect <<EOF
-M same
-EOF
-) &&
- touch messages.expect &&
+ test_cmp same kept &&
+ printf "M\t%s\n" same >messages.expect &&
test_cmp messages.expect messages
'
@@ -109,10 +105,7 @@ test_expect_success "checkout -m with dirty tree" '
test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
- (cat >expect.messages <<EOF
-M one
-EOF
-) &&
+ printf "M\t%s\n" one >expect.messages &&
test_cmp expect.messages messages &&
fill "M one" "A three" "D two" >expect.master &&
@@ -409,12 +402,12 @@ test_expect_success \
test_expect_success \
'checkout w/autosetupmerge=always sets up tracking' '
+ test_when_finished git config branch.autosetupmerge false &&
git config branch.autosetupmerge always &&
git checkout master &&
git checkout -b track2 &&
test "$(git config branch.track2.remote)" &&
- test "$(git config branch.track2.merge)"
- git config branch.autosetupmerge false'
+ test "$(git config branch.track2.merge)"'
test_expect_success 'checkout w/--track from non-branch HEAD fails' '
git checkout master^0 &&
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 15/25] t9502: fix &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (13 preceding siblings ...)
2015-03-20 10:12 ` [PATCH 14/25] t7201: fix &&-chain breakage Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 17:48 ` Johannes Sixt
2015-03-20 10:13 ` [PATCH 16/25] t6030: use modern test_* helpers Jeff King
` (13 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
This script misses a trivial &&-chain in one of its tests,
but it also has a weird reverse: it includes an &&-chain
outside of any test_expect block! This "cat" should never
fail, but if it did, we would not notice, as it would cause
us to skip the follow-on test entirely (which does not
appear intentional; there are many later tests which rely on
this cat).
Let's instead move the setup into its own test_expect_success
block, which is the standard practice nowadays.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t9502-gitweb-standalone-parse-output.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 86dfee2..a93e159 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -145,9 +145,11 @@ test_expect_success 'forks: not skipped unless "forks" feature enabled' '
grep -q ">fork of .*<" gitweb.body
'
-cat >>gitweb_config.perl <<\EOF &&
-$feature{'forks'}{'default'} = [1];
-EOF
+test_expect_success 'enable forks feature' '
+ cat >>gitweb_config.perl <<-\EOF
+ $feature{'forks'}{'default'} = [1];
+ EOF
+'
test_expect_success 'forks: forks skipped if "forks" feature enabled' '
gitweb_run "a=project_list" &&
@@ -173,7 +175,7 @@ test_expect_success 'forks: can access forked repository' '
'
test_expect_success 'forks: project_index lists all projects (incl. forks)' '
- cat >expected <<-\EOF
+ cat >expected <<-\EOF &&
.git
foo.bar.git
foo.git
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 16/25] t6030: use modern test_* helpers
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (14 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 15/25] t9502: " Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:13 ` [PATCH 17/25] t0020: " Jeff King
` (12 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
We can get rid of a lot of hand-rolled error messages by
using test_must_fail and test_expect_code. The existing code
was careful to use "|| return 1" when breaking the
&&-chain, but it did fool --chain-lint; the new code is more
idiomatic.
We also add some uses of test_when_finished, which is less
cryptic and more robust than putting code at the end of a
test. In two cases we run "git bisect reset" from a
subshell, which is a problem for test_when_finished (it
would not run). However, in both of these cases, we are
performing the tests in one-off sub-repos, so we do not need
to clean up at all (and in fact it is nicer not to if the
user wants to inspect the trash directory after a failure).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t6030-bisect-porcelain.sh | 91 +++++++++++++++------------------------------
1 file changed, 31 insertions(+), 60 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index e6abe65..06b4868 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -52,15 +52,8 @@ test_expect_success 'bisect starts with only one bad' '
test_expect_success 'bisect does not start with only one good' '
git bisect reset &&
git bisect start &&
- git bisect good $HASH1 || return 1
-
- if git bisect next
- then
- echo Oops, should have failed.
- false
- else
- :
- fi
+ git bisect good $HASH1 &&
+ test_must_fail git bisect next
'
test_expect_success 'bisect start with one bad and good' '
@@ -191,34 +184,27 @@ test_expect_success 'bisect start: no ".git/BISECT_START" if checkout error' '
# but $HASH2 is bad,
# so we should find $HASH2 as the first bad commit
test_expect_success 'bisect skip: successful result' '
+ test_when_finished git bisect reset &&
git bisect reset &&
git bisect start $HASH4 $HASH1 &&
git bisect skip &&
git bisect bad > my_bisect_log.txt &&
- grep "$HASH2 is the first bad commit" my_bisect_log.txt &&
- git bisect reset
+ grep "$HASH2 is the first bad commit" my_bisect_log.txt
'
# $HASH1 is good, $HASH4 is bad, we skip $HASH3 and $HASH2
# so we should not be able to tell the first bad commit
# among $HASH2, $HASH3 and $HASH4
test_expect_success 'bisect skip: cannot tell between 3 commits' '
+ test_when_finished git bisect reset &&
git bisect start $HASH4 $HASH1 &&
- git bisect skip || return 1
-
- if git bisect skip > my_bisect_log.txt
- then
- echo Oops, should have failed.
- false
- else
- test $? -eq 2 &&
- grep "first bad commit could be any of" my_bisect_log.txt &&
- ! grep $HASH1 my_bisect_log.txt &&
- grep $HASH2 my_bisect_log.txt &&
- grep $HASH3 my_bisect_log.txt &&
- grep $HASH4 my_bisect_log.txt &&
- git bisect reset
- fi
+ git bisect skip &&
+ test_expect_code 2 git bisect skip >my_bisect_log.txt &&
+ grep "first bad commit could be any of" my_bisect_log.txt &&
+ ! grep $HASH1 my_bisect_log.txt &&
+ grep $HASH2 my_bisect_log.txt &&
+ grep $HASH3 my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt
'
# $HASH1 is good, $HASH4 is bad, we skip $HASH3
@@ -226,22 +212,15 @@ test_expect_success 'bisect skip: cannot tell between 3 commits' '
# so we should not be able to tell the first bad commit
# among $HASH3 and $HASH4
test_expect_success 'bisect skip: cannot tell between 2 commits' '
+ test_when_finished git bisect reset &&
git bisect start $HASH4 $HASH1 &&
- git bisect skip || return 1
-
- if git bisect good > my_bisect_log.txt
- then
- echo Oops, should have failed.
- false
- else
- test $? -eq 2 &&
- grep "first bad commit could be any of" my_bisect_log.txt &&
- ! grep $HASH1 my_bisect_log.txt &&
- ! grep $HASH2 my_bisect_log.txt &&
- grep $HASH3 my_bisect_log.txt &&
- grep $HASH4 my_bisect_log.txt &&
- git bisect reset
- fi
+ git bisect skip &&
+ test_expect_code 2 git bisect good >my_bisect_log.txt &&
+ grep "first bad commit could be any of" my_bisect_log.txt &&
+ ! grep $HASH1 my_bisect_log.txt &&
+ ! grep $HASH2 my_bisect_log.txt &&
+ grep $HASH3 my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt
'
# $HASH1 is good, $HASH4 is both skipped and bad, we skip $HASH3
@@ -249,24 +228,18 @@ test_expect_success 'bisect skip: cannot tell between 2 commits' '
# so we should not be able to tell the first bad commit
# among $HASH3 and $HASH4
test_expect_success 'bisect skip: with commit both bad and skipped' '
+ test_when_finished git bisect reset &&
git bisect start &&
git bisect skip &&
git bisect bad &&
git bisect good $HASH1 &&
git bisect skip &&
- if git bisect good > my_bisect_log.txt
- then
- echo Oops, should have failed.
- false
- else
- test $? -eq 2 &&
- grep "first bad commit could be any of" my_bisect_log.txt &&
- ! grep $HASH1 my_bisect_log.txt &&
- ! grep $HASH2 my_bisect_log.txt &&
- grep $HASH3 my_bisect_log.txt &&
- grep $HASH4 my_bisect_log.txt &&
- git bisect reset
- fi
+ test_expect_code 2 git bisect good >my_bisect_log.txt &&
+ grep "first bad commit could be any of" my_bisect_log.txt &&
+ ! grep $HASH1 my_bisect_log.txt &&
+ ! grep $HASH2 my_bisect_log.txt &&
+ grep $HASH3 my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt
'
# We want to automatically find the commit that
@@ -601,8 +574,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout specified' '
git bisect bad $HASH4 &&
git bisect run eval \
"test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0" \
- >../nocheckout.log &&
- git bisect reset
+ >../nocheckout.log
) &&
grep "$HASH3 is the first bad commit" nocheckout.log
'
@@ -617,8 +589,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout defaulted' '
git bisect bad $HASH4 &&
git bisect run eval \
"test \$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0" \
- >../defaulted.log &&
- git bisect reset
+ >../defaulted.log
) &&
grep "$HASH3 is the first bad commit" defaulted.log
'
@@ -642,14 +613,14 @@ test_expect_success 'broken branch creation' '
mkdir missing &&
:> missing/MISSING &&
git add missing/MISSING &&
- git commit -m "6(broken): Added file that will be deleted"
+ git commit -m "6(broken): Added file that will be deleted" &&
git tag BROKEN_HASH6 &&
add_line_into_file "7(broken): second line on a broken branch" hello2 &&
git tag BROKEN_HASH7 &&
add_line_into_file "8(broken): third line on a broken branch" hello2 &&
git tag BROKEN_HASH8 &&
git rm missing/MISSING &&
- git commit -m "9(broken): Remove missing file"
+ git commit -m "9(broken): Remove missing file" &&
git tag BROKEN_HASH9 &&
rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 17/25] t0020: use modern test_* helpers
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (15 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 16/25] t6030: use modern test_* helpers Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-25 0:23 ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 18/25] t1301: use modern test_* helpers Jeff King
` (11 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
This test contains a lot of hand-rolled messages to show
when the test fails. We can omit most of these by using
"verbose" and "test_must_fail". A few of them are for
update-index, but we can assume it produces reasonable error
messages when it fails.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0020-crlf.sh | 144 +++++++++++---------------------------------------------
1 file changed, 28 insertions(+), 116 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index d2e51a8..9fa26df 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '
for f in one dir/two
do
append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo Oops
- false
- break
- }
+ git update-index -- $f ||
+ break
done &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
@@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '
for f in one dir/two
do
append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo "Oops $f"
- false
- break
- }
+ git update-index -- $f ||
+ break
done &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
@@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '
for f in one dir/two
do
remove_cr <"$f" >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo "Eh? $f"
- false
- break
- }
+ verbose git update-index -- $f ||
+ break
done &&
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
test_expect_success 'checkout with autocrlf=input' '
@@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
test_expect_success 'apply patch (autocrlf=input)' '
@@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply patch.file &&
- test "$patched" = "$(git hash-object --stdin <one)" || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = "$(git hash-object --stdin <one)"
'
test_expect_success 'apply patch --cached (autocrlf=input)' '
@@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached (autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply --cached patch.file &&
- test "$patched" = $(git rev-parse :one) || {
- echo "Eh? apply with --cached"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=input)' '
@@ -226,11 +199,8 @@ test_expect_success 'apply patch --index (autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply --index patch.file &&
- test "$patched" = $(git rev-parse :one) &&
- test "$patched" = $(git hash-object --stdin <one) || {
- echo "Eh? apply with --index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one) &&
+ verbose test "$patched" = $(git hash-object --stdin <one)
'
test_expect_success 'apply patch (autocrlf=true)' '
@@ -240,10 +210,7 @@ test_expect_success 'apply patch (autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply patch.file &&
- test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
'
test_expect_success 'apply patch --cached (autocrlf=true)' '
@@ -253,10 +220,7 @@ test_expect_success 'apply patch --cached (autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply --cached patch.file &&
- test "$patched" = $(git rev-parse :one) || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=true)' '
@@ -266,11 +230,8 @@ test_expect_success 'apply patch --index (autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply --index patch.file &&
- test "$patched" = $(git rev-parse :one) &&
- test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
- echo "Eh? apply with --index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one) &&
+ verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
'
test_expect_success '.gitattributes says two is binary' '
@@ -326,21 +287,8 @@ test_expect_success '.gitattributes says two and three are text' '
echo "t* crlf" >.gitattributes &&
git read-tree --reset -u HEAD &&
- if has_cr dir/two
- then
- : happy
- else
- echo "Huh?"
- false
- fi &&
-
- if has_cr three
- then
- : happy
- else
- echo "Huh?"
- false
- fi
+ verbose has_cr dir/two &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (1)' '
@@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' '
rm -rf tmp one dir .gitattributes patch.file three &&
git read-tree --reset -u HEAD &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (2)' '
@@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' '
git read-tree --reset HEAD &&
git checkout-index -f -q -u -a &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (3)' '
@@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' '
git checkout-index -u .gitattributes &&
git checkout-index -u one dir/two three &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (4)' '
@@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' '
git checkout-index -u one dir/two three &&
git checkout-index -u .gitattributes &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'checkout with existing .gitattributes' '
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 18/25] t1301: use modern test_* helpers
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (16 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 17/25] t0020: " Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-24 23:51 ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 19/25] t6034: " Jeff King
` (10 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
This shortens the code and fixes some &&-chaining.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1301-shared-repo.sh | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 7eecfb8..ac10875 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -12,12 +12,11 @@ setfacl -k . 2>/dev/null
# User must have read permissions to the repo -> failure on --shared=0400
test_expect_success 'shared = 0400 (faulty permission u-w)' '
+ test_when_finished "rm -rf sub" &&
mkdir sub && (
- cd sub && git init --shared=0400
+ cd sub &&
+ test_must_fail git init --shared=0400
)
- ret="$?"
- rm -rf sub
- test $ret != "0"
'
modebits () {
@@ -33,7 +32,7 @@ do
git init --shared=1 &&
test 1 = "$(git config core.sharedrepository)"
) &&
- actual=$(ls -l sub/.git/HEAD)
+ actual=$(ls -l sub/.git/HEAD) &&
case "$actual" in
-rw-rw-r--*)
: happy
@@ -90,10 +89,8 @@ do
rm -f .git/info/refs &&
git update-server-info &&
actual="$(modebits .git/info/refs)" &&
- test "x$actual" = "x-$y" || {
- ls -lt .git/info
- false
- }
+ verbose test "x$actual" = "x-$y"
+
'
umask 077 &&
@@ -102,10 +99,7 @@ do
rm -f .git/info/refs &&
git update-server-info &&
actual="$(modebits .git/info/refs)" &&
- test "x$actual" = "x-$x" || {
- ls -lt .git/info
- false
- }
+ verbose test "x$actual" = "x-$x"
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 19/25] t6034: use modern test_* helpers
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (17 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 18/25] t1301: use modern test_* helpers Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-24 23:43 ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 20/25] t4117: " Jeff King
` (9 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
These say roughly the same thing as the hand-rolled
messages. We do lose the "merge did not complete" debug
message, but merge and write-tree are prefectly capable of
writing useful error messages when they fail.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t6034-merge-rename-nocruft.sh | 66 ++++++++---------------------------------
1 file changed, 13 insertions(+), 53 deletions(-)
diff --git a/t/t6034-merge-rename-nocruft.sh b/t/t6034-merge-rename-nocruft.sh
index 65be95f..89871aa 100755
--- a/t/t6034-merge-rename-nocruft.sh
+++ b/t/t6034-merge-rename-nocruft.sh
@@ -73,33 +73,12 @@ test_expect_success 'merge white into red (A->B,M->N)' \
'
git checkout -b red-white red &&
git merge white &&
- git write-tree >/dev/null || {
- echo "BAD: merge did not complete"
- return 1
- }
-
- test -f B || {
- echo "BAD: B does not exist in working directory"
- return 1
- }
- test -f N || {
- echo "BAD: N does not exist in working directory"
- return 1
- }
- test -f R || {
- echo "BAD: R does not exist in working directory"
- return 1
- }
-
- test -f A && {
- echo "BAD: A still exists in working directory"
- return 1
- }
- test -f M && {
- echo "BAD: M still exists in working directory"
- return 1
- }
- return 0
+ git write-tree &&
+ test_path_is_file B &&
+ test_path_is_file N &&
+ test_path_is_file R &&
+ test_path_is_missing A &&
+ test_path_is_missing M
'
# This test broke in 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9
@@ -108,32 +87,13 @@ test_expect_success 'merge blue into white (A->B, mod A, A untracked)' \
git checkout -b white-blue white &&
echo dirty >A &&
git merge blue &&
- git write-tree >/dev/null || {
- echo "BAD: merge did not complete"
- return 1
- }
-
- test -f A || {
- echo "BAD: A does not exist in working directory"
- return 1
- }
- test `cat A` = dirty || {
- echo "BAD: A content is wrong"
- return 1
- }
- test -f B || {
- echo "BAD: B does not exist in working directory"
- return 1
- }
- test -f N || {
- echo "BAD: N does not exist in working directory"
- return 1
- }
- test -f M && {
- echo "BAD: M still exists in working directory"
- return 1
- }
- return 0
+ git write-tree &&
+ test_path_is_file A &&
+ echo dirty >expect &&
+ test_cmp expect A &&
+ test_path_is_file B &&
+ test_path_is_file N &&
+ test_path_is_missing M
'
test_done
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 20/25] t4117: use modern test_* helpers
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (18 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 19/25] t6034: " Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:13 ` [PATCH 21/25] t9001: use test_when_finished Jeff King
` (8 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
We can use test_must_fail and test_path_* to avoid some
hand-rolled if statements. This makes the code shorter, and
makes it more obvious when we are breaking the &&-chain.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4117-apply-reject.sh | 66 ++++++++-----------------------------------------
1 file changed, 10 insertions(+), 56 deletions(-)
diff --git a/t/t4117-apply-reject.sh b/t/t4117-apply-reject.sh
index 8e15ecb..d80187d 100755
--- a/t/t4117-apply-reject.sh
+++ b/t/t4117-apply-reject.sh
@@ -56,23 +56,13 @@ test_expect_success 'apply --reject is incompatible with --3way' '
test_expect_success 'apply without --reject should fail' '
- if git apply patch.1
- then
- echo "Eh? Why?"
- exit 1
- fi
-
+ test_must_fail git apply patch.1 &&
test_cmp file1 saved.file1
'
test_expect_success 'apply without --reject should fail' '
- if git apply --verbose patch.1
- then
- echo "Eh? Why?"
- exit 1
- fi
-
+ test_must_fail git apply --verbose patch.1 &&
test_cmp file1 saved.file1
'
@@ -81,21 +71,11 @@ test_expect_success 'apply with --reject should fail but update the file' '
cat saved.file1 >file1 &&
rm -f file1.rej file2.rej &&
- if git apply --reject patch.1
- then
- echo "succeeds with --reject?"
- exit 1
- fi
-
+ test_must_fail git apply --reject patch.1 &&
test_cmp file1 expected &&
cat file1.rej &&
-
- if test -f file2.rej
- then
- echo "file2 should not have been touched"
- exit 1
- fi
+ test_path_is_missing file2.rej
'
test_expect_success 'apply with --reject should fail but update the file' '
@@ -103,25 +83,12 @@ test_expect_success 'apply with --reject should fail but update the file' '
cat saved.file1 >file1 &&
rm -f file1.rej file2.rej file2 &&
- if git apply --reject patch.2 >rejects
- then
- echo "succeeds with --reject?"
- exit 1
- fi
-
- test -f file1 && {
- echo "file1 still exists?"
- exit 1
- }
+ test_must_fail git apply --reject patch.2 >rejects &&
+ test_path_is_missing file1 &&
test_cmp file2 expected &&
cat file2.rej &&
-
- if test -f file1.rej
- then
- echo "file2 should not have been touched"
- exit 1
- fi
+ test_path_is_missing file1.rej
'
@@ -130,25 +97,12 @@ test_expect_success 'the same test with --verbose' '
cat saved.file1 >file1 &&
rm -f file1.rej file2.rej file2 &&
- if git apply --reject --verbose patch.2 >rejects
- then
- echo "succeeds with --reject?"
- exit 1
- fi
-
- test -f file1 && {
- echo "file1 still exists?"
- exit 1
- }
+ test_must_fail git apply --reject --verbose patch.2 >rejects &&
+ test_path_is_missing file1 &&
test_cmp file2 expected &&
cat file2.rej &&
-
- if test -f file1.rej
- then
- echo "file2 should not have been touched"
- exit 1
- fi
+ test_path_is_missing file1.rej
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 21/25] t9001: use test_when_finished
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (19 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 20/25] t4117: " Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-25 2:00 ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 22/25] t0050: appease --chain-lint Jeff King
` (7 subsequent siblings)
28 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
The confirmation tests in t9001 all save the value of
sendemail.confirm, do something to it, then restore it at
the end, in a way that breaks the &&-chain (they are not
wrong, because they save the $? value, but it fools
--chain-lint).
Instead, they can all use test_when_finished, and we can
even make the code simpler by factoring out the shared
lines.
Note that we can _almost_ use test_config here, except that:
1. We do not restore the config with test_unconfig, but by
setting it back to some prior value.
2. We are not always setting a config variable. Sometimes
the change to be undone is unsetting it entirely.
We could teach test_config to handle these cases, but it's
not worth the complexity for a single call-site.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t9001-send-email.sh | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 37caa18..c9f54d5 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' '
test_confirm --confirm=compose --compose
'
-test_expect_success $PREREQ 'confirm by default (due to cc)' '
+save_confirm () {
CONFIRM=$(git config --get sendemail.confirm) &&
+ test_when_finished "git config sendemail.confirm ${CONFIRM:-never}"
+}
+
+test_expect_success $PREREQ 'confirm by default (due to cc)' '
+ save_confirm &&
git config --unset sendemail.confirm &&
test_confirm
- ret="$?"
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = "0"
'
test_expect_success $PREREQ 'confirm by default (due to --compose)' '
- CONFIRM=$(git config --get sendemail.confirm) &&
+ save_confirm &&
git config --unset sendemail.confirm &&
test_confirm --suppress-cc=all --compose
- ret="$?"
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = "0"
'
test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
- CONFIRM=$(git config --get sendemail.confirm) &&
+ save_confirm &&
git config --unset sendemail.confirm &&
rm -fr outdir &&
git format-patch -2 -o outdir &&
@@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
--to=nobody@example.com \
--smtp-server="$(pwd)/fake.sendmail" \
outdir/*.patch </dev/null
- ret="$?"
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = "0"
'
test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
- CONFIRM=$(git config --get sendemail.confirm) &&
+ save_confirm &&
git config sendemail.confirm auto &&
GIT_SEND_EMAIL_NOTTY=1 &&
export GIT_SEND_EMAIL_NOTTY &&
@@ -861,13 +857,10 @@ test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
--to=nobody@example.com \
--smtp-server="$(pwd)/fake.sendmail" \
$patches </dev/null
- ret="$?"
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = "0"
'
test_expect_success $PREREQ 'confirm does not loop forever' '
- CONFIRM=$(git config --get sendemail.confirm) &&
+ save_confirm &&
git config sendemail.confirm auto &&
GIT_SEND_EMAIL_NOTTY=1 &&
export GIT_SEND_EMAIL_NOTTY &&
@@ -876,9 +869,6 @@ test_expect_success $PREREQ 'confirm does not loop forever' '
--to=nobody@example.com \
--smtp-server="$(pwd)/fake.sendmail" \
$patches
- ret="$?"
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = "0"
'
test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 22/25] t0050: appease --chain-lint
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (20 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 21/25] t9001: use test_when_finished Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:13 ` [PATCH 23/25] t7004: fix embedded single-quotes Jeff King
` (6 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
Some of the symlink tests check an either-or case using the
"||". This is not wrong, but fools --chain-lint into
thinking the &&-chain is broken (in fact, there is no &&
chain here).
We can solve this by wrapping the "||" inside a {} block.
This is a bit more verbose, but this construct is rare, and
the {} block helps call attention to it.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0050-filesystem.sh | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 988c392..b29d749 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -33,16 +33,20 @@ test_expect_success "detection of case insensitive filesystem during repo init"
'
else
test_expect_success "detection of case insensitive filesystem during repo init" '
- test_must_fail git config --bool core.ignorecase >/dev/null ||
- test $(git config --bool core.ignorecase) = false
+ {
+ test_must_fail git config --bool core.ignorecase >/dev/null ||
+ test $(git config --bool core.ignorecase) = false
+ }
'
fi
if test_have_prereq SYMLINKS
then
test_expect_success "detection of filesystem w/o symlink support during repo init" '
- test_must_fail git config --bool core.symlinks ||
- test "$(git config --bool core.symlinks)" = true
+ {
+ test_must_fail git config --bool core.symlinks ||
+ test "$(git config --bool core.symlinks)" = true
+ }
'
else
test_expect_success "detection of filesystem w/o symlink support during repo init" '
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 23/25] t7004: fix embedded single-quotes
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (21 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 22/25] t0050: appease --chain-lint Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:13 ` [PATCH 24/25] t0005: fix broken &&-chains Jeff King
` (5 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
This test uses single quotes inside the single-quoted test
snippet, which effectively makes the contents unquoted.
Since they don't need quoted anyway, this isn't a problem,
but let's switch them to double-quotes to make it more
obviously correct.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7004-tag.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 347d3be..efb08c3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1181,7 +1181,7 @@ test_expect_success \
'message in editor has initial comment: remainder' '
# remove commented lines from the remainder -- should be empty
>rest.expect &&
- sed -e 1d -e '/^#/d' <actual >rest.actual &&
+ sed -e 1d -e "/^#/d" <actual >rest.actual &&
test_cmp rest.expect rest.actual
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 24/25] t0005: fix broken &&-chains
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (22 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 23/25] t7004: fix embedded single-quotes Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:13 ` [PATCH 25/25] t4104: drop hand-rolled error reporting Jeff King
` (4 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
The ":" noop command always returns true, so it is fine to
include these lines in an &&-chain (and it appeases
--chain-lint).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0005-signals.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh
index 5c5707d..e7f27eb 100755
--- a/t/t0005-signals.sh
+++ b/t/t0005-signals.sh
@@ -40,12 +40,12 @@ test_expect_success 'create blob' '
'
test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '
- OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 )
+ OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
test "$OUT" -eq 141
'
test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '
- OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 )
+ OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&
test "$OUT" -eq 141
'
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 25/25] t4104: drop hand-rolled error reporting
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (23 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 24/25] t0005: fix broken &&-chains Jeff King
@ 2015-03-20 10:13 ` Jeff King
2015-03-20 10:23 ` [PATCH 0/25] detecting &&-chain breakage Jeff King
` (3 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:13 UTC (permalink / raw)
To: git
This use of "||" fools --chain-lint into thinking the
&&-chain is broken (and indeed, it is somewhat broken; a
failure of update-index in these tests would show the patch
file, even if we never got to the part of the test where we
fed the patch to git-apply).
The extra blocks were there to include more debugging
output, but it hardly seems worth it; the user should know
which command failed (because git-apply will produce error
messages) and can look in the trash directory themselves.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t4104-apply-boundary.sh | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh
index 497afdc..32e3b0e 100755
--- a/t/t4104-apply-boundary.sh
+++ b/t/t4104-apply-boundary.sh
@@ -95,10 +95,7 @@ do
test_expect_success "apply $kind-patch $with context" '
cat original >victim &&
git update-index victim &&
- git apply --index '"$u$kind-patch.$with"' || {
- cat '"$kind-patch.$with"'
- (exit 1)
- } &&
+ git apply --index '"$u$kind-patch.$with"' &&
test_cmp '"$kind"'-expect victim
'
done
@@ -113,10 +110,7 @@ do
test_expect_success "apply non-git $kind-patch without context" '
cat original >victim &&
git update-index victim &&
- git apply --unidiff-zero --index '"$kind-ng.without"' || {
- cat '"$kind-ng.without"'
- (exit 1)
- } &&
+ git apply --unidiff-zero --index '"$kind-ng.without"' &&
test_cmp '"$kind"'-expect victim
'
done
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (24 preceding siblings ...)
2015-03-20 10:13 ` [PATCH 25/25] t4104: drop hand-rolled error reporting Jeff King
@ 2015-03-20 10:23 ` Jeff King
2015-03-20 14:28 ` Michael J Gruber
` (2 subsequent siblings)
28 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 10:23 UTC (permalink / raw)
To: git
On Fri, Mar 20, 2015 at 06:04:30AM -0400, Jeff King wrote:
> It's a lot of patches, and a few of them are long. I tried to group
> them by functionality rather than file (though as you can see, some of
> the tests were unique enough snowflakes that it made sense to discuss
> their issues separately). I'm hoping it should be an easy review, if
> not a short one.
I hoped that would make it easier to review, but I was also curious
about the patterns I would see. Here are a few things I noticed:
- the single most common place to forget an "&&" was at the start of a
here-doc
I complained earlier that I would prefer a way to put it at the end.
And indeed, I found somebody who agreed with me and was more clever
than I. They wrote in one test:
(cat >expect <<EOF
... whatever ...
EOF
) &&
which I suspect would be less error-prone, but is also quite ugly. I
actually pulled that out in favor of a more conventional form
(ironically, I found it because the author screwed up the
&&-operator a few lines above).
With automated &&-checking, I don't think there's any need to
contort our syntax. The test suite will remind us when we forget.
- the other common place I noticed was at the trailing ")" of a
sub-shell
- Running through old tests, I saw a lot of opportunity for cleanups
that I resisted. Places where we could use "verbose" for better
output, where "test_cmp" would be nicer than using "test X = Y",
where people had indented here-docs badly (mostly failing to use
"<<-", places where people had some arcane indentation style for the
test titles and snippets themselves, etc.
The series was already bordering on intractable, so I tried to leave
those be (though I did fix "cat > foo" redirects when I was touching
those particular lines :) ).
Which is all to say, please don't mention ugly style issues you see
in the context lines during review, unless it is to point me to your
patch series that comes on top of mine and cleans them up. :)
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (25 preceding siblings ...)
2015-03-20 10:23 ` [PATCH 0/25] detecting &&-chain breakage Jeff King
@ 2015-03-20 14:28 ` Michael J Gruber
2015-03-20 14:32 ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
2015-03-20 17:57 ` [PATCH 0/25] detecting " Jeff King
2015-03-20 17:44 ` Junio C Hamano
2015-03-20 23:18 ` Eric Sunshine
28 siblings, 2 replies; 68+ messages in thread
From: Michael J Gruber @ 2015-03-20 14:28 UTC (permalink / raw)
To: Jeff King, git
Jeff King venit, vidit, dixit 20.03.2015 11:04:
> This is a cleanup of the &&-chain lint I posted earlier:
>
> http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859
>
> I don't know who came up with the idea for it originally, but the
> concept certainly was floating in the back of my mind from Jonathan's
> earlier version that is referenced in that thread.
>
> The general idea is to detect &&-chain breakage that can lead to our
> tests yielding false success. The first patch implements and discusses
> the lint-check itself, which is quite simple. The bulk of the work was
> fixing all of the issues in the existing tests. :)
>
> That didn't all need to happen immediately. I mainly wanted to start on
> it to answer two questions:
>
> 1. Are &&-chain breakages actually preventing us from seeing any test
> failures? Or is it mostly just pedantry, and we miss out only on
> knowing whether "cat >expect <<-\EOF" failed (which presumably it
> never does).
>
> 2. How bad are the false positives? Both how common, and how bad to
> work around.
>
> But after a few hours, I reached a zen state and just kept going. So at
> the end of this series, the whole test suite is --chain-lint clean
> (modulo any tests that are skipped on my platform). We could even switch
> the checks on by default at the end of the series, but I didn't do that
> here. I think it would be sane to run them all the time, though; in the
> normal success case, they don't add any forks (the shell just runs
> "(exit) && ...", and realizes that the whole thing is one big &&-chain).
> I couldn't measure any time difference running the suite with and
> without it.
>
> Anyway, to answer the questions: Yes, there were definitely tests whose
> values were being thrown away, and we would not have noticed if they
> failed. The good news is that all of them did pass once we started
> checking their results. Hooray.
>
> There were a number of false positives, though as a percentage of the
> test suite, probably not many (it's just that we have quite a lot of
> tests). Most of them were in rather old tests, and IMHO the fixes I did
> actually improved the readability of the result. So overall I think this
> is a very positive change; I doubt it will get in people's way very
> often, and I look forward to having one less thing to worry about
> handling manually in review. The biggest downside is that I may have
> automated Eric Sunshine out of a job. :)
>
> The patches are:
>
> [01/25]: t/test-lib: introduce --chain-lint option
> [02/25]: t: fix severe &&-chain breakage
> [03/25]: t: fix moderate &&-chain breakage
> [04/25]: t: fix trivial &&-chain breakage
> [05/25]: t: assume test_cmp produces verbose output
> [06/25]: t: use verbose instead of hand-rolled errors
> [07/25]: t: use test_must_fail instead of hand-rolled blocks
> [08/25]: t: fix &&-chaining issues around setup which might fail
> [09/25]: t: use test_might_fail for diff and grep
> [10/25]: t: use test_expect_code instead of hand-rolled comparison
> [11/25]: t: wrap complicated expect_code users in a block
> [12/25]: t: avoid using ":" for comments
> [13/25]: t3600: fix &&-chain breakage for setup commands
> [14/25]: t7201: fix &&-chain breakage
> [15/25]: t9502: fix &&-chain breakage
> [16/25]: t6030: use modern test_* helpers
> [17/25]: t0020: use modern test_* helpers
> [18/25]: t1301: use modern test_* helpers
> [19/25]: t6034: use modern test_* helpers
> [20/25]: t4117: use modern test_* helpers
> [21/25]: t9001: use test_when_finished
> [22/25]: t0050: appease --chain-lint
> [23/25]: t7004: fix embedded single-quotes
> [24/25]: t0005: fix broken &&-chains
> [25/25]: t4104: drop hand-rolled error reporting
>
> It's a lot of patches, and a few of them are long. I tried to group
> them by functionality rather than file (though as you can see, some of
> the tests were unique enough snowflakes that it made sense to discuss
> their issues separately). I'm hoping it should be an easy review, if
> not a short one.
>
> I'll spare you the full per-file diffstat, but the total is a very
> satisfying:
>
> 84 files changed, 397 insertions(+), 647 deletions(-)
>
> That's a lot of changes in a lot of hunks, but most of them are in files
> that haven't been touched in a while. The series merges cleanly with
> "pu", so I don't think I've stepped on anyone's topics in flight.
>
> -Peff
Nice series! In fact, it's a great place to learn many of the test suite
features that we have nowadays.
With 1/25 only, I get 163 dubious or failed on current next.
With 1/25 and only chain-lint without running the actual test loads, I
get 213.
So, just as Jeff explained, we don't want a "chain-lint-only mode"
because it does not give correct results.
I reviewed the first 15 and they all look good.
I skimmed through the rest and it appears good to (though this is no
review).
You run without git svn tests, obviously... I'll follow up with two
patches for those.
Michael
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 14:28 ` Michael J Gruber
@ 2015-03-20 14:32 ` Michael J Gruber
2015-03-20 14:32 ` [PATCH 27/27] t9104: fix test for following larger parents Michael J Gruber
2015-03-20 18:04 ` [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage Junio C Hamano
2015-03-20 17:57 ` [PATCH 0/25] detecting " Jeff King
1 sibling, 2 replies; 68+ messages in thread
From: Michael J Gruber @ 2015-03-20 14:32 UTC (permalink / raw)
To: git; +Cc: Jeff King, Eric Wong
All of these cases are moderate since they would most probably not
lead to missed failing tests: Either they would fail otherwise,
or fail a rm in test_when_finished only.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
t/t2026-prune-linked-checkouts.sh | 4 ++--
t/t9158-git-svn-mergeinfo.sh | 4 ++--
t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
index 2936d52..e885baf 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '
'
test_expect_success 'not prune locked checkout' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir -p .git/worktrees/ghi &&
: >.git/worktrees/ghi/locked &&
git prune --worktrees &&
@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '
'
test_expect_success 'not prune recent checkouts' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir zz &&
mkdir -p .git/worktrees/jlm &&
echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir &&
diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh
index 8c9539e..13f78f2 100755
--- a/t/t9158-git-svn-mergeinfo.sh
+++ b/t/t9158-git-svn-mergeinfo.sh
@@ -34,7 +34,7 @@ test_expect_success 'change svn:mergeinfo' '
'
test_expect_success 'verify svn:mergeinfo' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) &&
test "$mergeinfo" = "/branches/foo:1-10"
'
@@ -46,7 +46,7 @@ test_expect_success 'change svn:mergeinfo multiline' '
'
test_expect_success 'verify svn:mergeinfo multiline' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) &&
test "$mergeinfo" = "/branches/bar:1-10
/branches/other:3-5,8,10-11"
'
diff --git a/t/t9161-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh
index 6cb0909..f113aca 100755
--- a/t/t9161-git-svn-mergeinfo-push.sh
+++ b/t/t9161-git-svn-mergeinfo-push.sh
@@ -24,7 +24,7 @@ test_expect_success 'propagate merge information' '
'
test_expect_success 'check svn:mergeinfo' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) &&
test "$mergeinfo" = "/branches/svnb2:3,8"
'
@@ -34,7 +34,7 @@ test_expect_success 'merge another branch' '
'
test_expect_success 'check primary parent mergeinfo respected' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) &&
test "$mergeinfo" = "/branches/svnb2:3,8
/branches/svnb3:4,9"
'
@@ -45,7 +45,7 @@ test_expect_success 'merge existing merge' '
'
test_expect_success "check both parents' mergeinfo respected" '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) &&
test "$mergeinfo" = "/branches/svnb2:3,8
/branches/svnb3:4,9
/branches/svnb4:5-6,10-12
@@ -70,7 +70,7 @@ test_expect_success 'second forward merge' '
'
test_expect_success 'check new mergeinfo added' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) &&
test "$mergeinfo" = "/branches/svnb2:3,8,16-17
/branches/svnb3:4,9
/branches/svnb4:5-6,10-12
@@ -84,7 +84,7 @@ test_expect_success 'reintegration merge' '
'
test_expect_success 'check reintegration mergeinfo' '
- mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4)
+ mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4) &&
test "$mergeinfo" = "/branches/svnb1:2-4,7-9,13-18
/branches/svnb2:3,8,16-17
/branches/svnb3:4,9
--
2.3.3.438.g7254779
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 27/27] t9104: fix test for following larger parents
2015-03-20 14:32 ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
@ 2015-03-20 14:32 ` Michael J Gruber
2015-03-20 18:04 ` [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage Junio C Hamano
1 sibling, 0 replies; 68+ messages in thread
From: Michael J Gruber @ 2015-03-20 14:32 UTC (permalink / raw)
To: git; +Cc: Jeff King, Eric Wong
This test is special for several reasons:
It ends with a "true" statement, which should be a no-op.
It is not because the &&-chain is broken right before it.
Also, looking at what the test intended to test according to
7f578c5 (git-svn: --follow-parent now works on sub-directories of larger
branches, 2007-01-24)
it is not clear how it would achieve that with the given steps.
Amend the test to include the second svn id to be tested for, and
change the tested refs to the ones which are to be expected, and which
make the test pass.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
In fact, these merge-base tests look kind of ugly, too.
But they appear in many more places here and should best be rewritten
separately.
t/t9104-git-svn-follow-parent.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index 13b179e..1bd1022 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -72,16 +72,18 @@ test_expect_success 'follow larger parent' '
svn import -m "import a larger parent" import "$svnrepo"/larger-parent &&
svn cp -m "hi" "$svnrepo"/larger-parent "$svnrepo"/another-larger &&
git svn init --minimize-url -i larger \
- "$svnrepo"/another-larger/trunk/thunk/bump/thud &&
+ "$svnrepo"/larger-parent/trunk/thunk/bump/thud &&
git svn fetch -i larger &&
+ git svn init --minimize-url -i larger-parent \
+ "$svnrepo"/another-larger/trunk/thunk/bump/thud &&
+ git svn fetch -i larger-parent &&
git rev-parse --verify refs/remotes/larger &&
git rev-parse --verify \
- refs/remotes/larger-parent/trunk/thunk/bump/thud &&
+ refs/remotes/larger-parent &&
test "`git merge-base \
- refs/remotes/larger-parent/trunk/thunk/bump/thud \
+ refs/remotes/larger-parent \
refs/remotes/larger`" = \
"`git rev-parse refs/remotes/larger`"
- true
'
test_expect_success 'follow higher-level parent' '
--
2.3.3.438.g7254779
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (26 preceding siblings ...)
2015-03-20 14:28 ` Michael J Gruber
@ 2015-03-20 17:44 ` Junio C Hamano
2015-03-20 18:00 ` Junio C Hamano
2015-03-20 23:18 ` Eric Sunshine
28 siblings, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-20 17:44 UTC (permalink / raw)
To: Jeff King; +Cc: git, Nguyễn Thái Ngọc Duy
Thanks. They applied cleanly on 'master' and all looked sensible.
I found 2026 and 5312 to be broken (there may be others that are
excluded in my usual test set) in 'pu'. As to these topics in "git
log --first-parent master..pu", my preference is to queue fixups on
the broken-out topics (available at http://github.com/gitster/git)
independently.
For example, this will go on top of nd/multiple-work-trees.
diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
index 2936d52..e885baf 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '
'
test_expect_success 'not prune locked checkout' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir -p .git/worktrees/ghi &&
: >.git/worktrees/ghi/locked &&
git prune --worktrees &&
@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '
'
test_expect_success 'not prune recent checkouts' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir zz &&
mkdir -p .git/worktrees/jlm &&
echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir &&
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 15/25] t9502: fix &&-chain breakage
2015-03-20 10:13 ` [PATCH 15/25] t9502: " Jeff King
@ 2015-03-20 17:48 ` Johannes Sixt
2015-03-20 18:03 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Johannes Sixt @ 2015-03-20 17:48 UTC (permalink / raw)
To: Jeff King, git
Am 20.03.2015 um 11:13 schrieb Jeff King:
> This script misses a trivial &&-chain in one of its tests,
> but it also has a weird reverse: it includes an &&-chain
> outside of any test_expect block! This "cat" should never
> fail, but if it did, we would not notice, as it would cause
> us to skip the follow-on test entirely (which does not
> appear intentional; there are many later tests which rely on
> this cat).
>
> Let's instead move the setup into its own test_expect_success
> block, which is the standard practice nowadays.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9502-gitweb-standalone-parse-output.sh | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
> index 86dfee2..a93e159 100755
> --- a/t/t9502-gitweb-standalone-parse-output.sh
> +++ b/t/t9502-gitweb-standalone-parse-output.sh
> @@ -145,9 +145,11 @@ test_expect_success 'forks: not skipped unless "forks" feature enabled' '
> grep -q ">fork of .*<" gitweb.body
> '
>
> -cat >>gitweb_config.perl <<\EOF &&
> -$feature{'forks'}{'default'} = [1];
> -EOF
> +test_expect_success 'enable forks feature' '
> + cat >>gitweb_config.perl <<-\EOF
> + $feature{'forks'}{'default'} = [1];
> + EOF
> +'
This loses the single-quotes in the generated perl script, doesn't it?
Most likely, it does not matter.
-- Hannes
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 14:28 ` Michael J Gruber
2015-03-20 14:32 ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
@ 2015-03-20 17:57 ` Jeff King
1 sibling, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 17:57 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Nguyễn Thái Ngọc Duy, git
On Fri, Mar 20, 2015 at 03:28:11PM +0100, Michael J Gruber wrote:
> With 1/25 only, I get 163 dubious or failed on current next.
> With 1/25 and only chain-lint without running the actual test loads, I
> get 213.
>
> So, just as Jeff explained, we don't want a "chain-lint-only mode"
> because it does not give correct results.
Thanks for checking. I had assumed there would be some weirdness, but I
didn't actually try a lint-only mode.
I only ran the lint against master earlier. There are two trivial broken
chains in pu. Patch is below (against nd/multiple-work-trees). Looks
like that is in 'next', so we can't just squash it in.
-- >8 --
Subject: t2026: fix broken &&-chains
These are totally trivial (test_when_finished should never
fail), but being complete with our &&-chaining makes the new
--chain-lint feature more useful.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t2026-prune-linked-checkouts.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
index 2936d52..e885baf 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '
'
test_expect_success 'not prune locked checkout' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir -p .git/worktrees/ghi &&
: >.git/worktrees/ghi/locked &&
git prune --worktrees &&
@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '
'
test_expect_success 'not prune recent checkouts' '
- test_when_finished rm -r .git/worktrees
+ test_when_finished rm -r .git/worktrees &&
mkdir zz &&
mkdir -p .git/worktrees/jlm &&
echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir &&
--
2.3.3.520.g3cfbb5d
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 17:44 ` Junio C Hamano
@ 2015-03-20 18:00 ` Junio C Hamano
2015-03-20 18:04 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-20 18:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> I found 2026 and 5312 to be broken (there may be others that are
> excluded in my usual test set) in 'pu'. As to these topics in "git
> log --first-parent master..pu", my preference is to queue fixups on
> the broken-out topics (available at http://github.com/gitster/git)
> independently.
>
> For example, this will go on top of nd/multiple-work-trees.
... and this goes on top of jk/prune-with-corrupt-refs until it is
rerolled.
-- >8 --
Subject: [PATCH] SQUASH??? t5312: fix broken &&-chain
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5312-prune-corruption.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index 8b54d16..9633d97 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -80,7 +80,7 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '
# skip processing a broken ref
test_expect_success 'create packed-refs file with broken ref' '
rm -f .git/refs/heads/master &&
- cat >.git/packed-refs <<-EOF
+ cat >.git/packed-refs <<-EOF &&
$missing refs/heads/master
$recoverable refs/heads/other
EOF
--
2.3.3-401-g402122f
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 15/25] t9502: fix &&-chain breakage
2015-03-20 17:48 ` Johannes Sixt
@ 2015-03-20 18:03 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-20 18:03 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
On Fri, Mar 20, 2015 at 06:48:35PM +0100, Johannes Sixt wrote:
> >-cat >>gitweb_config.perl <<\EOF &&
> >-$feature{'forks'}{'default'} = [1];
> >-EOF
> >+test_expect_success 'enable forks feature' '
> >+ cat >>gitweb_config.perl <<-\EOF
> >+ $feature{'forks'}{'default'} = [1];
> >+ EOF
> >+'
>
> This loses the single-quotes in the generated perl script, doesn't it? Most
> likely, it does not matter.
Eek, you're right. Thanks for noticing.
I agree it should not matter (it is OK in perl to use barewords as hash
keys, but it would probably be more obvious to omit them entirely, or to
just use double-quotes. I'll squash this in:
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index a93e159..0796a43 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -147,7 +147,7 @@ test_expect_success 'forks: not skipped unless "forks" feature enabled' '
test_expect_success 'enable forks feature' '
cat >>gitweb_config.perl <<-\EOF
- $feature{'forks'}{'default'} = [1];
+ $feature{"forks"}{"default"} = [1];
EOF
'
-Peff
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 14:32 ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
2015-03-20 14:32 ` [PATCH 27/27] t9104: fix test for following larger parents Michael J Gruber
@ 2015-03-20 18:04 ` Junio C Hamano
2015-03-20 19:35 ` Junio C Hamano
1 sibling, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-20 18:04 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King, Eric Wong
Michael J Gruber <git@drmicha.warpmail.net> writes:
> All of these cases are moderate since they would most probably not
> lead to missed failing tests: Either they would fail otherwise,
> or fail a rm in test_when_finished only.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> t/t2026-prune-linked-checkouts.sh | 4 ++--
> t/t9158-git-svn-mergeinfo.sh | 4 ++--
> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----
> 3 files changed, 9 insertions(+), 9 deletions(-)
Ahh, it seems that I should have read everything in my inbox before
starting my day X-<. I already queued an identical patch for 2026
on nd/multiple-work-trees, and its new tip is in 'next' now.
Which branches are the git-svn ones meant to apply? Are they meant
to fix an existing bug already in master, or are they new ones added
by still-in-flight topics? Can you split if necessary and mark them
for individual topios in flight if that is the case, so that we can
apply them independently from GIT_TEST_CHAIN_LINT series?
Thanks.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 18:00 ` Junio C Hamano
@ 2015-03-20 18:04 ` Jeff King
2015-03-20 18:33 ` Junio C Hamano
0 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 18:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Fri, Mar 20, 2015 at 11:00:05AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I found 2026 and 5312 to be broken (there may be others that are
> > excluded in my usual test set) in 'pu'. As to these topics in "git
> > log --first-parent master..pu", my preference is to queue fixups on
> > the broken-out topics (available at http://github.com/gitster/git)
> > independently.
> >
> > For example, this will go on top of nd/multiple-work-trees.
Heh, I just crossed emails with you over the same patch.
> ... and this goes on top of jk/prune-with-corrupt-refs until it is
> rerolled.
I'm actually about to send out a re-roll of that, as I think all of the
review comments have been addressed.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 18:04 ` Jeff King
@ 2015-03-20 18:33 ` Junio C Hamano
0 siblings, 0 replies; 68+ messages in thread
From: Junio C Hamano @ 2015-03-20 18:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
Jeff King <peff@peff.net> writes:
> I'm actually about to send out a re-roll of that, as I think all of the
> review comments have been addressed.
Thanks.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 18:04 ` [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage Junio C Hamano
@ 2015-03-20 19:35 ` Junio C Hamano
2015-03-20 20:02 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-20 19:35 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King, Eric Wong
Junio C Hamano <gitster@pobox.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> All of these cases are moderate since they would most probably not
>> lead to missed failing tests: Either they would fail otherwise,
>> or fail a rm in test_when_finished only.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>> t/t2026-prune-linked-checkouts.sh | 4 ++--
>> t/t9158-git-svn-mergeinfo.sh | 4 ++--
>> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----
>> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> Ahh, it seems that I should have read everything in my inbox before
> starting my day X-<. I already queued an identical patch for 2026
> on nd/multiple-work-trees, and its new tip is in 'next' now.
>
> Which branches are the git-svn ones meant to apply? Are they meant
> to fix an existing bug already in master, or are they new ones added
> by still-in-flight topics? Can you split if necessary and mark them
> for individual topios in flight if that is the case, so that we can
> apply them independently from GIT_TEST_CHAIN_LINT series?
>
> Thanks.
OK, I think I figured it out. All the git-svn bits in 26 and 27 are
for 'master', and should be part of Peff's original 25-patch series.
I'll drop 2026 from your 26/25 and queue it together with your 27/25
on top off it.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 19:35 ` Junio C Hamano
@ 2015-03-20 20:02 ` Jeff King
2015-03-20 20:13 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 20:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git, Eric Wong
On Fri, Mar 20, 2015 at 12:35:56PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Michael J Gruber <git@drmicha.warpmail.net> writes:
> >
> >> All of these cases are moderate since they would most probably not
> >> lead to missed failing tests: Either they would fail otherwise,
> >> or fail a rm in test_when_finished only.
> >>
> >> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> >> ---
> >> t/t2026-prune-linked-checkouts.sh | 4 ++--
> >> t/t9158-git-svn-mergeinfo.sh | 4 ++--
> >> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----
> >> 3 files changed, 9 insertions(+), 9 deletions(-)
> >
> > Ahh, it seems that I should have read everything in my inbox before
> > starting my day X-<. I already queued an identical patch for 2026
> > on nd/multiple-work-trees, and its new tip is in 'next' now.
> >
> > Which branches are the git-svn ones meant to apply? Are they meant
> > to fix an existing bug already in master, or are they new ones added
> > by still-in-flight topics? Can you split if necessary and mark them
> > for individual topios in flight if that is the case, so that we can
> > apply them independently from GIT_TEST_CHAIN_LINT series?
> >
> > Thanks.
>
> OK, I think I figured it out. All the git-svn bits in 26 and 27 are
> for 'master', and should be part of Peff's original 25-patch series.
> I'll drop 2026 from your 26/25 and queue it together with your 27/25
> on top off it.
Yeah, that was my impression, too. I don't have svn installed on my
system, so I missed those ones. I don't have CVS either. That might be
worth following up on.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 20:02 ` Jeff King
@ 2015-03-20 20:13 ` Jeff King
2015-03-23 9:36 ` Michael J Gruber
0 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-20 20:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git, Eric Wong
On Fri, Mar 20, 2015 at 04:02:39PM -0400, Jeff King wrote:
> Yeah, that was my impression, too. I don't have svn installed on my
> system, so I missed those ones. I don't have CVS either. That might be
> worth following up on.
Hmm, that turned out rather easy. No breakages at all in the cvs tests.
It almost makes me think I ran the tests wrong. ;)
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
` (27 preceding siblings ...)
2015-03-20 17:44 ` Junio C Hamano
@ 2015-03-20 23:18 ` Eric Sunshine
2015-03-21 8:19 ` Jeff King
28 siblings, 1 reply; 68+ messages in thread
From: Eric Sunshine @ 2015-03-20 23:18 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
On Fri, Mar 20, 2015 at 6:04 AM, Jeff King <peff@peff.net> wrote:
> [...]
> There were a number of false positives, though as a percentage of the
> test suite, probably not many (it's just that we have quite a lot of
> tests). Most of them were in rather old tests, and IMHO the fixes I did
> actually improved the readability of the result. So overall I think this
> is a very positive change; I doubt it will get in people's way very
> often, and I look forward to having one less thing to worry about
> handling manually in review. The biggest downside is that I may have
> automated Eric Sunshine out of a job. :)
Heh. I won't mind. Thanks for doing a thorough job.
Ironically, one of the broken here-doc &&-links you detected with
--chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb
(t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17).
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-20 23:18 ` Eric Sunshine
@ 2015-03-21 8:19 ` Jeff King
2015-03-21 18:01 ` Junio C Hamano
0 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-21 8:19 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List
On Fri, Mar 20, 2015 at 07:18:47PM -0400, Eric Sunshine wrote:
> Ironically, one of the broken here-doc &&-links you detected with
> --chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb
> (t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17).
Heh. I was afraid to look at my own.
I wondered if we had a good way of finding who wrote the specific lines
that were changed in a patch. I wrote the script below before realizing
that contrib/contacts does something similar (though I think it blames
whole hunks, including context, and I really want to care specifically
only about touched lines; I wonder if that is a distinction git-contacts
should make).
Running:
git diff origin origin/jk/test-chain-lint |
perl diff-blame.pl jk/test-chain-lint |
grep EOF
was fun. At least I am not the only one. :)
Nor the worst in the "severe" category. I will leave using the script to
read the list of "severe" names as an exercise to the reader; reading
such output is probably not overly healthy (and besides, to be fair it
really should be normalized over the number of contributions).
-- >8 --
# diff-blame.pl
use warnings FATAL => 'all';
use strict;
my $head = shift
or die "usage: git diff <start> <end> | $0 <start> [blame-opts]";
my $file;
my @lines;
my $line_nr;
while (<STDIN>) {
if (m{^--- .*?/(.*)}) {
flush();
$file = $1;
@lines = ();
}
elsif (m{^@@ -(\d+)}) {
$line_nr = $1;
}
elsif (m{^ }) {
$line_nr++;
}
elsif (m{^-}) {
push @lines, $line_nr++;
}
}
flush();
exit 0;
sub flush {
return unless defined $file && @lines > 0;
# XXX coalesce blocks of adjacent lines into ranges?
system(qw(git --no-pager blame), @ARGV,
(map { "-L$_,$_" } @lines), $head, $file);
}
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-21 8:19 ` Jeff King
@ 2015-03-21 18:01 ` Junio C Hamano
2015-03-21 22:23 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-21 18:01 UTC (permalink / raw)
To: Jeff King; +Cc: Eric Sunshine, Git List
Jeff King <peff@peff.net> writes:
> Running:
>
> git diff origin origin/jk/test-chain-lint |
> perl diff-blame.pl jk/test-chain-lint |
> grep EOF
>
> was fun. At least I am not the only one. :)
The parameter to diff-blame.pl should be origin, instead of
jk/test-chain-lint, I presume? You are grabbing the preimage line
numbers and asking blame to find out who wrote them.
> Nor the worst in the "severe" category.
I do not quite get what this means---the script does not seem to
judge what is severe and what is not, so I presume that this is to
be judged by whoever is reading the output from the above pipeline
after replacing "grep EOF" with "less" or something?
> # diff-blame.pl
> use warnings FATAL => 'all';
> use strict;
>
> my $head = shift
> or die "usage: git diff <start> <end> | $0 <start> [blame-opts]";
>
> my $file;
> my @lines;
> my $line_nr;
>
> while (<STDIN>) {
> if (m{^--- .*?/(.*)}) {
This may match a removal of a line that begins with "^-- something/" ;-)
> flush();
> $file = $1;
> @lines = ();
> }
> elsif (m{^@@ -(\d+)}) {
> $line_nr = $1;
> }
> elsif (m{^ }) {
> $line_nr++;
> }
> elsif (m{^-}) {
> push @lines, $line_nr++;
> }
> }
> flush();
> exit 0;
>
> sub flush {
> return unless defined $file && @lines > 0;
>
> # XXX coalesce blocks of adjacent lines into ranges?
> system(qw(git --no-pager blame), @ARGV,
You may want to pass an option to always show the filename here.
> (map { "-L$_,$_" } @lines), $head, $file);
> }
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/25] detecting &&-chain breakage
2015-03-21 18:01 ` Junio C Hamano
@ 2015-03-21 22:23 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-21 22:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Sunshine, Git List
On Sat, Mar 21, 2015 at 11:01:43AM -0700, Junio C Hamano wrote:
> > Running:
> >
> > git diff origin origin/jk/test-chain-lint |
> > perl diff-blame.pl jk/test-chain-lint |
> > grep EOF
> >
> > was fun. At least I am not the only one. :)
>
> The parameter to diff-blame.pl should be origin, instead of
> jk/test-chain-lint, I presume? You are grabbing the preimage line
> numbers and asking blame to find out who wrote them.
Yes, sorry, that was an error translating from what I actually ran in
the shell into the email. It should be "origin". And if the script
really wanted to be user-friendly, it should probably take two endpoints
and just run the diff itself (when I started it, I assume that you could
process any diff, but of course you must know the start point to get a
reasonable blame).
> > Nor the worst in the "severe" category.
>
> I do not quite get what this means---the script does not seem to
> judge what is severe and what is not, so I presume that this is to
> be judged by whoever is reading the output from the above pipeline
> after replacing "grep EOF" with "less" or something?
That was the exercise I left to the reader. :) In this case, it is
possible because I have already split the patches into "severe",
"moderate", and "trivial" cases, so you can blame only the severe patch
(using its parent as the start-point).
> > while (<STDIN>) {
> > if (m{^--- .*?/(.*)}) {
>
> This may match a removal of a line that begins with "^-- something/" ;-)
True. I was trying to avoid being stateful in my diff parsing. I guess
it would be enough to parse the hunk headers to know how many lines are
in the hunk. Not worth it for this one-off, but a good thing if somebody
wanted to pick this idea up for a "real" tool.
> > # XXX coalesce blocks of adjacent lines into ranges?
> > system(qw(git --no-pager blame), @ARGV,
>
> You may want to pass an option to always show the filename here.
I left that to the user. I actually found "--line-porcelain" useful for
gathering statistics (e.g., piped to "grep '^author ' | sort | uniq -c").
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
2015-03-20 20:13 ` Jeff King
@ 2015-03-23 9:36 ` Michael J Gruber
0 siblings, 0 replies; 68+ messages in thread
From: Michael J Gruber @ 2015-03-23 9:36 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git, Eric Wong
Jeff King venit, vidit, dixit 20.03.2015 21:13:
> On Fri, Mar 20, 2015 at 04:02:39PM -0400, Jeff King wrote:
>
>> Yeah, that was my impression, too. I don't have svn installed on my
>> system, so I missed those ones. I don't have CVS either. That might be
>> worth following up on.
>
> Hmm, that turned out rather easy. No breakages at all in the cvs tests.
> It almost makes me think I ran the tests wrong. ;)
>
> -Peff
>
Yes, sorry, just catching up.
I applied to Jeff's series to next at 9c5cf4d, and I run with svn and
cvs tests. Full suite except for:
expensive, ext. cred. helper, mac fs, svnserve, svn-info, git-cvsserver, p4.
I guess I should recheck some prerequisites and adjust the svn-info test
to newer svn...
Michael
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 19/25] t6034: use modern test_* helpers
2015-03-20 10:13 ` [PATCH 19/25] t6034: " Jeff King
@ 2015-03-24 23:43 ` SZEDER Gábor
0 siblings, 0 replies; 68+ messages in thread
From: SZEDER Gábor @ 2015-03-24 23:43 UTC (permalink / raw)
To: Jeff King; +Cc: git
Quoting Jeff King <peff@peff.net>:
> These say roughly the same thing as the hand-rolled
> messages. We do lose the "merge did not complete" debug
> message, but merge and write-tree are prefectly capable of
s/prefectly/perfectly/
> writing useful error messages when they fail.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 18/25] t1301: use modern test_* helpers
2015-03-20 10:13 ` [PATCH 18/25] t1301: use modern test_* helpers Jeff King
@ 2015-03-24 23:51 ` SZEDER Gábor
2015-03-25 2:45 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: SZEDER Gábor @ 2015-03-24 23:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
Quoting Jeff King <peff@peff.net>:
> This shortens the code and fixes some &&-chaining.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t1301-shared-repo.sh | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> index 7eecfb8..ac10875 100755
> --- a/t/t1301-shared-repo.sh
> +++ b/t/t1301-shared-repo.sh
> @@ -12,12 +12,11 @@ setfacl -k . 2>/dev/null
>
> # User must have read permissions to the repo -> failure on --shared=0400
> test_expect_success 'shared = 0400 (faulty permission u-w)' '
> + test_when_finished "rm -rf sub" &&
> mkdir sub && (
> - cd sub && git init --shared=0400
> + cd sub &&
> + test_must_fail git init --shared=0400
> )
> - ret="$?"
> - rm -rf sub
> - test $ret != "0"
> '
>
> modebits () {
> @@ -33,7 +32,7 @@ do
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> - actual=$(ls -l sub/.git/HEAD)
> + actual=$(ls -l sub/.git/HEAD) &&
> case "$actual" in
> -rw-rw-r--*)
> : happy
This hunk could go into the "moderate &&-chain breakage" patch.
Doesn't really matter, what matters most is that it's fixed, but I
really liked your classification of missing &&s in the early patches.
> @@ -90,10 +89,8 @@ do
> rm -f .git/info/refs &&
> git update-server-info &&
> actual="$(modebits .git/info/refs)" &&
> - test "x$actual" = "x-$y" || {
> - ls -lt .git/info
> - false
> - }
> + verbose test "x$actual" = "x-$y"
> +
> '
>
> umask 077 &&
> @@ -102,10 +99,7 @@ do
> rm -f .git/info/refs &&
> git update-server-info &&
> actual="$(modebits .git/info/refs)" &&
> - test "x$actual" = "x-$x" || {
> - ls -lt .git/info
> - false
> - }
> + verbose test "x$actual" = "x-$x"
>
> '
>
> --
> 2.3.3.520.g3cfbb5d
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 17/25] t0020: use modern test_* helpers
2015-03-20 10:13 ` [PATCH 17/25] t0020: " Jeff King
@ 2015-03-25 0:23 ` SZEDER Gábor
2015-03-25 2:56 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: SZEDER Gábor @ 2015-03-25 0:23 UTC (permalink / raw)
To: Jeff King; +Cc: git
Quoting Jeff King <peff@peff.net>:
> This test contains a lot of hand-rolled messages to show
> when the test fails. We can omit most of these by using
> "verbose" and "test_must_fail". A few of them are for
> update-index, but we can assume it produces reasonable error
> messages when it fails.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t0020-crlf.sh | 144
> +++++++++++---------------------------------------------
> 1 file changed, 28 insertions(+), 116 deletions(-)
>
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> index d2e51a8..9fa26df 100755
> --- a/t/t0020-crlf.sh
> +++ b/t/t0020-crlf.sh
> @@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '
> for f in one dir/two
> do
> append_cr <$f >tmp && mv -f tmp $f &&
> - git update-index -- $f || {
> - echo Oops
> - false
> - break
> - }
> + git update-index -- $f ||
> + break
> done &&
Ah, these tests are evil, I remember them from the time when I was
fiddling with Jonathan's patch. They can fail silently without
testing what they were supposed to test.
If something in the loop fails, the break will leave the loop but it
will do so with zero return value and, consequently, the test will
continue as if everything were OK.
And unless it was 'git update-index' that failed in a way that left a
borked index behind, the 'git diff-index --cached' below will not
error out or produce some output that would cause the test to fail.
i.e. I tried e.g.
append_cr <$f >tmp && mv -f tmp $f && false &&
in the loop and the test succeeded.
I think the best fix would be to unroll the loop: after this patch the
loop body consists of only two significant lines and we iterate
through the loop only twice, so the test would be even shorter.
> differs=$(git diff-index --cached HEAD) &&
> - test -z "$differs" || {
> - echo Oops "$differs"
> - false
> - }
> + verbose test -z "$differs"
>
> '
>
> @@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '
> for f in one dir/two
> do
> append_cr <$f >tmp && mv -f tmp $f &&
> - git update-index -- $f || {
> - echo "Oops $f"
> - false
> - break
> - }
> + git update-index -- $f ||
> + break
> done &&
>
> differs=$(git diff-index --cached HEAD) &&
> - test -z "$differs" || {
> - echo Oops "$differs"
> - false
> - }
> + verbose test -z "$differs"
>
> '
>
> @@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '
> for f in one dir/two
> do
> remove_cr <"$f" >tmp && mv -f tmp $f &&
> - git update-index -- $f || {
> - echo "Eh? $f"
> - false
> - break
> - }
> + verbose git update-index -- $f ||
> + break
> done &&
> test "$one" = $(git hash-object --stdin <one) &&
> test "$two" = $(git hash-object --stdin <dir/two) &&
> differs=$(git diff-index --cached HEAD) &&
> - test -z "$differs" || {
> - echo Oops "$differs"
> - false
> - }
> + verbose test -z "$differs"
> '
>
> test_expect_success 'checkout with autocrlf=input' '
> @@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '
> test "$one" = $(git hash-object --stdin <one) &&
> test "$two" = $(git hash-object --stdin <dir/two) &&
> differs=$(git diff-index --cached HEAD) &&
> - test -z "$differs" || {
> - echo Oops "$differs"
> - false
> - }
> + verbose test -z "$differs"
> '
>
> test_expect_success 'apply patch (autocrlf=input)' '
> @@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '
> git read-tree --reset -u HEAD &&
>
> git apply patch.file &&
> - test "$patched" = "$(git hash-object --stdin <one)" || {
> - echo "Eh? apply without index"
> - false
> - }
> + verbose test "$patched" = "$(git hash-object --stdin <one)"
> '
>
> test_expect_success 'apply patch --cached (autocrlf=input)' '
> @@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached
> (autocrlf=input)' '
> git read-tree --reset -u HEAD &&
>
> git apply --cached patch.file &&
> - test "$patched" = $(git rev-parse :one) || {
> - echo "Eh? apply with --cached"
> - false
> - }
> + verbose test "$patched" = $(git rev-parse :one)
> '
>
> test_expect_success 'apply patch --index (autocrlf=input)' '
> @@ -226,11 +199,8 @@ test_expect_success 'apply patch --index
> (autocrlf=input)' '
> git read-tree --reset -u HEAD &&
>
> git apply --index patch.file &&
> - test "$patched" = $(git rev-parse :one) &&
> - test "$patched" = $(git hash-object --stdin <one) || {
> - echo "Eh? apply with --index"
> - false
> - }
> + verbose test "$patched" = $(git rev-parse :one) &&
> + verbose test "$patched" = $(git hash-object --stdin <one)
> '
>
> test_expect_success 'apply patch (autocrlf=true)' '
> @@ -240,10 +210,7 @@ test_expect_success 'apply patch (autocrlf=true)' '
> git read-tree --reset -u HEAD &&
>
> git apply patch.file &&
> - test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
> - echo "Eh? apply without index"
> - false
> - }
> + verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
> '
>
> test_expect_success 'apply patch --cached (autocrlf=true)' '
> @@ -253,10 +220,7 @@ test_expect_success 'apply patch --cached
> (autocrlf=true)' '
> git read-tree --reset -u HEAD &&
>
> git apply --cached patch.file &&
> - test "$patched" = $(git rev-parse :one) || {
> - echo "Eh? apply without index"
> - false
> - }
> + verbose test "$patched" = $(git rev-parse :one)
> '
>
> test_expect_success 'apply patch --index (autocrlf=true)' '
> @@ -266,11 +230,8 @@ test_expect_success 'apply patch --index
> (autocrlf=true)' '
> git read-tree --reset -u HEAD &&
>
> git apply --index patch.file &&
> - test "$patched" = $(git rev-parse :one) &&
> - test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
> - echo "Eh? apply with --index"
> - false
> - }
> + verbose test "$patched" = $(git rev-parse :one) &&
> + verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
> '
>
> test_expect_success '.gitattributes says two is binary' '
> @@ -326,21 +287,8 @@ test_expect_success '.gitattributes says two and
> three are text' '
> echo "t* crlf" >.gitattributes &&
> git read-tree --reset -u HEAD &&
>
> - if has_cr dir/two
> - then
> - : happy
> - else
> - echo "Huh?"
> - false
> - fi &&
> -
> - if has_cr three
> - then
> - : happy
> - else
> - echo "Huh?"
> - false
> - fi
> + verbose has_cr dir/two &&
> + verbose has_cr three
> '
>
> test_expect_success 'in-tree .gitattributes (1)' '
> @@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' '
> rm -rf tmp one dir .gitattributes patch.file three &&
> git read-tree --reset -u HEAD &&
>
> - if has_cr one
> - then
> - echo "Eh? one should not have CRLF"
> - false
> - else
> - : happy
> - fi &&
> - has_cr three || {
> - echo "Eh? three should still have CRLF"
> - false
> - }
> + test_must_fail has_cr one &&
> + verbose has_cr three
> '
>
> test_expect_success 'in-tree .gitattributes (2)' '
> @@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' '
> git read-tree --reset HEAD &&
> git checkout-index -f -q -u -a &&
>
> - if has_cr one
> - then
> - echo "Eh? one should not have CRLF"
> - false
> - else
> - : happy
> - fi &&
> - has_cr three || {
> - echo "Eh? three should still have CRLF"
> - false
> - }
> + test_must_fail has_cr one &&
> + verbose has_cr three
> '
>
> test_expect_success 'in-tree .gitattributes (3)' '
> @@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' '
> git checkout-index -u .gitattributes &&
> git checkout-index -u one dir/two three &&
>
> - if has_cr one
> - then
> - echo "Eh? one should not have CRLF"
> - false
> - else
> - : happy
> - fi &&
> - has_cr three || {
> - echo "Eh? three should still have CRLF"
> - false
> - }
> + test_must_fail has_cr one &&
> + verbose has_cr three
> '
>
> test_expect_success 'in-tree .gitattributes (4)' '
> @@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' '
> git checkout-index -u one dir/two three &&
> git checkout-index -u .gitattributes &&
>
> - if has_cr one
> - then
> - echo "Eh? one should not have CRLF"
> - false
> - else
> - : happy
> - fi &&
> - has_cr three || {
> - echo "Eh? three should still have CRLF"
> - false
> - }
> + test_must_fail has_cr one &&
> + verbose has_cr three
> '
>
> test_expect_success 'checkout with existing .gitattributes' '
> --
> 2.3.3.520.g3cfbb5d
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 21/25] t9001: use test_when_finished
2015-03-20 10:13 ` [PATCH 21/25] t9001: use test_when_finished Jeff King
@ 2015-03-25 2:00 ` SZEDER Gábor
2015-03-25 2:47 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: SZEDER Gábor @ 2015-03-25 2:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
Quoting Jeff King <peff@peff.net>:
> The confirmation tests in t9001 all save the value of
> sendemail.confirm, do something to it, then restore it at
> the end, in a way that breaks the &&-chain (they are not
> wrong, because they save the $? value, but it fools
> --chain-lint).
>
> Instead, they can all use test_when_finished, and we can
> even make the code simpler by factoring out the shared
> lines.
I think that saving the value of 'sendemail.confirm' is not necessary.
There are two blocks of confirmation tests, this patch concerns only tests
of the second block. The first block of confirmation tests is nearly at
the beginning of the file in order to check the "no confirm" cases early.
If any of those fails the remainig tests in the file are skipped because
they might hang. The last of those tests sets 'sendemail.confirm' to
'never' and leaves it so to avoid unintentional prompting in the remaining
tests and then its value is not modified until that second block of
confirm tests are reached. This means that when those tests save the
value of 'sendemail.confirm' they always save 'never'. Then why save it,
just use test_when_finished to restore it to 'never' and all is well.
> Note that we can _almost_ use test_config here, except that:
>
> 1. We do not restore the config with test_unconfig, but by
> setting it back to some prior value.
>
> 2. We are not always setting a config variable. Sometimes
> the change to be undone is unsetting it entirely.
>
> We could teach test_config to handle these cases, but it's
> not worth the complexity for a single call-site.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9001-send-email.sh | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 37caa18..c9f54d5 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' '
> test_confirm --confirm=compose --compose
> '
>
> -test_expect_success $PREREQ 'confirm by default (due to cc)' '
> +save_confirm () {
> CONFIRM=$(git config --get sendemail.confirm) &&
> + test_when_finished "git config sendemail.confirm ${CONFIRM:-never}"
> +}
> +
> +test_expect_success $PREREQ 'confirm by default (due to cc)' '
> + save_confirm &&
> git config --unset sendemail.confirm &&
> test_confirm
> - ret="$?"
> - git config sendemail.confirm ${CONFIRM:-never}
> - test $ret = "0"
> '
>
> test_expect_success $PREREQ 'confirm by default (due to --compose)' '
> - CONFIRM=$(git config --get sendemail.confirm) &&
> + save_confirm &&
> git config --unset sendemail.confirm &&
> test_confirm --suppress-cc=all --compose
> - ret="$?"
> - git config sendemail.confirm ${CONFIRM:-never}
> - test $ret = "0"
> '
>
> test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
> - CONFIRM=$(git config --get sendemail.confirm) &&
> + save_confirm &&
> git config --unset sendemail.confirm &&
> rm -fr outdir &&
> git format-patch -2 -o outdir &&
> @@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF
> (inform assumes y)' '
> --to=nobody@example.com \
> --smtp-server="$(pwd)/fake.sendmail" \
> outdir/*.patch </dev/null
> - ret="$?"
> - git config sendemail.confirm ${CONFIRM:-never}
> - test $ret = "0"
> '
>
> test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
> - CONFIRM=$(git config --get sendemail.confirm) &&
> + save_confirm &&
> git config sendemail.confirm auto &&
> GIT_SEND_EMAIL_NOTTY=1 &&
> export GIT_SEND_EMAIL_NOTTY &&
> @@ -861,13 +857,10 @@ test_expect_success $PREREQ 'confirm detects EOF
> (auto causes failure)' '
> --to=nobody@example.com \
> --smtp-server="$(pwd)/fake.sendmail" \
> $patches </dev/null
> - ret="$?"
> - git config sendemail.confirm ${CONFIRM:-never}
> - test $ret = "0"
> '
>
> test_expect_success $PREREQ 'confirm does not loop forever' '
> - CONFIRM=$(git config --get sendemail.confirm) &&
> + save_confirm &&
> git config sendemail.confirm auto &&
> GIT_SEND_EMAIL_NOTTY=1 &&
> export GIT_SEND_EMAIL_NOTTY &&
> @@ -876,9 +869,6 @@ test_expect_success $PREREQ 'confirm does not loop
> forever' '
> --to=nobody@example.com \
> --smtp-server="$(pwd)/fake.sendmail" \
> $patches
> - ret="$?"
> - git config sendemail.confirm ${CONFIRM:-never}
> - test $ret = "0"
> '
>
> test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '
> --
> 2.3.3.520.g3cfbb5d
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 18/25] t1301: use modern test_* helpers
2015-03-24 23:51 ` SZEDER Gábor
@ 2015-03-25 2:45 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 2:45 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Wed, Mar 25, 2015 at 12:51:20AM +0100, SZEDER Gábor wrote:
> >@@ -33,7 +32,7 @@ do
> > git init --shared=1 &&
> > test 1 = "$(git config core.sharedrepository)"
> > ) &&
> >- actual=$(ls -l sub/.git/HEAD)
> >+ actual=$(ls -l sub/.git/HEAD) &&
> > case "$actual" in
> > -rw-rw-r--*)
> > : happy
>
> This hunk could go into the "moderate &&-chain breakage" patch.
> Doesn't really matter, what matters most is that it's fixed, but I really
> liked your classification of missing &&s in the early patches.
Yeah, these later ones are a mish-mash of real fixes and just quieting
--chain-lint. I pulled out ones that I felt needed a little more
explanation, and generally kept changes for a file together (though I am
not sure not always). I'm not sure it's worth the effort to go through
another round of classifying.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 21/25] t9001: use test_when_finished
2015-03-25 2:00 ` SZEDER Gábor
@ 2015-03-25 2:47 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 2:47 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Wed, Mar 25, 2015 at 03:00:22AM +0100, SZEDER Gábor wrote:
> >Instead, they can all use test_when_finished, and we can
> >even make the code simpler by factoring out the shared
> >lines.
>
> I think that saving the value of 'sendemail.confirm' is not necessary.
>
> There are two blocks of confirmation tests, this patch concerns only tests
> of the second block. The first block of confirmation tests is nearly at
> the beginning of the file in order to check the "no confirm" cases early.
> If any of those fails the remainig tests in the file are skipped because
> they might hang. The last of those tests sets 'sendemail.confirm' to
> 'never' and leaves it so to avoid unintentional prompting in the remaining
> tests and then its value is not modified until that second block of
> confirm tests are reached. This means that when those tests save the
> value of 'sendemail.confirm' they always save 'never'. Then why save it,
> just use test_when_finished to restore it to 'never' and all is well.
Yeah, I suspected this while writing it the patch, but I preferred to
keep it more obvious that there would be no accidental regression, since
the series was already so long (and also because calling save_confirm is
not any worse than test_when_finished).
I don't mind a patch on top simplifying out save_confirm, if you're
confident that's what we're always saving.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 01/25] t/test-lib: introduce --chain-lint option
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
@ 2015-03-25 2:53 ` SZEDER Gábor
2015-03-25 3:05 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: SZEDER Gábor @ 2015-03-25 2:53 UTC (permalink / raw)
To: Jeff King; +Cc: git
Quoting Jeff King <peff@peff.net>:
> However, there are a number of places it cannot reach:
>
> - it cannot find a failure to break out of loops on error,
> like:
>
> cmd1 &&
> for i in a b c; do
> cmd2 $i
> done &&
> cmd3
>
> which will not notice failures of "cmd2 a" or "cmd b"
s/cmd b/cmd2 b/ ?
> - it cannot find a missing &&-chain inside a block or
> subfunction, like:
>
> foo () {
> cmd1
> cmd2
> }
>
> foo &&
> bar
>
> which will not notice a failure of cmd1.
And inside subshells. I think it's worth mentioning, too, because
subshells are used frequently when setting environment variables
( GIT_FOO=bar && export GIT_FOO && cmd1 && ... ) && test_cmp
or changing directory
( cd subdir && cmd1 && ... ) && test_cmp
I was wondering whether we could do better here with helper functions,
something along the lines of:
# Set and export environment variable, automatically unsetting it after
# the test is over
test_setenv () {
eval "$1=\$2" &&
export "$1" &&
# sane_unset, to allow unsetting during the test
test_when_finished "sane_unset '$1'"
}
# Change to given directory, automatically return to current working
# directory after the test is over
test_cd () {
test_when_finished "cd '$PWD'" &&
cd "$1"
}
With these the above examples would become
test_setenv GIT_FOO bar && cmd1 && ... && test_cmp
and
test_cd subdir && cmd1 && ... && test_cmp
which means increased coverage for --chain-lint.
Thanks for working on this. I looked into this after seeing Jonathan's
patch back then, got quite far but never reached a chain-lint-clean
state, and only sent patches for the two most amusing breakages
(ddeaf7ef0d, 056f34bbcd).
I'm glad it's off my TODO list and I don't have to rebase a 1.5 year old
branch to current master :)
Gábor
> - it only checks tests that you run; every platform will
> have some tests skipped due to missing prequisites,
> so it's impossible to say from one run that the test
> suite is free of broken &&-chains. However, all tests get
> run by _somebody_, so eventually we will notice problems.
>
> - it does not operate on test_when_finished or prerequisite
> blocks. It could, but these tends to be much shorter and
> less of a problem, so I punted on them in this patch.
>
> This patch was inspired by an earlier patch by Jonathan
> Nieder:
>
> http://article.gmane.org/gmane.comp.version-control.git/235913
>
> This implementation and all bugs are mine.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/README | 10 ++++++++++
> t/test-lib.sh | 16 ++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/t/README b/t/README
> index d5bb0c9..35438bc 100644
> --- a/t/README
> +++ b/t/README
> @@ -168,6 +168,16 @@ appropriately before running "make".
> Using this option with a RAM-based filesystem (such as tmpfs)
> can massively speed up the test suite.
>
> +--chain-lint::
> +--no-chain-lint::
> + If --chain-lint is enabled, the test harness will check each
> + test to make sure that it properly "&&-chains" all commands (so
> + that a failure in the middle does not go unnoticed by the final
> + exit code of the test). This check is performed in addition to
> + running the tests themselves. You may also enable or disable
> + this feature by setting the GIT_TEST_CHAIN_LINT environment
> + variable to "1" or "0", respectively.
> +
> You can also set the GIT_TEST_INSTALLED environment variable to
> the bindir of an existing git installation to test that installation.
> You still need to have built this git sandbox, from which various
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index c096778..50b3d3f 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -232,6 +232,12 @@ do
> --root=*)
> root=$(expr "z$1" : 'z[^=]*=\(.*\)')
> shift ;;
> + --chain-lint)
> + GIT_TEST_CHAIN_LINT=1
> + shift ;;
> + --no-chain-lint)
> + GIT_TEST_CHAIN_LINT=0
> + shift ;;
> -x)
> trace=t
> verbose=t
> @@ -524,6 +530,16 @@ test_eval_ () {
> test_run_ () {
> test_cleanup=:
> expecting_failure=$2
> +
> + if test "${GIT_TEST_CHAIN_LINT:-0}" != 0; then
> + # 117 is magic because it is unlikely to match the exit
> + # code of other programs
> + test_eval_ "(exit 117) && $1"
> + if test "$?" != 117; then
> + error "bug in the test script: broken &&-chain: $1"
> + fi
> + fi
> +
> setup_malloc_check
> test_eval_ "$1"
> eval_ret=$?
> --
> 2.3.3.520.g3cfbb5d
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 17/25] t0020: use modern test_* helpers
2015-03-25 0:23 ` SZEDER Gábor
@ 2015-03-25 2:56 ` Jeff King
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-25 2:56 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Wed, Mar 25, 2015 at 01:23:23AM +0100, SZEDER Gábor wrote:
> > for f in one dir/two
> > do
> > append_cr <$f >tmp && mv -f tmp $f &&
> >- git update-index -- $f || {
> >- echo Oops
> >- false
> >- break
> >- }
> >+ git update-index -- $f ||
> >+ break
> > done &&
>
> Ah, these tests are evil, I remember them from the time when I was fiddling
> with Jonathan's patch. They can fail silently without testing what they
> were supposed to test.
>
> If something in the loop fails, the break will leave the loop but it will do
> so with zero return value and, consequently, the test will continue as if
> everything were OK.
> And unless it was 'git update-index' that failed in a way that left a borked
> index behind, the 'git diff-index --cached' below will not error out or
> produce some output that would cause the test to fail. i.e. I tried e.g.
>
> append_cr <$f >tmp && mv -f tmp $f && false &&
>
> in the loop and the test succeeded.
Ugh, you're right. I remembered that for loops were tricky in &&-chains,
but for some reason was thinking that "break" would give you the last
exit code, But I just re-tested, and of course it does not work.
> I think the best fix would be to unroll the loop: after this patch the loop
> body consists of only two significant lines and we iterate through the loop
> only twice, so the test would be even shorter.
Yeah, unrolling may be the best thing here, given the size of the loops.
As a general rule, I think it has to be a subshell with an exit, like:
(
for i in one two three; do
echo $i &&
test $i = one ||
exit 1
done
)
echo exit=$?
which should yield one, two, and exit=1. 7b1732c (t7510: use consistent
&&-chains in loop, 2014-06-16) deals with this in another test.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 01/25] t/test-lib: introduce --chain-lint option
2015-03-25 2:53 ` SZEDER Gábor
@ 2015-03-25 3:05 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 3:05 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Wed, Mar 25, 2015 at 03:53:52AM +0100, SZEDER Gábor wrote:
> > cmd1 &&
> > for i in a b c; do
> > cmd2 $i
> > done &&
> > cmd3
> >
> > which will not notice failures of "cmd2 a" or "cmd b"
>
> s/cmd b/cmd2 b/ ?
Yes, but the patches are already in next, so it is sadly too late for
commit message fixups.
> > - it cannot find a missing &&-chain inside a block or
> > subfunction, like:
> [...]
> And inside subshells. [...]
Yeah, I had mentally filed them with "block", but true subshells are
probably the most common place. However, I'd suspect a good portion of
them are going to be the "trivial" type, especially if they involve
setting up the sub-environment at the top of the subshell. E.g.,
something like this:
cmd1 &&
(
FOO=bar; export FOO
cmd2
) &&
cmd3
does not break the outer chain (which is what --chain-lint checks). It
does break the chain inside the subshell, but we never expect variable
assignment or export to fail (it is nice to fix it, of course, but the
main purpose in fixing the ones in my "trivial" patch was more about
shutting up --chain-lint to make the real breakages more obvious).
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 0/8] more &&-chaining test fixups
2015-03-25 2:56 ` Jeff King
@ 2015-03-25 5:24 ` Jeff King
2015-03-25 5:25 ` [PATCH 1/8] perf-lib: fix ignored exit code inside loop Jeff King
` (8 more replies)
0 siblings, 9 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:24 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
Here's what I found looking for loops like:
for i in a b c; do
something_important $i || break
done &&
something_else
which presumably expect the chain to stop when something_important fails
for any loop element. The solutions are one of (depending on the
surrounding code):
1. Switch the break to "return 1". The tests are all &&-chained, so
the effect of a failed command is to exit the test immediately
anyway. And we wrap our eval'd test snippets inside an extra layer
of function call to explicitly allow early returns like this.
2. Switch the break to "exit 1". Calling "return" from a subshell
inside a function is a bit weird. It doesn't exit the function at
all, but rather just the subshell (in both bash and dash). But if
you are not in a function, calling "return" at all is an error in
bash (subshell or no), and OK in dash (where it acts like "exit").
POSIX explicitly marks the "outside of a function" behavior as
unspecified, but I couldn't find anything about the subshell
behavior.
So I'm loathe to depend on it, even though it does seem to do what
we want, as I do not want to even think what some more obscure
shells might do with it. And especially because we know that "exit
1" portably does what we want (the only downside is that you have
to recognize which situation you are in and use "exit" versus
"return").
3. Unroll the loops. In some cases the result is actually shorter, and
(IMHO) more readable.
These sites were all found with:
git grep -E '(^|\|\|)[ ]*break' t/t*.sh
(that's a space and a tab in the brackets). There are some matches
there that I did not touch, because they were already fine. E.g., t5302
and t7700 use loops to assign a value to a variable and break out early,
and then check the value of the variable.
That's just the tip of the iceberg, though. Searching for
git grep 'for .* in '
yields hundreds of hits. Most of which are probably fine (quite a few
are outside &&-chains entirely). I focused on the ones that called
break, because that indicated to me that the author was trying to
address the &&-chain. Certainly anybody else is welcome to take a stab
at the rest, but I'm also happy to fix them up as we touch nearby code
and notice them. Most of the loops are in setup code that we do not
expect to fail anyway, so examining them is a lot of work for a little
gain.
There were a few legitimate problems, though. I've ordered the patches
below by descending severity. These apply on top of jk/test-chain-lint.
[1/8]: perf-lib: fix ignored exit code inside loop
[2/8]: t0020: fix ignored exit code inside loops
[3/8]: t3305: fix ignored exit code inside loop
[4/8]: t7701: fix ignored exit code inside loop
These four are actual bugs.
[5/8]: t: fix some trivial cases of ignored exit codes in loops
These ones are in setup code, and so would almost certainly never
fail.
[6/8]: t: simplify loop exit-code status variables
[7/8]: t0020: use test_* helpers instead of hand-rolled messages
[8/8]: t9001: drop save_confirm helper
These last three are pure cleanup, no behavior changes. The last two
are not even strictly related to the same topic, but I noticed them
while in the area.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 1/8] perf-lib: fix ignored exit code inside loop
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
@ 2015-03-25 5:25 ` Jeff King
2015-03-25 5:28 ` [PATCH 2/8] t0020: fix ignored exit code inside loops Jeff King
` (7 subsequent siblings)
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:25 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
When copying the test repository, we try to detect whether
the copy succeeded. However, most of the heavy lifting is
done inside a for loop, where our "break" will lose the exit
code of the failing "cp". We can take advantage of the fact
that we are in a subshell, and just "exit 1" to break out
with a code.
Signed-off-by: Jeff King <peff@peff.net>
---
t/perf/perf-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index a8c9574..5cf74ed 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -91,7 +91,7 @@ test_perf_create_repo_from () {
*/objects|*/hooks|*/config)
;;
*)
- cp -R "$stuff" . || break
+ cp -R "$stuff" . || exit 1
;;
esac
done &&
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 2/8] t0020: fix ignored exit code inside loops
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
2015-03-25 5:25 ` [PATCH 1/8] perf-lib: fix ignored exit code inside loop Jeff King
@ 2015-03-25 5:28 ` Jeff King
2015-03-25 5:28 ` [PATCH 3/8] t3305: fix ignored exit code inside loop Jeff King
` (6 subsequent siblings)
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:28 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
A loop like:
for f in one two; do
something $f ||
break
done
will correctly break out of the loop when we see a failure
of one item, but the resulting exit code will always be
zero. We can fix that by putting the loop into a function or
subshell, but in this case it is simpler still to just
unroll the loop. We do add a helper function, which
hopefully makes the end result even more readable (in
addition to being shorter).
Reported-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
---
You can make each call site a one-liner that does the loop inside the
function (including the update-index call). But I tried to shoot for
readability rather than absolute minimum characters.
t/t0020-crlf.sh | 54 +++++++++++++++++++-----------------------------------
1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 9fa26df..144fdcd 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -8,6 +8,13 @@ has_cr() {
tr '\015' Q <"$1" | grep Q >/dev/null
}
+# add or remove CRs to disk file in-place
+# usage: munge_cr <append|remove> <file>
+munge_cr () {
+ "${1}_cr" <"$2" >tmp &&
+ mv tmp "$2"
+}
+
test_expect_success setup '
git config core.autocrlf false &&
@@ -100,14 +107,9 @@ test_expect_success 'update with autocrlf=input' '
rm -f tmp one dir/two three &&
git read-tree --reset -u HEAD &&
git config core.autocrlf input &&
-
- for f in one dir/two
- do
- append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f ||
- break
- done &&
-
+ munge_cr append one &&
+ munge_cr append dir/two &&
+ git update-index -- one dir/two &&
differs=$(git diff-index --cached HEAD) &&
verbose test -z "$differs"
@@ -118,14 +120,9 @@ test_expect_success 'update with autocrlf=true' '
rm -f tmp one dir/two three &&
git read-tree --reset -u HEAD &&
git config core.autocrlf true &&
-
- for f in one dir/two
- do
- append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f ||
- break
- done &&
-
+ munge_cr append one &&
+ munge_cr append dir/two &&
+ git update-index -- one dir/two &&
differs=$(git diff-index --cached HEAD) &&
verbose test -z "$differs"
@@ -136,13 +133,9 @@ test_expect_success 'checkout with autocrlf=true' '
rm -f tmp one dir/two three &&
git config core.autocrlf true &&
git read-tree --reset -u HEAD &&
-
- for f in one dir/two
- do
- remove_cr <"$f" >tmp && mv -f tmp $f &&
- verbose git update-index -- $f ||
- break
- done &&
+ munge_cr remove one &&
+ munge_cr remove dir/two &&
+ git update-index -- one dir/two &&
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
@@ -154,18 +147,9 @@ test_expect_success 'checkout with autocrlf=input' '
rm -f tmp one dir/two three &&
git config core.autocrlf input &&
git read-tree --reset -u HEAD &&
-
- for f in one dir/two
- do
- if has_cr "$f"
- then
- echo "Eh? $f"
- false
- break
- else
- git update-index -- $f
- fi
- done &&
+ test_must_fail has_cr one &&
+ test_must_fail has_cr two &&
+ git update-index -- one dir/two &&
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 3/8] t3305: fix ignored exit code inside loop
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
2015-03-25 5:25 ` [PATCH 1/8] perf-lib: fix ignored exit code inside loop Jeff King
2015-03-25 5:28 ` [PATCH 2/8] t0020: fix ignored exit code inside loops Jeff King
@ 2015-03-25 5:28 ` Jeff King
2015-03-25 8:40 ` Johan Herland
2015-03-25 5:29 ` [PATCH 4/8] t7701: " Jeff King
` (5 subsequent siblings)
8 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:28 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
When we test deleting notes, we run "git notes remove" in a
loop. However, the exit value of the loop will only reflect
the final note we process. We should break out of the loop
with a failing exit code as soon as we see a problem.
Note that we can call "exit 1" here without explicitly
creating a subshell, because the while loop on the
right-hand side of a pipe executes in its own implicit
subshell.
Note also that the "break" above does not suffer the same
problem; it is meant to exit the loop early at a certain
number of iterations. We can bump it into the conditional of
the loop to make this more obvious.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t3305-notes-fanout.sh | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
index b1ea64b..54460be 100755
--- a/t/t3305-notes-fanout.sh
+++ b/t/t3305-notes-fanout.sh
@@ -51,15 +51,12 @@ test_expect_success 'deleting most notes with git-notes' '
num_notes=250 &&
i=0 &&
git rev-list HEAD |
- while read sha1
+ while test $i -lt $num_notes && read sha1
do
i=$(($i + 1)) &&
- if test $i -gt $num_notes
- then
- break
- fi &&
test_tick &&
- git notes remove "$sha1"
+ git notes remove "$sha1" ||
+ exit 1
done
'
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 4/8] t7701: fix ignored exit code inside loop
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (2 preceding siblings ...)
2015-03-25 5:28 ` [PATCH 3/8] t3305: fix ignored exit code inside loop Jeff King
@ 2015-03-25 5:29 ` Jeff King
2015-03-25 5:29 ` [PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops Jeff King
` (4 subsequent siblings)
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:29 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
When checking a list of file mtimes, we use a loop and break
out early from the loop if any entry does not match.
However, the exit code of a loop exited via break is always
0, meaning that the test will fail to notice we had a
mismatch. Since the loop is inside a function, we can fix
this by doing an early "return 1".
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7701-repack-unpack-unreachable.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index aad8a9c..b66e383 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -57,7 +57,7 @@ compare_mtimes ()
{
read tref rest &&
while read t rest; do
- test "$tref" = "$t" || break
+ test "$tref" = "$t" || return 1
done
}
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (3 preceding siblings ...)
2015-03-25 5:29 ` [PATCH 4/8] t7701: " Jeff King
@ 2015-03-25 5:29 ` Jeff King
2015-03-25 5:30 ` [PATCH 6/8] t: simplify loop exit-code status variables Jeff King
` (3 subsequent siblings)
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:29 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
These are all cases where we do a setup step of the form:
for i in $foo; do
set_up $i || break
done &&
more_setup
would not notice a failure in set_up (because break always
returns a 0 exit code). These are just setup steps that we
do not expect to fail, but it does not hurt to be defensive.
Most can be fixed by converting the "break" to a "return 1"
(since we eval our tests inside a function for just this
purpose). A few of the loops are inside subshells, so we can
use just "exit 1" to break out of the subshell. And a few
can actually be made shorter by just unrolling the loop.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t3010-ls-files-killed-modified.sh | 11 ++++-------
t/t3031-merge-criscross.sh | 2 +-
t/t3202-show-branch-octopus.sh | 2 +-
t/t4024-diff-optimize-common.sh | 2 +-
t/t4046-diff-unmerged.sh | 8 ++++----
t/t4151-am-abort.sh | 2 +-
t/t5505-remote.sh | 8 ++++----
t/t5514-fetch-multiple.sh | 4 ++--
t/t6026-merge-attr.sh | 6 +++---
t/t6040-tracking-info.sh | 7 +++----
10 files changed, 24 insertions(+), 28 deletions(-)
diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
index 62fce10..580e158 100755
--- a/t/t3010-ls-files-killed-modified.sh
+++ b/t/t3010-ls-files-killed-modified.sh
@@ -55,13 +55,10 @@ test_expect_success 'git update-index --add to add various paths.' '
: >path9 &&
date >path10 &&
git update-index --add -- path0 path?/file? pathx/ju path7 path8 path9 path10 &&
- for i in 1 2
- do
- git init submod$i &&
- (
- cd submod$i && git commit --allow-empty -m "empty $i"
- ) || break
- done &&
+ git init submod1 &&
+ git -C submod1 commit --allow-empty -m "empty 1" &&
+ git init submod2 &&
+ git -C submod2 commit --allow-empty -m "empty 2" &&
git update-index --add submod[12] &&
(
cd submod1 &&
diff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh
index 7f41607..e59b0a3 100755
--- a/t/t3031-merge-criscross.sh
+++ b/t/t3031-merge-criscross.sh
@@ -32,7 +32,7 @@ test_expect_success 'setup repo with criss-cross history' '
do
echo $n > data/$n &&
n=$(($n+1)) ||
- break
+ return 1
done &&
# check them in
diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
index 0a5d5e6..6adf478 100755
--- a/t/t3202-show-branch-octopus.sh
+++ b/t/t3202-show-branch-octopus.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
> file$i &&
git add file$i &&
test_tick &&
- git commit -m branch$i || break
+ git commit -m branch$i || return 1
done
'
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index c4d733f..7e76018 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -139,7 +139,7 @@ test_expect_success setup '
( printf C; zs $n ) >file-c$n &&
( echo D; zs $n ) >file-d$n &&
- expect_pattern $n || break
+ expect_pattern $n || return 1
done >expect
'
diff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh
index 25d50a6..d0f1447 100755
--- a/t/t4046-diff-unmerged.sh
+++ b/t/t4046-diff-unmerged.sh
@@ -8,7 +8,7 @@ test_expect_success setup '
do
blob=$(echo $i | git hash-object --stdin) &&
eval "blob$i=$blob" &&
- eval "m$i=\"100644 \$blob$i $i\"" || break
+ eval "m$i=\"100644 \$blob$i $i\"" || return 1
done &&
paths= &&
for b in o x
@@ -24,9 +24,9 @@ test_expect_success setup '
case "$b" in x) echo "$m1$p" ;; esac &&
case "$o" in x) echo "$m2$p" ;; esac &&
case "$t" in x) echo "$m3$p" ;; esac ||
- break
- done || break
- done || break
+ return 1
+ done
+ done
done >ls-files-s.expect &&
git update-index --index-info <ls-files-s.expect &&
git ls-files -s >ls-files-s.actual &&
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 1176bcc..8d90634 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -20,7 +20,7 @@ test_expect_success setup '
echo $i >otherfile-$i &&
git add otherfile-$i &&
test_tick &&
- git commit -a -m $i || break
+ git commit -a -m $i || return 1
done &&
git format-patch --no-numbered initial &&
git checkout -b side initial &&
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 17c6330..7a8499c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -579,7 +579,7 @@ test_expect_success 'update with arguments' '
cd one &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git remote add manduca ../mirror &&
git remote add megaloprepus ../mirror &&
@@ -622,7 +622,7 @@ test_expect_success 'update default' '
cd one &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git config remote.drosophila.skipDefaultUpdate true &&
git remote update default &&
@@ -642,7 +642,7 @@ test_expect_success 'update default (overridden, with funny whitespace)' '
cd one &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git config remotes.default "$(printf "\t drosophila \n")" &&
git remote update default &&
@@ -656,7 +656,7 @@ test_expect_success 'update (with remotes.default defined)' '
cd one &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git config remotes.default "drosophila" &&
git remote update &&
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index 0f81409..4b4b667 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -120,7 +120,7 @@ test_expect_success 'git fetch --all (skipFetchAll)' '
(cd test4 &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git remote add three ../three &&
git config remote.three.skipFetchAll true &&
@@ -144,7 +144,7 @@ test_expect_success 'git fetch --multiple (ignoring skipFetchAll)' '
(cd test4 &&
for b in $(git branch -r)
do
- git branch -r -d $b || break
+ git branch -r -d $b || exit 1
done &&
git fetch --multiple one two three &&
git branch -r > output &&
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 5e43997..3c21938 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -11,7 +11,7 @@ test_expect_success setup '
for f in text binary union
do
- echo Initial >$f && git add $f || break
+ echo Initial >$f && git add $f || return 1
done &&
test_tick &&
git commit -m Initial &&
@@ -19,7 +19,7 @@ test_expect_success setup '
git branch side &&
for f in text binary union
do
- echo Master >>$f && git add $f || break
+ echo Master >>$f && git add $f || return 1
done &&
test_tick &&
git commit -m Master &&
@@ -27,7 +27,7 @@ test_expect_success setup '
git checkout side &&
for f in text binary union
do
- echo Side >>$f && git add $f || break
+ echo Side >>$f && git add $f || return 1
done &&
test_tick &&
git commit -m Side &&
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 7ac8fd0..3d5c238 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -12,10 +12,9 @@ advance () {
}
test_expect_success setup '
- for i in a b c;
- do
- advance $i || break
- done &&
+ advance a &&
+ advance b &&
+ advance c &&
git clone . test &&
(
cd test &&
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 6/8] t: simplify loop exit-code status variables
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (4 preceding siblings ...)
2015-03-25 5:29 ` [PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops Jeff King
@ 2015-03-25 5:30 ` Jeff King
2015-03-25 17:27 ` Junio C Hamano
2015-03-25 5:31 ` [PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages Jeff King
` (2 subsequent siblings)
8 siblings, 1 reply; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:30 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
Since shell loops may drop the exit code of failed commands
inside the loop, some tests try to keep track of the status
by setting a variable. This can end up cumbersome and hard
to read; it is much simpler to just exit directly from the
loop using "return 1" (since each case is either in a helper
function or inside a test snippet).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t3060-ls-files-with-tree.sh | 13 ++++---------
t/t3901-i18n-patch.sh | 8 ++------
2 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
index 61c1f53..36b10f7 100755
--- a/t/t3060-ls-files-with-tree.sh
+++ b/t/t3060-ls-files-with-tree.sh
@@ -25,15 +25,10 @@ test_expect_success setup '
do
num=00$n$m &&
>sub/file-$num &&
- echo file-$num >>expected || {
- bad=t
- break
- }
- done && test -z "$bad" || {
- bad=t
- break
- }
- done && test -z "$bad" &&
+ echo file-$num >>expected ||
+ return 1
+ done
+ done &&
git add . &&
git commit -m "add a bunch of files" &&
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index a392f3d..75cf3ff 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -9,7 +9,7 @@ test_description='i18n settings and format-patch | am pipe'
check_encoding () {
# Make sure characters are not corrupted
- cnt="$1" header="$2" i=1 j=0 bad=0
+ cnt="$1" header="$2" i=1 j=0
while test "$i" -le $cnt
do
git format-patch --encoding=UTF-8 --stdout HEAD~$i..HEAD~$j |
@@ -20,14 +20,10 @@ check_encoding () {
grep "^encoding ISO8859-1" ;;
*)
grep "^encoding ISO8859-1"; test "$?" != 0 ;;
- esac || {
- bad=1
- break
- }
+ esac || return 1
j=$i
i=$(($i+1))
done
- (exit $bad)
}
test_expect_success setup '
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (5 preceding siblings ...)
2015-03-25 5:30 ` [PATCH 6/8] t: simplify loop exit-code status variables Jeff King
@ 2015-03-25 5:31 ` Jeff King
2015-03-25 5:32 ` [PATCH 8/8] t9001: drop save_confirm helper Jeff King
2015-03-25 17:29 ` [PATCH 0/8] more &&-chaining test fixups Junio C Hamano
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:31 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
These tests are not wrong, but it is much shorter and more
idiomatic to say "verbose" or "test_must_fail" rather than
printing our own messages on failure. Likewise, there is no
need to say "happy" at the end of a test; the test suite
takes care of that.
Signed-off-by: Jeff King <peff@peff.net>
---
I somehow missed these when doing 9157c5c in the earlier series.
t/t0020-crlf.sh | 38 +++++---------------------------------
1 file changed, 5 insertions(+), 33 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 144fdcd..f94120a 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -35,9 +35,7 @@ test_expect_success setup '
for w in Some extra lines here; do echo $w; done >>one &&
git diff >patch.file &&
patched=$(git hash-object --stdin <one) &&
- git read-tree --reset -u HEAD &&
-
- echo happy.
+ git read-tree --reset -u HEAD
'
test_expect_success 'safecrlf: autocrlf=input, all CRLF' '
@@ -225,29 +223,9 @@ test_expect_success '.gitattributes says two is binary' '
git config core.autocrlf true &&
git read-tree --reset -u HEAD &&
- if has_cr dir/two
- then
- echo "Huh?"
- false
- else
- : happy
- fi &&
-
- if has_cr one
- then
- : happy
- else
- echo "Huh?"
- false
- fi &&
-
- if has_cr three
- then
- echo "Huh?"
- false
- else
- : happy
- fi
+ test_must_fail has_cr dir/two &&
+ verbose has_cr one &&
+ test_must_fail has_cr three
'
test_expect_success '.gitattributes says two is input' '
@@ -256,13 +234,7 @@ test_expect_success '.gitattributes says two is input' '
echo "two crlf=input" >.gitattributes &&
git read-tree --reset -u HEAD &&
- if has_cr dir/two
- then
- echo "Huh?"
- false
- else
- : happy
- fi
+ test_must_fail has_cr dir/two
'
test_expect_success '.gitattributes says two and three are text' '
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 8/8] t9001: drop save_confirm helper
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (6 preceding siblings ...)
2015-03-25 5:31 ` [PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages Jeff King
@ 2015-03-25 5:32 ` Jeff King
2015-03-25 17:29 ` [PATCH 0/8] more &&-chaining test fixups Junio C Hamano
8 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 5:32 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
The idea of this helper is that we want to save the current
value of a config variable and then restore it again after
the test completes. However, there's no point in actually
saving the value; it should always be restored to the string
"never" (which you can confirm by instrumenting
save_confirm to print the value it finds).
Let's just replace it with a single test_when_finished call.
Suggested-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
---
t/t9001-send-email.sh | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c9f54d5..7be14a4 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -817,25 +817,20 @@ test_expect_success $PREREQ '--confirm=compose' '
test_confirm --confirm=compose --compose
'
-save_confirm () {
- CONFIRM=$(git config --get sendemail.confirm) &&
- test_when_finished "git config sendemail.confirm ${CONFIRM:-never}"
-}
-
test_expect_success $PREREQ 'confirm by default (due to cc)' '
- save_confirm &&
+ test_when_finished git config sendemail.confirm never &&
git config --unset sendemail.confirm &&
test_confirm
'
test_expect_success $PREREQ 'confirm by default (due to --compose)' '
- save_confirm &&
+ test_when_finished git config sendemail.confirm never &&
git config --unset sendemail.confirm &&
test_confirm --suppress-cc=all --compose
'
test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
- save_confirm &&
+ test_when_finished git config sendemail.confirm never &&
git config --unset sendemail.confirm &&
rm -fr outdir &&
git format-patch -2 -o outdir &&
@@ -848,7 +843,7 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '
'
test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
- save_confirm &&
+ test_when_finished git config sendemail.confirm never &&
git config sendemail.confirm auto &&
GIT_SEND_EMAIL_NOTTY=1 &&
export GIT_SEND_EMAIL_NOTTY &&
@@ -860,7 +855,7 @@ test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
'
test_expect_success $PREREQ 'confirm does not loop forever' '
- save_confirm &&
+ test_when_finished git config sendemail.confirm never &&
git config sendemail.confirm auto &&
GIT_SEND_EMAIL_NOTTY=1 &&
export GIT_SEND_EMAIL_NOTTY &&
--
2.3.4.635.gd6ffcfe
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 3/8] t3305: fix ignored exit code inside loop
2015-03-25 5:28 ` [PATCH 3/8] t3305: fix ignored exit code inside loop Jeff King
@ 2015-03-25 8:40 ` Johan Herland
0 siblings, 0 replies; 68+ messages in thread
From: Johan Herland @ 2015-03-25 8:40 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, Git mailing list
On Wed, Mar 25, 2015 at 6:28 AM, Jeff King <peff@peff.net> wrote:
> When we test deleting notes, we run "git notes remove" in a
> loop. However, the exit value of the loop will only reflect
> the final note we process. We should break out of the loop
> with a failing exit code as soon as we see a problem.
>
> Note that we can call "exit 1" here without explicitly
> creating a subshell, because the while loop on the
> right-hand side of a pipe executes in its own implicit
> subshell.
>
> Note also that the "break" above does not suffer the same
> problem; it is meant to exit the loop early at a certain
> number of iterations. We can bump it into the conditional of
> the loop to make this more obvious.
>
> Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johan Herland <johan@herland.net>
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 6/8] t: simplify loop exit-code status variables
2015-03-25 5:30 ` [PATCH 6/8] t: simplify loop exit-code status variables Jeff King
@ 2015-03-25 17:27 ` Junio C Hamano
2015-03-25 17:43 ` Jeff King
0 siblings, 1 reply; 68+ messages in thread
From: Junio C Hamano @ 2015-03-25 17:27 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, git
Jeff King <peff@peff.net> writes:
> Since shell loops may drop the exit code of failed commands
> inside the loop, some tests try to keep track of the status
> by setting a variable. This can end up cumbersome and hard
> to read; it is much simpler to just exit directly from the
> loop using "return 1" (since each case is either in a helper
> function or inside a test snippet).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t3060-ls-files-with-tree.sh | 13 ++++---------
> t/t3901-i18n-patch.sh | 8 ++------
> 2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
> index 61c1f53..36b10f7 100755
> --- a/t/t3060-ls-files-with-tree.sh
> +++ b/t/t3060-ls-files-with-tree.sh
> @@ -25,15 +25,10 @@ test_expect_success setup '
> do
> num=00$n$m &&
> >sub/file-$num &&
> - echo file-$num >>expected || {
> - bad=t
> - break
> - }
> - done && test -z "$bad" || {
> - bad=t
> - break
> - }
> - done && test -z "$bad" &&
> + echo file-$num >>expected ||
> + return 1
> + done
> + done &&
> git add . &&
> git commit -m "add a bunch of files" &&
The empty initialization for $bad can also go for this one, right?
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 0/8] more &&-chaining test fixups
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
` (7 preceding siblings ...)
2015-03-25 5:32 ` [PATCH 8/8] t9001: drop save_confirm helper Jeff King
@ 2015-03-25 17:29 ` Junio C Hamano
8 siblings, 0 replies; 68+ messages in thread
From: Junio C Hamano @ 2015-03-25 17:29 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, git
Jeff King <peff@peff.net> writes:
> There were a few legitimate problems, though. I've ordered the patches
> below by descending severity. These apply on top of jk/test-chain-lint.
>
> [1/8]: perf-lib: fix ignored exit code inside loop
> [2/8]: t0020: fix ignored exit code inside loops
> [3/8]: t3305: fix ignored exit code inside loop
> [4/8]: t7701: fix ignored exit code inside loop
>
> These four are actual bugs.
>
> [5/8]: t: fix some trivial cases of ignored exit codes in loops
>
> These ones are in setup code, and so would almost certainly never
> fail.
>
> [6/8]: t: simplify loop exit-code status variables
> [7/8]: t0020: use test_* helpers instead of hand-rolled messages
> [8/8]: t9001: drop save_confirm helper
>
> These last three are pure cleanup, no behavior changes. The last two
> are not even strictly related to the same topic, but I noticed them
> while in the area.
Thanks. All looked sensible.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 6/8] t: simplify loop exit-code status variables
2015-03-25 17:27 ` Junio C Hamano
@ 2015-03-25 17:43 ` Jeff King
0 siblings, 0 replies; 68+ messages in thread
From: Jeff King @ 2015-03-25 17:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: SZEDER Gábor, git
On Wed, Mar 25, 2015 at 10:27:49AM -0700, Junio C Hamano wrote:
> > diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh
> > index 61c1f53..36b10f7 100755
> > --- a/t/t3060-ls-files-with-tree.sh
> > +++ b/t/t3060-ls-files-with-tree.sh
> > @@ -25,15 +25,10 @@ test_expect_success setup '
> > do
> > num=00$n$m &&
> > >sub/file-$num &&
> > - echo file-$num >>expected || {
> > - bad=t
> > - break
> > - }
> > - done && test -z "$bad" || {
> > - bad=t
> > - break
> > - }
> > - done && test -z "$bad" &&
> > + echo file-$num >>expected ||
> > + return 1
> > + done
> > + done &&
> > git add . &&
> > git commit -m "add a bunch of files" &&
>
> The empty initialization for $bad can also go for this one, right?
Yeah, it can.
-Peff
^ permalink raw reply [flat|nested] 68+ messages in thread
end of thread, other threads:[~2015-03-25 17:43 UTC | newest]
Thread overview: 68+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
2015-03-25 2:53 ` SZEDER Gábor
2015-03-25 3:05 ` Jeff King
2015-03-20 10:06 ` [PATCH 02/25] t: fix severe &&-chain breakage Jeff King
2015-03-20 10:06 ` [PATCH 03/25] t: fix moderate " Jeff King
2015-03-20 10:07 ` [PATCH 04/25] t: fix trivial " Jeff King
2015-03-20 10:07 ` [PATCH 05/25] t: assume test_cmp produces verbose output Jeff King
2015-03-20 10:09 ` [PATCH 06/25] t: use verbose instead of hand-rolled errors Jeff King
2015-03-20 10:09 ` [PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks Jeff King
2015-03-20 10:10 ` [PATCH 08/25] t: fix &&-chaining issues around setup which might fail Jeff King
2015-03-20 10:11 ` [PATCH 09/25] t: use test_might_fail for diff and grep Jeff King
2015-03-20 10:11 ` [PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison Jeff King
2015-03-20 10:12 ` [PATCH 11/25] t: wrap complicated expect_code users in a block Jeff King
2015-03-20 10:12 ` [PATCH 12/25] t: avoid using ":" for comments Jeff King
2015-03-20 10:12 ` [PATCH 13/25] t3600: fix &&-chain breakage for setup commands Jeff King
2015-03-20 10:12 ` [PATCH 14/25] t7201: fix &&-chain breakage Jeff King
2015-03-20 10:13 ` [PATCH 15/25] t9502: " Jeff King
2015-03-20 17:48 ` Johannes Sixt
2015-03-20 18:03 ` Jeff King
2015-03-20 10:13 ` [PATCH 16/25] t6030: use modern test_* helpers Jeff King
2015-03-20 10:13 ` [PATCH 17/25] t0020: " Jeff King
2015-03-25 0:23 ` SZEDER Gábor
2015-03-25 2:56 ` Jeff King
2015-03-25 5:24 ` [PATCH 0/8] more &&-chaining test fixups Jeff King
2015-03-25 5:25 ` [PATCH 1/8] perf-lib: fix ignored exit code inside loop Jeff King
2015-03-25 5:28 ` [PATCH 2/8] t0020: fix ignored exit code inside loops Jeff King
2015-03-25 5:28 ` [PATCH 3/8] t3305: fix ignored exit code inside loop Jeff King
2015-03-25 8:40 ` Johan Herland
2015-03-25 5:29 ` [PATCH 4/8] t7701: " Jeff King
2015-03-25 5:29 ` [PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops Jeff King
2015-03-25 5:30 ` [PATCH 6/8] t: simplify loop exit-code status variables Jeff King
2015-03-25 17:27 ` Junio C Hamano
2015-03-25 17:43 ` Jeff King
2015-03-25 5:31 ` [PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages Jeff King
2015-03-25 5:32 ` [PATCH 8/8] t9001: drop save_confirm helper Jeff King
2015-03-25 17:29 ` [PATCH 0/8] more &&-chaining test fixups Junio C Hamano
2015-03-20 10:13 ` [PATCH 18/25] t1301: use modern test_* helpers Jeff King
2015-03-24 23:51 ` SZEDER Gábor
2015-03-25 2:45 ` Jeff King
2015-03-20 10:13 ` [PATCH 19/25] t6034: " Jeff King
2015-03-24 23:43 ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 20/25] t4117: " Jeff King
2015-03-20 10:13 ` [PATCH 21/25] t9001: use test_when_finished Jeff King
2015-03-25 2:00 ` SZEDER Gábor
2015-03-25 2:47 ` Jeff King
2015-03-20 10:13 ` [PATCH 22/25] t0050: appease --chain-lint Jeff King
2015-03-20 10:13 ` [PATCH 23/25] t7004: fix embedded single-quotes Jeff King
2015-03-20 10:13 ` [PATCH 24/25] t0005: fix broken &&-chains Jeff King
2015-03-20 10:13 ` [PATCH 25/25] t4104: drop hand-rolled error reporting Jeff King
2015-03-20 10:23 ` [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 14:28 ` Michael J Gruber
2015-03-20 14:32 ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
2015-03-20 14:32 ` [PATCH 27/27] t9104: fix test for following larger parents Michael J Gruber
2015-03-20 18:04 ` [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage Junio C Hamano
2015-03-20 19:35 ` Junio C Hamano
2015-03-20 20:02 ` Jeff King
2015-03-20 20:13 ` Jeff King
2015-03-23 9:36 ` Michael J Gruber
2015-03-20 17:57 ` [PATCH 0/25] detecting " Jeff King
2015-03-20 17:44 ` Junio C Hamano
2015-03-20 18:00 ` Junio C Hamano
2015-03-20 18:04 ` Jeff King
2015-03-20 18:33 ` Junio C Hamano
2015-03-20 23:18 ` Eric Sunshine
2015-03-21 8:19 ` Jeff King
2015-03-21 18:01 ` Junio C Hamano
2015-03-21 22:23 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).