{"thread":{"id":"38863","subject":"[PATCH 0/25] detecting &&-chain breakage","startedAt":"2015-03-20T10:04:30Z","lastAt":"2015-03-25T17:43:19Z","messageCount":68,"participants":["Jeff King","Michael J Gruber","Junio C Hamano","Johannes Sixt","Eric Sunshine","SZEDER Gábor","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":25},"messages":[{"id":"258097","messageId":"20150320100429.GA17354@peff.net","threadId":"38863","inReplyTo":null,"subject":"[PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:04:30Z","receivedAt":"2015-03-20T10:04:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is a cleanup of the &&-chain lint I posted earlier:\n\n http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859\n\nI don't know who came up with the idea for it originally, but the\nconcept certainly was floating in the back of my mind from Jonathan's\nearlier version that is referenced in that thread.\n\nThe general idea is to detect &&-chain breakage that can lead to our\ntests yielding false success. The first patch implements and discusses\nthe lint-check itself, which is quite simple. The bulk of the work was\nfixing all of the issues in the existing tests. :)\n\nThat didn't all need to happen immediately. I mainly wanted to start on\nit to answer two questions:\n\n 1. Are &&-chain breakages actually preventing us from seeing any test\n failures? Or is it mostly just pedantry, and we miss out only on\n knowing whether \"cat >expect <<-\\EOF\" failed (which presumably it\n never does).\n\n 2. How bad are the false positives? Both how common, and how bad to\n work around.\n\nBut after a few hours, I reached a zen state and just kept going. So at\nthe end of this series, the whole test suite is --chain-lint clean\n(modulo any tests that are skipped on my platform). We could even switch\nthe checks on by default at the end of the series, but I didn't do that\nhere. I think it would be sane to run them all the time, though; in the\nnormal success case, they don't add any forks (the shell just runs\n\"(exit) && ...\", and realizes that the whole thing is one big &&-chain).\nI couldn't measure any time difference running the suite with and\nwithout it.\n\nAnyway, to answer the questions: Yes, there were definitely tests whose\nvalues were being thrown away, and we would not have noticed if they\nfailed. The good news is that all of them did pass once we started\nchecking their results. Hooray.\n\nThere were a number of false positives, though as a percentage of the\ntest suite, probably not many (it's just that we have quite a lot of\ntests). Most of them were in rather old tests, and IMHO the fixes I did\nactually improved the readability of the result. So overall I think this\nis a very positive change; I doubt it will get in people's way very\noften, and I look forward to having one less thing to worry about\nhandling manually in review. The biggest downside is that I may have\nautomated Eric Sunshine out of a job. :)\n\nThe patches are:\n\n [01/25]: t/test-lib: introduce --chain-lint option\n [02/25]: t: fix severe &&-chain breakage\n [03/25]: t: fix moderate &&-chain breakage\n [04/25]: t: fix trivial &&-chain breakage\n [05/25]: t: assume test_cmp produces verbose output\n [06/25]: t: use verbose instead of hand-rolled errors\n [07/25]: t: use test_must_fail instead of hand-rolled blocks\n [08/25]: t: fix &&-chaining issues around setup which might fail\n [09/25]: t: use test_might_fail for diff and grep\n [10/25]: t: use test_expect_code instead of hand-rolled comparison\n [11/25]: t: wrap complicated expect_code users in a block\n [12/25]: t: avoid using \":\" for comments\n [13/25]: t3600: fix &&-chain breakage for setup commands\n [14/25]: t7201: fix &&-chain breakage\n [15/25]: t9502: fix &&-chain breakage\n [16/25]: t6030: use modern test_* helpers\n [17/25]: t0020: use modern test_* helpers\n [18/25]: t1301: use modern test_* helpers\n [19/25]: t6034: use modern test_* helpers\n [20/25]: t4117: use modern test_* helpers\n [21/25]: t9001: use test_when_finished\n [22/25]: t0050: appease --chain-lint\n [23/25]: t7004: fix embedded single-quotes\n [24/25]: t0005: fix broken &&-chains\n [25/25]: t4104: drop hand-rolled error reporting\n\nIt's a lot of patches, and a few of them are long. I tried to group\nthem by functionality rather than file (though as you can see, some of\nthe tests were unique enough snowflakes that it made sense to discuss\ntheir issues separately). I'm hoping it should be an easy review, if\nnot a short one.\n\nI'll spare you the full per-file diffstat, but the total is a very\nsatisfying:\n\n 84 files changed, 397 insertions(+), 647 deletions(-)\n\nThat's a lot of changes in a lot of hunks, but most of them are in files\nthat haven't been touched in a while. The series merges cleanly with\n\"pu\", so I don't think I've stepped on anyone's topics in flight.\n\n-Peff\n"},{"id":"258098","messageId":"20150320100548.GA12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 01/25] t/test-lib: introduce --chain-lint option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:05:48Z","receivedAt":"2015-03-20T10:05:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's easy to miss an \"&&\"-chain in a test script, like:\n\n test_expect_success 'check something important' '\n\tcmd1 &&\n\tcmd2\n\tcmd3\n '\n\nThe test harness will notice if cmd3 fails, but a failure of\ncmd1 or cmd2 will go unnoticed, as their exit status is lost\nafter cmd3 runs.\n\nThe toy example above is easy to spot because the \"cmds\" are\nall the same length, but real code is much more complicated.\nIt's also difficult to detect these situations by statically\nanalyzing the shell code with regexps (like the\ncheck-non-portable-shell script does); there's too much\ncontext required to know whether a &&-chain is appropriate\non a given line or not.\n\nThis patch instead lets the shell check each test by\nsticking a command with a specific and unusual return code\nat the top of each test, like:\n\n (exit 117) &&\n cmd1 &&\n cmd2\n cmd3\n\nIn a well-formed test, the non-zero exit from the first\ncommand prevents any of the rest from being run, and the\ntest's exit code is 117. In a bad test (like the one above),\nthe 117 is lost, and cmd3 is run.\n\nWhen we encounter a failure of this check, we abort the test\nscript entirely. For one thing, we have no clue which subset\nof the commands in the test snippet were actually run.\nRunning further tests would be pointless, because we're now\nin an unknown state. And two, this is not a \"test failure\"\nin the traditional sense. The test script is buggy, not the\ncode it is testing. We should be able to fix these problems\nin the script once, and not have them come back later as a\nregression in git's code.\n\nAfter checking a test snippet for --chain-lint, we do still\nrun the test itself. We could actually have a pure-lint\nmode which just checks each test, but there are a few\nreasons not to. One, because the tests are executing\narbitrary code, which could impact the later environment\n(e.g., that could impact which set of tests we run at all).\nAnd two, because a pure-lint mode would still be expensive\nto run, because a significant amount of code runs outside of\nthe test_expect_* blocks. Instead, this option is designed\nto be used as part of a normal test suite run, where it adds\nvery little overhead.\n\nTurning on this option detects quite a few problems in\nexisting tests, which will be fixed in subsequent patches.\nHowever, there are a number of places it cannot reach:\n\n - it cannot find a failure to break out of loops on error,\n like:\n\n cmd1 &&\n for i in a b c; do\n\t cmd2 $i\n done &&\n cmd3\n\n which will not notice failures of \"cmd2 a\" or \"cmd b\"\n\n - it cannot find a missing &&-chain inside a block or\n subfunction, like:\n\n foo () {\n\t cmd1\n\t cmd2\n }\n\n foo &&\n bar\n\n which will not notice a failure of cmd1.\n\n - it only checks tests that you run; every platform will\n have some tests skipped due to missing prequisites,\n so it's impossible to say from one run that the test\n suite is free of broken &&-chains. However, all tests get\n run by _somebody_, so eventually we will notice problems.\n\n - it does not operate on test_when_finished or prerequisite\n blocks. It could, but these tends to be much shorter and\n less of a problem, so I punted on them in this patch.\n\nThis patch was inspired by an earlier patch by Jonathan\nNieder:\n\n http://article.gmane.org/gmane.comp.version-control.git/235913\n\nThis implementation and all bugs are mine.\n\nSigned-off-by: Jeff King \n---\n t/README | 10 ++++++++++\n t/test-lib.sh | 16 ++++++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex d5bb0c9..35438bc 100644\n--- a/t/README\n+++ b/t/README\n@@ -168,6 +168,16 @@ appropriately before running \"make\".\n \tUsing this option with a RAM-based filesystem (such as tmpfs)\n \tcan massively speed up the test suite.\n \n+--chain-lint::\n+--no-chain-lint::\n+\tIf --chain-lint is enabled, the test harness will check each\n+\ttest to make sure that it properly \"&&-chains\" all commands (so\n+\tthat a failure in the middle does not go unnoticed by the final\n+\texit code of the test). This check is performed in addition to\n+\trunning the tests themselves. You may also enable or disable\n+\tthis feature by setting the GIT_TEST_CHAIN_LINT environment\n+\tvariable to \"1\" or \"0\", respectively.\n+\n You can also set the GIT_TEST_INSTALLED environment variable to\n the bindir of an existing git installation to test that installation.\n You still need to have built this git sandbox, from which various\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex c096778..50b3d3f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -232,6 +232,12 @@ do\n \t--root=*)\n \t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n \t\tshift ;;\n+\t--chain-lint)\n+\t\tGIT_TEST_CHAIN_LINT=1\n+\t\tshift ;;\n+\t--no-chain-lint)\n+\t\tGIT_TEST_CHAIN_LINT=0\n+\t\tshift ;;\n \t-x)\n \t\ttrace=t\n \t\tverbose=t\n@@ -524,6 +530,16 @@ test_eval_ () {\n test_run_ () {\n \ttest_cleanup=:\n \texpecting_failure=$2\n+\n+\tif test \"${GIT_TEST_CHAIN_LINT:-0}\" != 0; then\n+\t\t# 117 is magic because it is unlikely to match the exit\n+\t\t# code of other programs\n+\t\ttest_eval_ \"(exit 117) && $1\"\n+\t\tif test \"$?\" != 117; then\n+\t\t\terror \"bug in the test script: broken &&-chain: $1\"\n+\t\tfi\n+\tfi\n+\n \tsetup_malloc_check\n \ttest_eval_ \"$1\"\n \teval_ret=$?\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258099","messageId":"20150320100614.GB12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 02/25] t: fix severe &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:06:15Z","receivedAt":"2015-03-20T10:06:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are tests which are missing a link in their &&-chain,\nin a location which causes a significant portion of the test\nto be missed (e.g., the test effectively does nothing, or\nconsists of a long string of actions and output comparisons,\nand we throw away the exit code of at least one part of the\nstring).\n\nSigned-off-by: Jeff King \n---\n t/t1300-repo-config.sh | 2 +-\n t/t1403-show-ref.sh | 10 +++++-----\n t/t1700-split-index.sh | 2 +-\n t/t3600-rm.sh | 2 +-\n t/t4047-diff-dirstat.sh | 2 +-\n t/t4104-apply-boundary.sh | 4 ++--\n t/t4202-log.sh | 2 +-\n t/t5100-mailinfo.sh | 2 +-\n t/t5510-fetch.sh | 2 +-\n t/t5540-http-push-webdav.sh | 2 +-\n t/t5551-http-fetch-smart.sh | 2 +-\n t/t6036-recursive-corner-cases.sh | 2 +-\n t/t9300-fast-import.sh | 2 +-\n t/t9902-completion.sh | 2 +-\n 14 files changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 938fc8b..bc0b392 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -677,7 +677,7 @@ test_expect_success 'invalid unit' '\n \techo 1auto >expect &&\n \tgit config aninvalid.unit >actual &&\n \ttest_cmp expect actual &&\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in .git/config: invalid unit\n \tEOF\n \ttest_must_fail git config --int --get aninvalid.unit 2>actual &&\ndiff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh\nindex 3e500ed..7e10bcf 100755\n--- a/t/t1403-show-ref.sh\n+++ b/t/t1403-show-ref.sh\n@@ -28,7 +28,7 @@ test_expect_success 'show-ref' '\n \n \t>expect &&\n \n-\ttest_must_fail git show-ref D >actual\n+\ttest_must_fail git show-ref D >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -62,7 +62,7 @@ test_expect_success 'show-ref --verify' '\n \ttest_must_fail git show-ref --verify tags/A >actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_must_fail git show-ref --verify D >actual\n+\ttest_must_fail git show-ref --verify D >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -78,7 +78,7 @@ test_expect_success 'show-ref --verify -q' '\n \ttest_must_fail git show-ref --verify -q tags/A >actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_must_fail git show-ref --verify -q D >actual\n+\ttest_must_fail git show-ref --verify -q D >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -105,10 +105,10 @@ test_expect_success 'show-ref -d' '\n \ttest_cmp expect actual &&\n \n \tgit show-ref -d refs/heads/master >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n \n \tgit show-ref -d --verify refs/heads/master >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n \n \t>expect &&\n \ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 94fb473..a55f5bc 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -182,7 +182,7 @@ test_expect_success 'unify index, two files remain' '\n 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tone\n 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\ttwo\n EOF\n-\ttest_cmp ls-files.expect ls-files.actual\n+\ttest_cmp ls-files.expect ls-files.actual &&\n \n \ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n \tcat >expect <actual_diff_dirstat_M &&\n \ttest_cmp expect_diff_dirstat_M actual_diff_dirstat_M &&\n \tgit diff --dirstat=files,10,cumulative,changes,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC &&\n-\ttest_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC\n+\ttest_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC &&\n \tgit diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 HEAD^..HEAD >actual_diff_dirstat &&\n \ttest_cmp expect_diff_dirstat actual_diff_dirstat &&\n \tgit diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 -M HEAD^..HEAD >actual_diff_dirstat_M &&\ndiff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh\nindex c617c2a..c97aad1 100755\n--- a/t/t4104-apply-boundary.sh\n+++ b/t/t4104-apply-boundary.sh\n@@ -78,8 +78,8 @@ test_expect_success setup '\n \t\techo $i\n \tdone >victim &&\n \tcat victim >del-z-expect &&\n-\tgit diff victim >del-z-patch.with\n-\tgit diff --unified=0 >del-z-patch.without &&\n+\tgit diff victim >del-z-patch.with &&\n+\tgit diff --unified=0 >del-z-patch.without\n \n \t: done\n '\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 5f2b290..a22ac7c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -481,7 +481,7 @@ test_expect_success 'log.decorate configuration' '\n \tgit log --oneline --no-decorate >actual &&\n \ttest_cmp expect.none actual &&\n \tgit log --oneline --decorate >actual &&\n-\ttest_cmp expect.short actual\n+\ttest_cmp expect.short actual &&\n \n \ttest_unconfig log.decorate &&\n \tgit log --pretty=raw >expect.raw &&\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex 60df10f..e97cfb2 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -34,7 +34,7 @@ do\n \t\tif test -f \"$TEST_DIRECTORY\"/t5100/msg$mail--no-inbody-headers\n \t\tthen\n \t\t\tcheck_mailinfo $mail --no-inbody-headers\n-\t\tfi\n+\t\tfi &&\n \t\tif test -f \"$TEST_DIRECTORY\"/t5100/msg$mail--message-id\n \t\tthen\n \t\t\tcheck_mailinfo $mail --message-id\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d78f320..d3fa2c9 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -124,7 +124,7 @@ test_expect_success 'fetch --prune handles overlapping refspecs' '\n \tgit rev-parse origin/master &&\n \tgit rev-parse origin/pr/42 &&\n \n-\tgit config --unset-all remote.origin.fetch\n+\tgit config --unset-all remote.origin.fetch &&\n \tgit config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&\n \tgit config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&\n \ndiff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh\nindex 8d7b3c5..88ff5a4 100755\n--- a/t/t5540-http-push-webdav.sh\n+++ b/t/t5540-http-push-webdav.sh\n@@ -169,7 +169,7 @@ test_expect_failure 'push to password-protected repository (no user in URL)' '\n \ttest_commit pw-nouser &&\n \tset_askpass user@host pass@host &&\n \tgit push \"$HTTPD_URL/auth/dumb/test_repo.git\" HEAD &&\n-\texpect_askpass both user@host\n+\texpect_askpass both user@host &&\n \tgit rev-parse --verify HEAD >expect &&\n \tgit --git-dir=\"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git\" \\\n \t\trev-parse --verify HEAD >actual &&\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 6cbc12d..eae20d9 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -169,7 +169,7 @@ test_expect_success 'GIT_SMART_HTTP can disable smart http' '\n '\n \n test_expect_success 'invalid Content-Type rejected' '\n-\ttest_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual\n+\ttest_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual &&\n \tgrep \"not valid:\" actual\n '\n \ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex a86087b..b43d031 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -533,7 +533,7 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '\n \n \ttest $(git rev-parse :3:a) = $(git rev-parse B:a) &&\n \ttest $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) &&\n-\ttest $(git rev-parse :1:a/file) = $(git rev-parse C:a/file)\n+\ttest $(git rev-parse :1:a/file) = $(git rev-parse C:a/file) &&\n \ttest $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) &&\n \n \ttest -f a~D^0\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex c538e0a..6bd5a00 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2228,7 +2228,7 @@ test_expect_success 'R: feature import-marks-if-exists' '\n \t>expect &&\n \n \tgit fast-import --import-marks-if-exists=not_io.marks \\\n-\t\t\t--export-marks=io.marks <<-\\EOF\n+\t\t\t--export-marks=io.marks <<-\\EOF &&\n \tfeature import-marks-if-exists=io.marks\n \tEOF\n \ttest_cmp expect io.marks\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 7a883d1..4a14a58 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -568,7 +568,7 @@ test_expect_success 'complete files' '\n \ttest_completion \"git commit \" \"modified\" &&\n \n \t: TODO .gitignore should not be here &&\n-\ttest_completion \"git ls-files \" <<-\\EOF\n+\ttest_completion \"git ls-files \" <<-\\EOF &&\n \t.gitignore\n \tdir\n \tmodified\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258100","messageId":"20150320100644.GC12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 03/25] t: fix moderate &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:06:44Z","receivedAt":"2015-03-20T10:06:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are tests which are missing a link in their &&-chain,\nbut in a way that probably does not effect the outcome of\nthe test. Most of these are of the form:\n\n some_cmd >actual\n test_cmp expect actual\n\nThe main point of the test is to verify the output, and a\nfailure in some_cmd would probably be noticed by bogus\noutput. But it is good for the tests to also confirm that\n\"some_cmd\" does not die unexpectedly after producing its\noutput.\n\nSigned-off-by: Jeff King \n---\n t/t0062-revision-walking.sh | 2 +-\n t/t0201-gettext-fallbacks.sh | 4 ++--\n t/t1300-repo-config.sh | 2 +-\n t/t3001-ls-files-others-exclude.sh | 2 +-\n t/t3010-ls-files-killed-modified.sh | 4 ++--\n t/t3404-rebase-interactive.sh | 2 +-\n t/t4041-diff-submodule-option.sh | 2 +-\n t/t4052-stat-output.sh | 34 +++++++++++++++++-----------------\n t/t4212-log-corrupt.sh | 2 +-\n t/t5551-http-fetch-smart.sh | 2 +-\n t/t5709-clone-refspec.sh | 2 +-\n t/t6028-merge-up-to-date.sh | 2 +-\n t/t6132-pathspec-exclude.sh | 28 ++++++++++++++--------------\n 13 files changed, 44 insertions(+), 44 deletions(-)\n\ndiff --git a/t/t0062-revision-walking.sh b/t/t0062-revision-walking.sh\nindex 3d98eb8..113c728 100755\n--- a/t/t0062-revision-walking.sh\n+++ b/t/t0062-revision-walking.sh\n@@ -26,7 +26,7 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'revision walking can be done twice' '\n-\ttest-revision-walking run-twice > run_twice_actual\n+\ttest-revision-walking run-twice >run_twice_actual &&\n \ttest_cmp run_twice_expected run_twice_actual\n '\n \ndiff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh\nindex 5d80a98..00c6d3d 100755\n--- a/t/t0201-gettext-fallbacks.sh\n+++ b/t/t0201-gettext-fallbacks.sh\n@@ -52,7 +52,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v\n cmdline=\"git am\" &&\n export cmdline;\n printf \"When you have resolved this problem, run git am --resolved.\" >expect &&\n- eval_gettext \"When you have resolved this problem, run \\$cmdline --resolved.\" >actual\n+ eval_gettext \"When you have resolved this problem, run \\$cmdline --resolved.\" >actual &&\n test_i18ncmp expect actual\n '\n \n@@ -60,7 +60,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v\n cmdline=\"git am\" &&\n export cmdline;\n printf \"When you have resolved this problem, run \\\"git am --resolved\\\".\" >expect &&\n- eval_gettext \"When you have resolved this problem, run \\\"\\$cmdline --resolved\\\".\" >actual\n+ eval_gettext \"When you have resolved this problem, run \\\"\\$cmdline --resolved\\\".\" >actual &&\n test_i18ncmp expect actual\n '\n \ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex bc0b392..66dd286 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1165,7 +1165,7 @@ test_expect_failure 'adding a key into an empty section reuses header' '\n \tQkey = value\n \tEOF\n \n-\tgit config section.key value\n+\tgit config section.key value &&\n \ttest_cmp expect .git/config\n '\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex b2798fe..3fc484e 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -294,7 +294,7 @@ one/a.1\n one/two/a.1\n three/a.1\n EOF\n-\tgit ls-files -o -i --exclude \"**/a.1\" >actual\n+\tgit ls-files -o -i --exclude \"**/a.1\" >actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 6d3b828..ea6a678 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -99,12 +99,12 @@ test_expect_success 'git ls-files -k to show killed files.' '\n '\n \n test_expect_success 'git ls-files -k output (w/o icase)' '\n-\tgit ls-files -k >.output\n+\tgit ls-files -k >.output &&\n \ttest_cmp .expected .output\n '\n \n test_expect_success 'git ls-files -k output (w/ icase)' '\n-\tgit -c core.ignorecase=true ls-files -k >.output\n+\tgit -c core.ignorecase=true ls-files -k >.output &&\n \ttest_cmp .expected .output\n '\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 034eb35..edad20d 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -950,7 +950,7 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '\n \tset_fake_editor &&\n \tFAKE_LINES=\"edit 1 2 3\" git rebase -i HEAD~3 &&\n \tFAKE_LINES=\"2 1\" git rebase --edit-todo &&\n-\tgit rebase --continue\n+\tgit rebase --continue &&\n \ttest M = $(git cat-file commit HEAD^ | sed -ne \\$p) &&\n \ttest L = $(git cat-file commit HEAD | sed -ne \\$p)\n '\ndiff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\nindex e432896..ff61341 100755\n--- a/t/t4041-diff-submodule-option.sh\n+++ b/t/t4041-diff-submodule-option.sh\n@@ -539,7 +539,7 @@ test_expect_success 'diff --submodule with objects referenced by alternates' '\n \t\t\tgit checkout origin/master\n \t\t) &&\n \t\tgit diff --submodule > ../actual\n-\t)\n+\t) &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex b68afef..4a02b17 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -94,7 +94,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output\n+\t\tCOLUMNS=200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -102,7 +102,7 @@ do\n \ttest \"$cmd\" != diff || continue\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output\n+\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -122,7 +122,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args >output\n+\t\tCOLUMNS=40 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -130,7 +130,7 @@ do\n \ttest \"$cmd\" != diff || continue\n \n \ttest_expect_success \"$cmd --graph $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args --graph >output\n+\t\tCOLUMNS=40 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -150,7 +150,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb statGraphWidth config\" '\n-\t\tgit -c diff.statGraphWidth=26 $cmd $args >output\n+\t\tgit -c diff.statGraphWidth=26 $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -158,7 +158,7 @@ do\n \ttest \"$cmd\" != diff || continue\n \n \ttest_expect_success \"$cmd --graph $verb statGraphWidth config\" '\n-\t\tgit -c diff.statGraphWidth=26 $cmd $args --graph >output\n+\t\tgit -c diff.statGraphWidth=26 $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -179,19 +179,19 @@ EOF\n while read cmd args\n do\n \ttest_expect_success \"$cmd --stat=width with big change\" '\n-\t\tgit $cmd $args --stat=40 >output\n+\t\tgit $cmd $args --stat=40 >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp expect actual\n \t'\n \n \ttest_expect_success \"$cmd --stat-width=width with big change\" '\n-\t\tgit $cmd $args --stat-width=40 >output\n+\t\tgit $cmd $args --stat-width=40 >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp expect actual\n \t'\n \n \ttest_expect_success \"$cmd --stat-graph-width with big change\" '\n-\t\tgit $cmd $args --stat-graph-width=26 >output\n+\t\tgit $cmd $args --stat-graph-width=26 >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp expect actual\n \t'\n@@ -199,13 +199,13 @@ do\n \ttest \"$cmd\" != diff || continue\n \n \ttest_expect_success \"$cmd --stat-width=width --graph with big change\" '\n-\t\tgit $cmd $args --stat-width=40 --graph >output\n+\t\tgit $cmd $args --stat-width=40 --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp expect-graph actual\n \t'\n \n \ttest_expect_success \"$cmd --stat-graph-width --graph with big change\" '\n-\t\tgit $cmd $args --stat-graph-width=26 --graph >output\n+\t\tgit $cmd $args --stat-graph-width=26 --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp expect-graph actual\n \t'\n@@ -265,7 +265,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output\n+\t\tCOLUMNS=200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -273,7 +273,7 @@ do\n \ttest \"$cmd\" != diff || continue\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output\n+\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -294,7 +294,7 @@ while read verb expect cmd args\n do\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args >output\n+\t\tCOLUMNS=1 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -303,7 +303,7 @@ do\n \n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd --graph $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args --graph >output\n+\t\tCOLUMNS=1 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -320,7 +320,7 @@ EOF\n test_expect_success 'merge --stat respects COLUMNS (big change)' '\n \tgit checkout -b branch HEAD^^ &&\n \tCOLUMNS=100 git merge --stat --no-ff master^ >output &&\n-\tgrep \" | \" output >actual\n+\tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -329,7 +329,7 @@ cat >expect <<'EOF'\n EOF\n test_expect_success 'merge --stat respects COLUMNS (long filename)' '\n \tCOLUMNS=100 git merge --stat --no-ff master >output &&\n-\tgrep \" | \" output >actual\n+\tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh\nindex 67bd8ec..22aa8b7 100755\n--- a/t/t4212-log-corrupt.sh\n+++ b/t/t4212-log-corrupt.sh\n@@ -61,7 +61,7 @@ test_expect_success 'unparsable dates produce sentinel value' '\n test_expect_success 'unparsable dates produce sentinel value (%ad)' '\n \tcommit=$(munge_author_date HEAD totally_bogus) &&\n \techo >expect &&\n-\tgit log -1 --format=%ad $commit >actual\n+\tgit log -1 --format=%ad $commit >actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex eae20d9..ef2feea 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -209,7 +209,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n \tgit config http.cookiefile cookies.txt &&\n \tgit config http.savecookies true &&\n \tgit ls-remote $HTTPD_URL/smart_cookies/repo.git master &&\n-\ttail -3 cookies.txt > cookies_tail.txt\n+\ttail -3 cookies.txt >cookies_tail.txt &&\n \ttest_cmp expect_cookies.txt cookies_tail.txt\n '\n \ndiff --git a/t/t5709-clone-refspec.sh b/t/t5709-clone-refspec.sh\nindex 6f1ea98..7ace253 100755\n--- a/t/t5709-clone-refspec.sh\n+++ b/t/t5709-clone-refspec.sh\n@@ -147,7 +147,7 @@ test_expect_success '--single-branch with detached' '\n \t\tgit for-each-ref refs/remotes/origin |\n \t\tsed -e \"/HEAD$/d\" \\\n \t\t -e \"s|/remotes/origin/|/heads/|\" >../actual\n-\t)\n+\t) &&\n \t# nothing\n \t>expect &&\n \ttest_cmp expect actual\ndiff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh\nindex c518e9c..7763c1b 100755\n--- a/t/t6028-merge-up-to-date.sh\n+++ b/t/t6028-merge-up-to-date.sh\n@@ -83,7 +83,7 @@ test_expect_success 'merge fast-forward octopus' '\n \n \tgit reset --hard c0 &&\n \ttest_tick &&\n-\tgit merge c1 c2\n+\tgit merge c1 c2 &&\n \texpect=$(git rev-parse c2) &&\n \tcurrent=$(git rev-parse HEAD) &&\n \ttest \"$expect\" = \"$current\"\ndiff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh\nindex 62049be..e1e1b1f 100755\n--- a/t/t6132-pathspec-exclude.sh\n+++ b/t/t6132-pathspec-exclude.sh\n@@ -30,7 +30,7 @@ test_expect_success 'exclude only should error out' '\n '\n \n test_expect_success 't_e_i() exclude sub' '\n-\tgit log --oneline --format=%s -- . \":(exclude)sub\" >actual\n+\tgit log --oneline --format=%s -- . \":(exclude)sub\" >actual &&\n \tcat <expect &&\n sub2/file\n file\n@@ -39,7 +39,7 @@ EOF\n '\n \n test_expect_success 't_e_i() exclude sub/sub/file' '\n-\tgit log --oneline --format=%s -- . \":(exclude)sub/sub/file\" >actual\n+\tgit log --oneline --format=%s -- . \":(exclude)sub/sub/file\" >actual &&\n \tcat <expect &&\n sub2/file\n sub/sub/sub/file\n@@ -51,7 +51,7 @@ EOF\n '\n \n test_expect_success 't_e_i() exclude sub using mnemonic' '\n-\tgit log --oneline --format=%s -- . \":!sub\" >actual\n+\tgit log --oneline --format=%s -- . \":!sub\" >actual &&\n \tcat <expect &&\n sub2/file\n file\n@@ -60,7 +60,7 @@ EOF\n '\n \n test_expect_success 't_e_i() exclude :(icase)SUB' '\n-\tgit log --oneline --format=%s -- . \":(exclude,icase)SUB\" >actual\n+\tgit log --oneline --format=%s -- . \":(exclude,icase)SUB\" >actual &&\n \tcat <expect &&\n sub2/file\n file\n@@ -71,7 +71,7 @@ EOF\n test_expect_success 't_e_i() exclude sub2 from sub' '\n \t(\n \tcd sub &&\n-\tgit log --oneline --format=%s -- :/ \":/!sub2\" >actual\n+\tgit log --oneline --format=%s -- :/ \":/!sub2\" >actual &&\n \tcat <expect &&\n sub/sub/sub/file\n sub/file2\n@@ -84,7 +84,7 @@ EOF\n '\n \n test_expect_success 't_e_i() exclude sub/*file' '\n-\tgit log --oneline --format=%s -- . \":(exclude)sub/*file\" >actual\n+\tgit log --oneline --format=%s -- . \":(exclude)sub/*file\" >actual &&\n \tcat <expect &&\n sub2/file\n sub/file2\n@@ -94,7 +94,7 @@ EOF\n '\n \n test_expect_success 't_e_i() exclude :(glob)sub/*/file' '\n-\tgit log --oneline --format=%s -- . \":(exclude,glob)sub/*/file\" >actual\n+\tgit log --oneline --format=%s -- . \":(exclude,glob)sub/*/file\" >actual &&\n \tcat <expect &&\n sub2/file\n sub/sub/sub/file\n@@ -106,7 +106,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude sub' '\n-\tgit ls-files -- . \":(exclude)sub\" >actual\n+\tgit ls-files -- . \":(exclude)sub\" >actual &&\n \tcat <expect &&\n file\n sub2/file\n@@ -115,7 +115,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude sub/sub/file' '\n-\tgit ls-files -- . \":(exclude)sub/sub/file\" >actual\n+\tgit ls-files -- . \":(exclude)sub/sub/file\" >actual &&\n \tcat <expect &&\n file\n sub/file\n@@ -127,7 +127,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude sub using mnemonic' '\n-\tgit ls-files -- . \":!sub\" >actual\n+\tgit ls-files -- . \":!sub\" >actual &&\n \tcat <expect &&\n file\n sub2/file\n@@ -136,7 +136,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude :(icase)SUB' '\n-\tgit ls-files -- . \":(exclude,icase)SUB\" >actual\n+\tgit ls-files -- . \":(exclude,icase)SUB\" >actual &&\n \tcat <expect &&\n file\n sub2/file\n@@ -147,7 +147,7 @@ EOF\n test_expect_success 'm_p_d() exclude sub2 from sub' '\n \t(\n \tcd sub &&\n-\tgit ls-files -- :/ \":/!sub2\" >actual\n+\tgit ls-files -- :/ \":/!sub2\" >actual &&\n \tcat <expect &&\n ../file\n file\n@@ -160,7 +160,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude sub/*file' '\n-\tgit ls-files -- . \":(exclude)sub/*file\" >actual\n+\tgit ls-files -- . \":(exclude)sub/*file\" >actual &&\n \tcat <expect &&\n file\n sub/file2\n@@ -170,7 +170,7 @@ EOF\n '\n \n test_expect_success 'm_p_d() exclude :(glob)sub/*/file' '\n-\tgit ls-files -- . \":(exclude,glob)sub/*/file\" >actual\n+\tgit ls-files -- . \":(exclude,glob)sub/*/file\" >actual &&\n \tcat <expect &&\n file\n sub/file\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258101","messageId":"20150320100714.GD12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 04/25] t: fix trivial &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:07:15Z","receivedAt":"2015-03-20T10:07:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are tests which are missing a link in their &&-chain,\nbut during a setup phase. We may fail to notice failure in\ncommands that build the test environment, but these are\ntypically not expected to fail at all (but it's still good\nto double-check that our test environment is what we\nexpect).\n\nSigned-off-by: Jeff King \n---\n t/annotate-tests.sh | 2 +-\n t/t0000-basic.sh | 4 ++--\n t/t0011-hashmap.sh | 2 +-\n t/t0201-gettext-fallbacks.sh | 4 ++--\n t/t1006-cat-file.sh | 2 +-\n t/t3010-ls-files-killed-modified.sh | 2 +-\n t/t3404-rebase-interactive.sh | 2 +-\n t/t3405-rebase-malformed.sh | 4 ++--\n t/t3425-rebase-topology-merges.sh | 4 ++--\n t/t3600-rm.sh | 4 ++--\n t/t4014-format-patch.sh | 2 +-\n t/t4041-diff-submodule-option.sh | 10 ++++++----\n t/t4049-diff-stat-count.sh | 6 +++---\n t/t4054-diff-bogus-tree.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t5510-fetch.sh | 2 +-\n t/t5526-fetch-submodules.sh | 10 +++++-----\n t/t5531-deep-submodule-push.sh | 2 +-\n t/t5533-push-cas.sh | 4 ++--\n t/t5541-http-push-smart.sh | 2 +-\n t/t5550-http-fetch-dumb.sh | 2 +-\n t/t5551-http-fetch-smart.sh | 2 +-\n t/t6000-rev-list-misc.sh | 4 ++--\n t/t6006-rev-list-format.sh | 2 +-\n t/t6022-merge-rename.sh | 6 +++---\n t/t6036-recursive-corner-cases.sh | 2 +-\n t/t6111-rev-list-treesame.sh | 2 +-\n t/t6200-fmt-merge-msg.sh | 2 +-\n t/t7004-tag.sh | 2 +-\n t/t7006-pager.sh | 2 +-\n t/t7009-filter-branch-null-sha1.sh | 2 +-\n t/t7400-submodule-basic.sh | 4 ++--\n t/t7406-submodule-update.sh | 2 +-\n t/t7508-status.sh | 8 ++++----\n t/t7510-signed-commit.sh | 2 +-\n t/t7600-merge.sh | 2 +-\n t/t7612-merge-verify-signatures.sh | 2 +-\n t/t8003-blame-corner-cases.sh | 8 ++++----\n t/t8008-blame-formats.sh | 2 +-\n t/t9001-send-email.sh | 2 +-\n t/t9300-fast-import.sh | 2 +-\n t/t9500-gitweb-standalone-no-errors.sh | 2 +-\n t/t9903-bash-prompt.sh | 2 +-\n 43 files changed, 70 insertions(+), 68 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 071e4d7..f5c0175 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -405,7 +405,7 @@ test_expect_success 'setup -L :regex' '\n \tmv hello.c hello.orig &&\n \techo \"#include \" >hello.c &&\n \tcat hello.orig >>hello.c &&\n-\ttr Q \"\\\\t\" >>hello.c <<-\\EOF\n+\ttr Q \"\\\\t\" >>hello.c <<-\\EOF &&\n \tvoid mail()\n \t{\n \tQputs(\"mail\");\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex f10ba4a..79b9074 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -253,7 +253,7 @@ test_expect_success 'test --verbose' '\n \ttest_expect_success \"failing test\" false\n \ttest_done\n \tEOF\n-\tmv test-verbose/out test-verbose/out+\n+\tmv test-verbose/out test-verbose/out+ &&\n \tgrep -v \"^Initialized empty\" test-verbose/out+ >test-verbose/out &&\n \tcheck_sub_test_lib_test test-verbose <<-\\EOF\n \t> expecting success: true\n@@ -974,7 +974,7 @@ test_expect_success 'writing this tree with --missing-ok' '\n \n ################################################################\n test_expect_success 'git read-tree followed by write-tree should be idempotent' '\n-\trm -f .git/index\n+\trm -f .git/index &&\n \tgit read-tree $tree &&\n \ttest -f .git/index &&\n \tnewtree=$(git write-tree) &&\ndiff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh\nindex f97c805..9c217d9 100755\n--- a/t/t0011-hashmap.sh\n+++ b/t/t0011-hashmap.sh\n@@ -218,7 +218,7 @@ test_expect_success 'grow / shrink' '\n \techo size >> in &&\n \techo 64 51 >> expect &&\n \techo put key52 value52 >> in &&\n-\techo NULL >> expect\n+\techo NULL >> expect &&\n \techo size >> in &&\n \techo 256 52 >> expect &&\n \tfor n in $(test_seq 12)\ndiff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh\nindex 00c6d3d..90da1c7 100755\n--- a/t/t0201-gettext-fallbacks.sh\n+++ b/t/t0201-gettext-fallbacks.sh\n@@ -50,7 +50,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v\n \n test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces' '\n cmdline=\"git am\" &&\n- export cmdline;\n+ export cmdline &&\n printf \"When you have resolved this problem, run git am --resolved.\" >expect &&\n eval_gettext \"When you have resolved this problem, run \\$cmdline --resolved.\" >actual &&\n test_i18ncmp expect actual\n@@ -58,7 +58,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v\n \n test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces and quotes' '\n cmdline=\"git am\" &&\n- export cmdline;\n+ export cmdline &&\n printf \"When you have resolved this problem, run \\\"git am --resolved\\\".\" >expect &&\n eval_gettext \"When you have resolved this problem, run \\\"\\$cmdline --resolved\\\".\" >actual &&\n test_i18ncmp expect actual\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex a72e700..ab36b1e 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -274,7 +274,7 @@ test_expect_success 'setup blobs which are likely to delta' '\n '\n \n test_expect_success 'confirm that neither loose blob is a delta' '\n-\tcat >expect <<-EOF\n+\tcat >expect <<-EOF &&\n \t$_z40\n \t$_z40\n \tEOF\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex ea6a678..62fce10 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -62,7 +62,7 @@ test_expect_success 'git update-index --add to add various paths.' '\n \t\t\tcd submod$i && git commit --allow-empty -m \"empty $i\"\n \t\t) || break\n \tdone &&\n-\tgit update-index --add submod[12]\n+\tgit update-index --add submod[12] &&\n \t(\n \t\tcd submod1 &&\n \t\tgit commit --allow-empty -m \"empty 1 (updated)\"\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex edad20d..eed76cc 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1007,7 +1007,7 @@ test_expect_success 'rebase -i with --strategy and -X' '\n '\n \n test_expect_success 'rebase -i error on commits with \\ in message' '\n-\tcurrent_head=$(git rev-parse HEAD)\n+\tcurrent_head=$(git rev-parse HEAD) &&\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n \ttest_commit TO-REMOVE will-conflict old-content &&\n \ttest_commit \"\\temp\" will-conflict new-content dummy &&\ndiff --git a/t/t3405-rebase-malformed.sh b/t/t3405-rebase-malformed.sh\nindex 19eddad..ff8c360 100755\n--- a/t/t3405-rebase-malformed.sh\n+++ b/t/t3405-rebase-malformed.sh\n@@ -24,7 +24,7 @@ test_expect_success setup '\n \tgit add file1 file2 &&\n \ttest_tick &&\n \tgit commit -m \"Initial commit\" &&\n-\tgit branch diff-in-message\n+\tgit branch diff-in-message &&\n \n \tgit checkout -b multi-line-subject &&\n \tcat F >file2 &&\n@@ -36,7 +36,7 @@ test_expect_success setup '\n \n \tgit checkout diff-in-message &&\n \techo \"commit log message containing a diff\" >G &&\n-\techo \"\" >>G\n+\techo \"\" >>G &&\n \tcat G >file2 &&\n \tgit add file2 &&\n \tgit diff --cached >>G &&\ndiff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh\nindex 1d195fb..846f85c 100755\n--- a/t/t3425-rebase-topology-merges.sh\n+++ b/t/t3425-rebase-topology-merges.sh\n@@ -24,7 +24,7 @@ test_expect_success 'setup of non-linear-history' '\n \ttest_commit c &&\n \tgit checkout b &&\n \ttest_commit d &&\n-\ttest_commit e\n+\ttest_commit e &&\n \n \tgit checkout c &&\n \ttest_commit g &&\n@@ -33,7 +33,7 @@ test_expect_success 'setup of non-linear-history' '\n \tcherry_pick gp g &&\n \ttest_commit i &&\n \tgit checkout b &&\n-\ttest_commit f\n+\ttest_commit f &&\n \n \tgit checkout d &&\n \ttest_commit n &&\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex b0db89b..1962989 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -183,7 +183,7 @@ test_expect_success 'refuse to remove cached empty file with modifications' '\n \n test_expect_success 'remove intent-to-add file without --force' '\n \techo content >intent-to-add &&\n-\tgit add -N intent-to-add\n+\tgit add -N intent-to-add &&\n \tgit rm --cached intent-to-add\n '\n \n@@ -201,7 +201,7 @@ test_expect_success 'Recursive without -r fails' '\n '\n \n test_expect_success 'Recursive with -r but dirty' '\n-\techo qfwfq >>frotz/nitfol\n+\techo qfwfq >>frotz/nitfol &&\n \ttest_must_fail git rm -r frotz &&\n \ttest -d frotz &&\n \ttest -f frotz/nitfol\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 256affc..c39e500 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -802,7 +802,7 @@ test_expect_success '--no-signature suppresses format.signaturefile ' '\n '\n \n test_expect_success '--signature-file overrides format.signaturefile' '\n-\tcat >other-mail-signature <<-\\EOF\n+\tcat >other-mail-signature <<-\\EOF &&\n \tUse this other signature instead of mail-signature.\n \tEOF\n \ttest_config format.signaturefile mail-signature &&\ndiff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\nindex ff61341..2d9731b 100755\n--- a/t/t4041-diff-submodule-option.sh\n+++ b/t/t4041-diff-submodule-option.sh\n@@ -528,10 +528,12 @@ test_expect_success 'diff --submodule with objects referenced by alternates' '\n \t\tsha1_before=$(git rev-parse --short HEAD)\n \t\techo b >b &&\n \t\tgit add b &&\n-\t\tgit commit -m b\n-\t\tsha1_after=$(git rev-parse --short HEAD)\n-\t\techo \"Submodule sub $sha1_before..$sha1_after:\n- > b\" >../expected\n+\t\tgit commit -m b &&\n+\t\tsha1_after=$(git rev-parse --short HEAD) &&\n+\t\t{\n+\t\t\techo \"Submodule sub $sha1_before..$sha1_after:\" &&\n+\t\t\techo \" > b\"\n+\t\t} >../expected\n \t) &&\n \t(cd super &&\n \t\t(cd sub &&\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex 5b594e8..a341217 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -18,7 +18,7 @@ test_expect_success 'mode-only change show as a 0-line change' '\n \ttest_chmod +x b d &&\n \techo a >a &&\n \techo c >c &&\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t a | 1 +\n \t b | 0\n \t ...\n@@ -33,7 +33,7 @@ test_expect_success 'binary changes do not count in lines' '\n \techo a >a &&\n \techo c >c &&\n \tcat \"$TEST_DIRECTORY\"/test-binary-1.png >d &&\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t a | 1 +\n \t c | 1 +\n \t ...\n@@ -55,7 +55,7 @@ test_expect_success 'exclude unmerged entries from total file count' '\n \tdone |\n \tgit update-index --index-info &&\n \techo d >d &&\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t a | 1 +\n \t b | 1 +\n \t ...\ndiff --git a/t/t4054-diff-bogus-tree.sh b/t/t4054-diff-bogus-tree.sh\nindex 0843c87..1d6efab 100755\n--- a/t/t4054-diff-bogus-tree.sh\n+++ b/t/t4054-diff-bogus-tree.sh\n@@ -16,7 +16,7 @@ test_expect_success 'create bogus tree' '\n test_expect_success 'create tree with matching file' '\n \techo bar >foo &&\n \tgit add foo &&\n-\tgood_tree=$(git write-tree)\n+\tgood_tree=$(git write-tree) &&\n \tblob=$(git rev-parse :foo)\n '\n \ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex bd37f04..692d717 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -414,7 +414,7 @@ test_expect_success 'setup tests for the --stdin parameter' '\n \tdo\n \t\tgit tag $head $head\n \tdone &&\n-\tcat >input <<-\\EOF\n+\tcat >input <<-\\EOF &&\n \trefs/heads/C\n \trefs/heads/A\n \trefs/heads/D\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d3fa2c9..0ba9db0 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -596,7 +596,7 @@ test_configured_prune () {\n \t\t\ttest_unconfig remote.origin.prune &&\n \t\t\tgit fetch &&\n \t\t\tgit rev-parse --verify refs/remotes/origin/newbranch\n-\t\t)\n+\t\t) &&\n \n \t\t# now remove it\n \t\tgit branch -d newbranch &&\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex ca5b027..a4532b0 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -216,7 +216,7 @@ test_expect_success \"Recursion stops when no new submodule commits are fetched\"\n \thead2=$(git rev-parse --short HEAD) &&\n \techo \"Fetching submodule submodule\" > expect.out.sub &&\n \techo \"From $pwd/.\" > expect.err.sub &&\n-\techo \" $head1..$head2 master -> origin/master\" >> expect.err.sub\n+\techo \" $head1..$head2 master -> origin/master\" >>expect.err.sub &&\n \thead -2 expect.err >> expect.err.sub &&\n \t(\n \t\tcd downstream &&\n@@ -315,7 +315,7 @@ test_expect_success \"'--recurse-submodules=on-demand' doesn't recurse when no ne\n \t\t) &&\n \t\thead1=$(git rev-parse --short HEAD^) &&\n \t\tgit add subdir/deepsubmodule &&\n-\t\tgit commit -m \"new deepsubmodule\"\n+\t\tgit commit -m \"new deepsubmodule\" &&\n \t\thead2=$(git rev-parse --short HEAD) &&\n \t\techo \"From $pwd/submodule\" > ../expect.err.sub &&\n \t\techo \" $head1..$head2 master -> origin/master\" >> ../expect.err.sub\n@@ -337,7 +337,7 @@ test_expect_success \"'--recurse-submodules=on-demand' recurses as deep as necess\n \thead2=$(git rev-parse --short HEAD) &&\n \ttail -2 expect.err > expect.err.deepsub &&\n \techo \"From $pwd/.\" > expect.err &&\n-\techo \" $head1..$head2 master -> origin/master\" >> expect.err\n+\techo \" $head1..$head2 master -> origin/master\" >>expect.err &&\n \tcat expect.err.sub >> expect.err &&\n \tcat expect.err.deepsub >> expect.err &&\n \t(\n@@ -387,7 +387,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \tgit commit -m \"new submodule\" &&\n \thead2=$(git rev-parse --short HEAD) &&\n \techo \"From $pwd/.\" > expect.err.2 &&\n-\techo \" $head1..$head2 master -> origin/master\" >> expect.err.2\n+\techo \" $head1..$head2 master -> origin/master\" >>expect.err.2 &&\n \thead -2 expect.err >> expect.err.2 &&\n \t(\n \t\tcd downstream &&\n@@ -415,7 +415,7 @@ test_expect_success \"'submodule..fetchRecurseSubmodules=on-demand' override\n \tgit commit -m \"new submodule\" &&\n \thead2=$(git rev-parse --short HEAD) &&\n \techo \"From $pwd/.\" > expect.err.2 &&\n-\techo \" $head1..$head2 master -> origin/master\" >> expect.err.2\n+\techo \" $head1..$head2 master -> origin/master\" >>expect.err.2 &&\n \thead -2 expect.err >> expect.err.2 &&\n \t(\n \t\tcd downstream &&\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 445bb5f..6507487 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -104,7 +104,7 @@ test_expect_success 'push fails when commit on multiple branches if one branch h\n '\n \n test_expect_success 'push succeeds if submodule has no remote and is on the first superproject commit' '\n-\tgit init --bare a\n+\tgit init --bare a &&\n \tgit clone a a1 &&\n \t(\n \t\tcd a1 &&\ndiff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh\nindex ba20d83..dccf8a6 100755\n--- a/t/t5533-push-cas.sh\n+++ b/t/t5533-push-cas.sh\n@@ -159,7 +159,7 @@ test_expect_success 'cover everything with default force-with-lease (protected)'\n \t(\n \t\tcd src &&\n \t\tgit branch naster master^\n-\t)\n+\t) &&\n \tgit ls-remote src refs/heads/\\* >expect &&\n \t(\n \t\tcd dst &&\n@@ -174,7 +174,7 @@ test_expect_success 'cover everything with default force-with-lease (allowed)' '\n \t(\n \t\tcd src &&\n \t\tgit branch naster master^\n-\t)\n+\t) &&\n \t(\n \t\tcd dst &&\n \t\tgit fetch &&\ndiff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh\nindex d2c681e..47cee53 100755\n--- a/t/t5541-http-push-smart.sh\n+++ b/t/t5541-http-push-smart.sh\n@@ -158,7 +158,7 @@ test_http_push_nonff \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '\n \t# create a dissimilarly-named remote ref so that git is unable to match the\n \t# two refs (viz. local, remote) unless an explicit refspec is provided.\n-\tgit push origin master:retsam\n+\tgit push origin master:retsam &&\n \n \techo \"change changed\" > path2 &&\n \tgit commit -a -m path2 --amend &&\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 2731ad4..61e087e 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -15,7 +15,7 @@ test_expect_success 'setup repository' '\n \tgit config push.default matching &&\n \techo content1 >file &&\n \tgit add file &&\n-\tgit commit -m one\n+\tgit commit -m one &&\n \techo content2 >file &&\n \tgit add file &&\n \tgit commit -m two\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex ef2feea..cf0a6ea 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -83,7 +83,7 @@ test_expect_success 'clone http repository' '\n test_expect_success 'fetch changes via http' '\n \techo content >>file &&\n \tgit commit -a -m two &&\n-\tgit push public\n+\tgit push public &&\n \t(cd clone && git pull) &&\n \ttest_cmp file clone/file\n '\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 2602086..7911ed9 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -42,7 +42,7 @@ test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \ttest_tick &&\n \tgit commit -m that &&\n \n-\tONE=$(git rev-parse HEAD:one)\n+\tONE=$(git rev-parse HEAD:one) &&\n \tgit rev-list --objects HEAD two >output &&\n \tgrep \"$ONE two/three\" output &&\n \t! grep one output\n@@ -85,7 +85,7 @@ test_expect_success 'rev-list can show index objects' '\n \t# - we do not show the root tree; since we updated the index, it\n \t# does not have a valid cache tree\n \t#\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t8e4020bb5a8d8c873b25de15933e75cc0fc275df one\n \td9d3a7417b9605cfd88ee6306b28dadc29e6ab08 only-in-index\n \t9200b628cf9dc883a85a7abc8d6e6730baee589c two\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex a02a45a..2b7c0f0 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -45,7 +45,7 @@ test_expect_success 'setup' '\n \thead2=$(git rev-parse --verify HEAD) &&\n \thead2_short=$(git rev-parse --verify --short $head2) &&\n \ttree2=$(git rev-parse --verify HEAD:) &&\n-\ttree2_short=$(git rev-parse --verify --short $tree2)\n+\ttree2_short=$(git rev-parse --verify --short $tree2) &&\n \tgit config --unset i18n.commitEncoding\n '\n \ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex a89dfbe..05ebba7 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -685,7 +685,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '\n \tgit add -A &&\n \tgit commit -mA &&\n \n-\tgit checkout -b side\n+\tgit checkout -b side &&\n \tgit rm -rf df &&\n \tgit commit -mB &&\n \n@@ -716,7 +716,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '\n \tgit add -A &&\n \tgit commit -mA &&\n \n-\tgit checkout -b side\n+\tgit checkout -b side &&\n \tgit rm -f file &&\n \tgit commit -m \"Delete file\" &&\n \n@@ -745,7 +745,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '\n \tgit add -A &&\n \tgit commit -mA &&\n \n-\tgit checkout -b side\n+\tgit checkout -b side &&\n \tcp file newfile &&\n \tgit add -A &&\n \tgit commit -m \"Add file copy\" &&\ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex b43d031..8923b04 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -86,7 +86,7 @@ test_expect_success 'setup criss-cross + rename merges with basic modification'\n \trm -rf .git &&\n \tgit init &&\n \n-\tten=\"0 1 2 3 4 5 6 7 8 9\"\n+\tten=\"0 1 2 3 4 5 6 7 8 9\" &&\n \tfor i in $ten\n \tdo\n \t\techo line $i in a sample file\ndiff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh\nindex 88b84df..45e3673 100755\n--- a/t/t6111-rev-list-treesame.sh\n+++ b/t/t6111-rev-list-treesame.sh\n@@ -58,7 +58,7 @@ test_expect_success setup '\n \n \tgit checkout master &&\n \ttest_tick && git merge --no-ff fiddler-branch &&\n-\tnote K\n+\tnote K &&\n \n \ttest_commit \"file=Part 1\" file \"Part 1\" L &&\n \ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 54b5744..2e2fb0e 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -194,7 +194,7 @@ test_expect_success '--log=5 with custom comment character' '\n '\n \n test_expect_success 'merge.log=0 disables shortlog' '\n-\techo \"Merge branch ${apos}left${apos}\" >expected\n+\techo \"Merge branch ${apos}left${apos}\" >expected &&\n \tgit -c merge.log=0 fmt-merge-msg <.git/FETCH_HEAD >actual &&\n \ttest_cmp expected actual\n '\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 35c805a..347d3be 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1180,7 +1180,7 @@ test_expect_success 'message in editor has initial comment: first line' '\n test_expect_success \\\n \t'message in editor has initial comment: remainder' '\n \t# remove commented lines from the remainder -- should be empty\n-\t>rest.expect\n+\t>rest.expect &&\n \tsed -e 1d -e '/^#/d' rest.actual &&\n \ttest_cmp rest.expect rest.actual\n '\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex da958a8..947b690 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -396,7 +396,7 @@ test_expect_success TTY 'command-specific pager overrides core.pager' '\n \tsane_unset PAGER GIT_PAGER &&\n \techo \"foo:initial\" >expect &&\n \t>actual &&\n-\ttest_config core.pager \"exit 1\"\n+\ttest_config core.pager \"exit 1\" &&\n \ttest_config pager.log \"sed s/^/foo:/ >actual\" &&\n \ttest_terminal git log --format=%s -1 &&\n \ttest_cmp expect actual\ndiff --git a/t/t7009-filter-branch-null-sha1.sh b/t/t7009-filter-branch-null-sha1.sh\nindex a997f7a..c27f90f 100755\n--- a/t/t7009-filter-branch-null-sha1.sh\n+++ b/t/t7009-filter-branch-null-sha1.sh\n@@ -13,7 +13,7 @@ test_expect_success 'setup: a commit with a bogus null sha1 in the tree' '\n \t{\n \t\tgit ls-tree HEAD &&\n \t\tprintf \"160000 commit $_z40\\\\tbroken\\\\n\"\n-\t} >broken-tree\n+\t} >broken-tree &&\n \techo \"add broken entry\" >msg &&\n \n \ttree=$(git mktree expect\n-\t)\n+\t) &&\n \tmv addtest addtest2 &&\n \t(\n \t\tcd addtest2 &&\n@@ -987,7 +987,7 @@ test_expect_success 'submodule with UTF-8 name' '\n \n test_expect_success 'submodule add clone shallow submodule' '\n \tmkdir super &&\n-\tpwd=$(pwd)\n+\tpwd=$(pwd) &&\n \t(\n \t\tcd super &&\n \t\tgit init &&\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 29d3d2c..dda3929 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -754,7 +754,7 @@ test_expect_success SYMLINKS 'submodule update can handle symbolic links in pwd'\n \n test_expect_success 'submodule update clone shallow submodule' '\n \tgit clone cloned super3 &&\n-\tpwd=$(pwd)\n+\tpwd=$(pwd) &&\n \t(cd super3 &&\n \t sed -e \"s#url = ../#url = file://$pwd/#\" <.gitmodules >.gitmodules.tmp &&\n \t mv -f .gitmodules.tmp .gitmodules &&\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 6b16bcb..0f9ad4c 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -431,7 +431,7 @@ test_expect_success 'status -s -uno' '\n '\n \n test_expect_success 'status -s (status.showUntrackedFiles no)' '\n-\tgit config status.showuntrackedfiles no\n+\tgit config status.showuntrackedfiles no &&\n \tgit status -s >output &&\n \ttest_cmp expect output\n '\n@@ -465,7 +465,7 @@ EOF\n '\n \n test_expect_success 'status (status.showUntrackedFiles normal)' '\n-\ttest_config status.showuntrackedfiles normal\n+\ttest_config status.showuntrackedfiles normal &&\n \tgit status >output &&\n \ttest_i18ncmp expect output\n '\n@@ -485,7 +485,7 @@ test_expect_success 'status -s -unormal' '\n '\n \n test_expect_success 'status -s (status.showUntrackedFiles normal)' '\n-\tgit config status.showuntrackedfiles normal\n+\tgit config status.showuntrackedfiles normal &&\n \tgit status -s >output &&\n \ttest_cmp expect output\n '\n@@ -520,7 +520,7 @@ EOF\n '\n \n test_expect_success 'status (status.showUntrackedFiles all)' '\n-\ttest_config status.showuntrackedfiles all\n+\ttest_config status.showuntrackedfiles all &&\n \tgit status >output &&\n \ttest_i18ncmp expect output\n '\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 474dab3..fc1ff45 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -42,7 +42,7 @@ test_expect_success GPG 'create signed commits' '\n \tgit tag seventh-unsigned &&\n \n \ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n-\tgit tag seventh-signed\n+\tgit tag seventh-signed &&\n \n \techo 8 >file && test_tick && git commit -a -m eighth -SB7227189 &&\n \tgit tag eighth-signed-alt\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex b164621..75c50ee 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -133,7 +133,7 @@ test_expect_success 'setup' '\n \ttest_tick &&\n \tgit commit -m \"commit 3\" &&\n \tgit tag c3 &&\n-\tc3=$(git rev-parse HEAD)\n+\tc3=$(git rev-parse HEAD) &&\n \tgit reset --hard \"$c0\" &&\n \tcreate_merge_msgs\n '\ndiff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh\nindex 21a0bf8..8ae69a6 100755\n--- a/t/t7612-merge-verify-signatures.sh\n+++ b/t/t7612-merge-verify-signatures.sh\n@@ -29,7 +29,7 @@ test_expect_success GPG 'create signed commits' '\n \n \tgit checkout -b side-untrusted &&\n \techo 3 >baz && git add baz &&\n-\ttest_tick && git commit -SB7227189 -m \"untrusted on side\"\n+\ttest_tick && git commit -SB7227189 -m \"untrusted on side\" &&\n \n \tgit checkout master\n '\ndiff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh\nindex 2a3469b..32895e5 100755\n--- a/t/t8003-blame-corner-cases.sh\n+++ b/t/t8003-blame-corner-cases.sh\n@@ -26,7 +26,7 @@ test_expect_success setup '\n \tcat one >uno &&\n \tmv two dos &&\n \tcat one >>tres &&\n-\techo DEF >>mouse\n+\techo DEF >>mouse &&\n \tgit add uno dos tres mouse &&\n \ttest_tick &&\n \tGIT_AUTHOR_NAME=Second git commit -a -m Second &&\n@@ -153,15 +153,15 @@ test_expect_success 'blame path that used to be a directory' '\n '\n \n test_expect_success 'blame to a commit with no author name' '\n- TREE=`git rev-parse HEAD:`\n- cat >badcommit <badcommit < 1234567890 +0000\n committer David Reiss 1234567890 +0000\n \n some message\n EOF\n- COMMIT=`git hash-object -t commit -w badcommit`\n+ COMMIT=`git hash-object -t commit -w badcommit` &&\n git --no-pager blame $COMMIT -- uno >/dev/null\n '\n \ndiff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh\nindex d15f8b3..29f84a6 100755\n--- a/t/t8008-blame-formats.sh\n+++ b/t/t8008-blame-formats.sh\n@@ -5,7 +5,7 @@ test_description='blame output in various formats on a simple case'\n \n test_expect_success 'setup' '\n \techo a >file &&\n-\tgit add file\n+\tgit add file &&\n \ttest_tick &&\n \tgit commit -m one &&\n \techo b >>file &&\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0513055..37caa18 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1325,7 +1325,7 @@ test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data'\n \n test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' '\n \tclean_fake_sendmail &&\n-\tgit config sendemail.transferEncoding 8bit\n+\tgit config sendemail.transferEncoding 8bit &&\n \ttest_must_fail git send-email \\\n \t\t--transfer-encoding=7bit \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 6bd5a00..584b538 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1132,7 +1132,7 @@ test_expect_success \\\n \t compare_diff_raw expect actual'\n \n test_expect_success PIPE 'N: read and copy directory' '\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc f1fb5da718392694d0076d677d6d0e364c79b0bc C100\tfile2/newf\tfile3/newf\n \t:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1 7123f7f44e39be127c5eb701e5968176ee9d78b1 C100\tfile2/oldf\tfile3/oldf\n \tEOF\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e74b9ab..f9f078e 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -297,7 +297,7 @@ test_expect_success 'setup incomplete lines' '\n \techo \"Dominus regit me,\" >file &&\n \techo \"incomplete line\" | tr -d \"\\\\012\" >>file &&\n \tgit commit -a -m \"Change incomplete line\" &&\n-\tgit tag incomplete_lines_chg\n+\tgit tag incomplete_lines_chg &&\n \techo \"Dominus regit me,\" >file &&\n \tgit commit -a -m \"Remove incomplete line\" &&\n \tgit tag incomplete_lines_rem\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex 51ecd3e..0c6acdd 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -166,7 +166,7 @@ test_expect_success 'prompt - inside bare repository' '\n '\n \n test_expect_success 'prompt - interactive rebase' '\n-\tprintf \" (b1|REBASE-i 2/3)\" >expected\n+\tprintf \" (b1|REBASE-i 2/3)\" >expected &&\n \twrite_script fake_editor.sh <<-\\EOF &&\n \t\techo \"exec echo\" >\"$1\"\n \t\techo \"edit $(git log -1 --format=\"%h\")\" >>\"$1\"\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258102","messageId":"20150320100752.GE12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 05/25] t: assume test_cmp produces verbose output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:07:52Z","receivedAt":"2015-03-20T10:07:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some tests call test_cmp, and if it fails show the actual\noutput generated. This is mostly pointless, as test_cmp will\nalready show a diff between the expected and actual output.\nIt also fools --chain-lint by putting an \"||\" in the middle\nof the chain, so we'd rather not use this construct.\n\nNote that these cases actually show a pre-processed version\nof the data, rather than exactly what test_cmp would show.\nHowever, test_cmp's output is generally good for pointing\nthe user in the right direction, and they can then dig in\nthe trash directory themselves if they want to see more\ndetails.\n\nSigned-off-by: Jeff King \n---\n t/t6012-rev-list-simplify.sh | 10 ++--------\n t/t6111-rev-list-treesame.sh | 5 +----\n 2 files changed, 3 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex fde5e71..b89cd6b 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -95,10 +95,7 @@ check_outcome () {\n \t\tgit log --pretty=\"$FMT\" --parents $param |\n \t\tunnote >actual &&\n \t\tsed -e \"s/^.*\t\\([^ ]*\\) .*/\\1/\" >check actual &&\n \tsed -e \"s/^.*\t\\([^ ]*\\) .*/\\1/\" >check actual &&\n \t\tsed -e \"$munge_actual\" check &&\n-\t\ttest_cmp expect check || {\n-\t\t\tcat actual\n-\t\t\tfalse\n-\t\t}\n+\t\ttest_cmp expect check\n \t'\n }\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258103","messageId":"20150320100900.GF12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 06/25] t: use verbose instead of hand-rolled errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:09:00Z","receivedAt":"2015-03-20T10:09:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many tests that predate the \"verbose\" helper function use a\npattern like:\n\n test ... || {\n\t echo ...\n\t false\n }\n\nto give more verbose output. Using the helper, we can do\nthis with a single line, and avoid a || which interacts\nbadly with &&-chaining (besides fooling --chain-lint, we hit\nthe error block no matter which command in the chain failed,\nso we may often show useless results).\n\nIn most cases, the messages printed by \"verbose\" are equally\ngood (in some cases better; t6006 accidentally redirects the\nmessage to a file!). The exception is t7001, whose output\nsuffers slightly. However, it's still enough to show the\nuser which part failed, given that we will have just printed\nthe test script to stderr.\n\nSigned-off-by: Jeff King \n---\n t/t4022-diff-rewrite.sh | 5 +----\n t/t4202-log.sh | 30 +++++-------------------------\n t/t6006-rev-list-format.sh | 5 +----\n t/t7001-mv.sh | 5 +----\n t/t7300-clean.sh | 10 ++--------\n 5 files changed, 10 insertions(+), 45 deletions(-)\n\ndiff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh\nindex 2d030a4..cb51d9f 100755\n--- a/t/t4022-diff-rewrite.sh\n+++ b/t/t4022-diff-rewrite.sh\n@@ -20,10 +20,7 @@ test_expect_success setup '\n test_expect_success 'detect rewrite' '\n \n \tactual=$(git diff-files -B --summary test) &&\n-\texpr \"$actual\" : \" rewrite test ([0-9]*%)$\" || {\n-\t\techo \"Eh? <<$actual>>\"\n-\t\tfalse\n-\t}\n+\tverbose expr \"$actual\" : \" rewrite test ([0-9]*%)$\"\n \n '\n \ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex a22ac7c..85230a8 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -113,11 +113,7 @@ test_expect_success 'diff-filter=M' '\n \n \tactual=$(git log --pretty=\"format:%s\" --diff-filter=M HEAD) &&\n \texpect=$(echo second) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$actual\" = \"$expect\"\n \n '\n \n@@ -125,11 +121,7 @@ test_expect_success 'diff-filter=D' '\n \n \tactual=$(git log --pretty=\"format:%s\" --diff-filter=D HEAD) &&\n \texpect=$(echo sixth ; echo third) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$actual\" = \"$expect\"\n \n '\n \n@@ -137,11 +129,7 @@ test_expect_success 'diff-filter=R' '\n \n \tactual=$(git log -M --pretty=\"format:%s\" --diff-filter=R HEAD) &&\n \texpect=$(echo third) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$actual\" = \"$expect\"\n \n '\n \n@@ -149,11 +137,7 @@ test_expect_success 'diff-filter=C' '\n \n \tactual=$(git log -C -C --pretty=\"format:%s\" --diff-filter=C HEAD) &&\n \texpect=$(echo fourth) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$actual\" = \"$expect\"\n \n '\n \n@@ -161,11 +145,7 @@ test_expect_success 'git log --follow' '\n \n \tactual=$(git log --follow --pretty=\"format:%s\" ichi) &&\n \texpect=$(echo third ; echo second ; echo initial) &&\n-\ttest \"$actual\" = \"$expect\" || {\n-\t\techo Oops\n-\t\techo \"Actual: $actual\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$actual\" = \"$expect\"\n \n '\n \ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 2b7c0f0..b77d4c9 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -358,10 +358,7 @@ test_expect_success 'empty email' '\n \ttest_tick &&\n \tC=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} failure\n-\t\tfalse\n-\t}\n+\tverbose test \"$A\" = \"A U Thor,,Thu Apr 7 15:14:13 2005 -0700\"\n '\n \n test_expect_success 'del LF before empty (1)' '\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 69f11bd..7b56081 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -161,10 +161,7 @@ test_expect_success \"Michael Cassar's test case\" '\n \tgit mv papers/unsorted/Thesis.pdf papers/all-papers/moo-blah.pdf &&\n \n \tT=`git write-tree` &&\n-\tgit ls-tree -r $T | grep partA/outline.txt || {\n-\t\tgit ls-tree -r $T\n-\t\t(exit 1)\n-\t}\n+\tgit ls-tree -r $T | verbose grep partA/outline.txt\n '\n \n rm -fr papers partA path?\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 04118ad..99be5d9 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -119,10 +119,7 @@ test_expect_success C_LOCALE_OUTPUT 'git clean with relative prefix' '\n \t\tgit clean -n ../src |\n \t\tsed -n -e \"s|^Would remove ||p\"\n \t) &&\n-\ttest \"$would_clean\" = ../src/part3.c || {\n-\t\techo \"OOps <$would_clean>\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$would_clean\" = ../src/part3.c\n '\n \n test_expect_success C_LOCALE_OUTPUT 'git clean with absolute path' '\n@@ -134,10 +131,7 @@ test_expect_success C_LOCALE_OUTPUT 'git clean with absolute path' '\n \t\tgit clean -n \"$(pwd)/../src\" |\n \t\tsed -n -e \"s|^Would remove ||p\"\n \t) &&\n-\ttest \"$would_clean\" = ../src/part3.c || {\n-\t\techo \"OOps <$would_clean>\"\n-\t\tfalse\n-\t}\n+\tverbose test \"$would_clean\" = ../src/part3.c\n '\n \n test_expect_success 'git clean with out of work tree relative path' '\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258104","messageId":"20150320100922.GG12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:09:22Z","receivedAt":"2015-03-20T10:09:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These test scripts likely predate test_must_fail, and can be\nmade simpler by using it (in addition to making them pass\n--chain-lint).\n\nThe case in t6036 loses some verbosity in the failure case,\nbut it is so tied to a specific failure mode that it is not\nworth keeping around (and the outcome of the test is not\naffected at all).\n\nSigned-off-by: Jeff King \n---\n t/t4124-apply-ws-rule.sh | 5 ++---\n t/t6036-recursive-corner-cases.sh | 7 +------\n t/t9300-fast-import.sh | 7 ++-----\n 3 files changed, 5 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex c6474de..d350065 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -99,9 +99,8 @@ test_expect_success 'whitespace=warn, default rule' '\n \n test_expect_success 'whitespace=error-all, default rule' '\n \n-\tapply_patch --whitespace=error-all && return 1\n-\ttest -s target && return 1\n-\t: happy\n+\ttest_must_fail apply_patch --whitespace=error-all &&\n+\t! test -s target\n \n '\n \ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex 8923b04..9d6621c 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -195,12 +195,7 @@ test_expect_success 'git detects differently handled merges conflict' '\n \tgit reset --hard &&\n \tgit checkout D^0 &&\n \n-\tgit merge -s recursive E^0 && {\n-\t\techo \"BAD: should have conflicted\"\n-\t\ttest \"Incorrectly merged content\" = \"$(cat new_a)\" &&\n-\t\t\techo \"BAD: Silently accepted wrong content\"\n-\t\treturn 1\n-\t}\n+\ttest_must_fail git merge -s recursive E^0 &&\n \n \ttest 3 = $(git ls-files -s | wc -l) &&\n \ttest 3 = $(git ls-files -u | wc -l) &&\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 584b538..aac126f 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2853,8 +2853,8 @@ test_expect_success 'S: notemodify with garbage after mark commit-ish must fail'\n # from\n #\n test_expect_success 'S: from with garbage after mark must fail' '\n-\t# no &&\n-\tgit fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err\n+\ttest_must_fail \\\n+\tgit fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err &&\n \tcommit refs/heads/S2\n \tmark :303\n \tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n@@ -2865,9 +2865,6 @@ test_expect_success 'S: from with garbage after mark must fail' '\n \tM 100644 :403 hello.c\n \tEOF\n \n-\tret=$? &&\n-\techo returned $ret &&\n-\ttest $ret -ne 0 && # failed, but it created the commit\n \n \t# go create the commit, need it for merge test\n \tgit fast-import --import-marks=marks --export-marks=marks <<-EOF &&\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258105","messageId":"20150320101021.GH12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 08/25] t: fix &&-chaining issues around setup which might fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:10:21Z","receivedAt":"2015-03-20T10:10:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many tests have an initial setup step that might fail based\non whether earlier tests in the script have succeeded or\nnot. Using a trick like \"|| true\" breaks the &&-chain,\nmissing earlier failures (and fooling --chain-lint).\n\nWe can use test_might_fail in some cases, which is correct\nand makes the intent more obvious. We can also use\ntest_unconfig for unsetting config (and which is more\nrobust, as well).\n\nThe case in t9500 is an oddball. It wants to run cmd1 _or_\ncmd2, and does it like:\n\n cmd1 || cmd2 &&\n other_stuff\n\nIt's not wrong in this case, but it's a bad habit to get\ninto, because it breaks the &&-chain if used anywhere except\nat the beginning of the test (and we use the correct\nsolution here, putting it inside a block for precedence).\n\nSigned-off-by: Jeff King \n---\n t/t5503-tagfollow.sh | 4 ++--\n t/t6032-merge-large-rename.sh | 6 +++---\n t/t7201-co.sh | 2 +-\n t/t7508-status.sh | 2 +-\n t/t9500-gitweb-standalone-no-errors.sh | 5 ++++-\n 5 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh\nindex f30c038..4ca48f0 100755\n--- a/t/t5503-tagfollow.sh\n+++ b/t/t5503-tagfollow.sh\n@@ -139,8 +139,8 @@ EOF\n '\n \n test_expect_success 'new clone fetch master and tags' '\n-\tgit branch -D cat\n-\trm -f $U\n+\ttest_might_fail git branch -D cat &&\n+\trm -f $U &&\n \t(\n \t\tmkdir clone2 &&\n \t\tcd clone2 &&\ndiff --git a/t/t6032-merge-large-rename.sh b/t/t6032-merge-large-rename.sh\nindex 15beecc..0f79268 100755\n--- a/t/t6032-merge-large-rename.sh\n+++ b/t/t6032-merge-large-rename.sh\n@@ -28,10 +28,10 @@ make_text() {\n \n test_rename() {\n \ttest_expect_success \"rename ($1, $2)\" '\n-\tn='$1'\n-\texpect='$2'\n+\tn='$1' &&\n+\texpect='$2' &&\n \tgit checkout -f master &&\n-\tgit branch -D test$n || true &&\n+\ttest_might_fail git branch -D test$n &&\n \tgit reset --hard initial &&\n \tfor i in $(count $n); do\n \t\tmake_text $i initial initial >$i\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex eae9e5a..a7fe4e6 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -591,7 +591,7 @@ test_expect_success 'checkout --conflict=merge, overriding config' '\n '\n \n test_expect_success 'checkout --conflict=diff3' '\n-\tgit config --unset merge.conflictstyle\n+\ttest_unconfig merge.conflictstyle &&\n \tsetup_conflicting_index &&\n \techo \"none of the above\" >sample &&\n \techo ourside >expect &&\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 0f9ad4c..c3ed7cb 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -538,7 +538,7 @@ A dir2/added\n ?? untracked\n EOF\n test_expect_success 'status -s -uall' '\n-\tgit config --unset status.showuntrackedfiles\n+\ttest_unconfig status.showuntrackedfiles &&\n \tgit status -s -uall >output &&\n \ttest_cmp expect output\n '\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex f9f078e..e94b2f1 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -779,7 +779,10 @@ test_expect_success \\\n \n test_expect_success \\\n \t'unborn HEAD: \"summary\" page (with \"heads\" subview)' \\\n-\t'git checkout orphan_branch || git checkout --orphan orphan_branch &&\n+\t'{\n+\t\tgit checkout orphan_branch ||\n+\t\tgit checkout --orphan orphan_branch\n+\t } &&\n \t test_when_finished \"git checkout master\" &&\n \t gitweb_run \"p=.git;a=summary\"'\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258106","messageId":"20150320101132.GI12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 09/25] t: use test_might_fail for diff and grep","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:11:32Z","receivedAt":"2015-03-20T10:11:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some tests run diff or grep to produce an output, and then\ncompare the output to an expected value. We know the exit\ncode we expect these processes to have (e.g., grep yields 0\nif it produced output and 1 otherwise), so it would not make\nthe test wrong to look for it. But the difference between\ntheir output and the expected output (e.g., shown by\ntest_cmp) is much more useful to somebody debugging the test\nthan the test just bailing out.\n\nThese tests break the &&-chain to skip the exit-code check\nof the process. However, we can get the same effect by using\ntest_might_fail. Note that in some cases the test did use\n\"|| return 1\", which meant the test was not wrong, but it\ndid fool --chain-lint.\n\nSigned-off-by: Jeff King \n---\n t/t1002-read-tree-m-u-2way.sh | 8 ++++----\n t/t5550-http-fetch-dumb.sh | 4 ++--\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex fed877b..e3bf821 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -75,8 +75,8 @@ test_expect_success \\\n echo yomin >yomin &&\n git update-index --add yomin &&\n read_tree_u_must_succeed -m -u $treeH $treeM &&\n- git ls-files --stage >4.out || return 1\n- git diff -U0 --no-index M.out 4.out >4diff.out\n+ git ls-files --stage >4.out &&\n+ test_might_fail git diff -U0 --no-index M.out 4.out >4diff.out &&\n compare_change 4diff.out expected &&\n check_cache_at yomin clean &&\n sum bozbar frotz nitfol >actual4.sum &&\n@@ -94,8 +94,8 @@ test_expect_success \\\n git update-index --add yomin &&\n echo yomin yomin >yomin &&\n read_tree_u_must_succeed -m -u $treeH $treeM &&\n- git ls-files --stage >5.out || return 1\n- git diff -U0 --no-index M.out 5.out >5diff.out\n+ git ls-files --stage >5.out &&\n+ test_might_fail git diff -U0 --no-index M.out 5.out >5diff.out &&\n compare_change 5diff.out expected &&\n check_cache_at yomin dirty &&\n sum bozbar frotz nitfol >actual5.sum &&\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 61e087e..3d11b7a 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -184,8 +184,8 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '\n '\n \n test_expect_success 'did not use upload-pack service' '\n-\tgrep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act\n-\t: >exp\n+\ttest_might_fail grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act &&\n+\t: >exp &&\n \ttest_cmp exp act\n '\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258107","messageId":"20150320101146.GJ12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:11:46Z","receivedAt":"2015-03-20T10:11:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This makes our output in the event of a failure slightly\nnicer, and it means that we do not break the &&-chain.\n\nSigned-off-by: Jeff King \n---\n t/t0040-parse-options.sh | 12 +++------\n t/t4035-diff-quiet.sh | 66 +++++++++++++++++++++++-------------------------\n t/t4053-diff-no-index.sh | 4 +--\n 3 files changed, 38 insertions(+), 44 deletions(-)\n\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex a90c86b..b044785 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -172,12 +172,9 @@ test_expect_success 'long options' '\n '\n \n test_expect_success 'missing required value' '\n-\ttest-parse-options -s;\n-\ttest $? = 129 &&\n-\ttest-parse-options --string;\n-\ttest $? = 129 &&\n-\ttest-parse-options --file;\n-\ttest $? = 129\n+\ttest_expect_code 129 test-parse-options -s &&\n+\ttest_expect_code 129 test-parse-options --string &&\n+\ttest_expect_code 129 test-parse-options --file\n '\n \n cat > expect << EOF\n@@ -227,8 +224,7 @@ test_expect_success 'unambiguously abbreviated option with \"=\"' '\n '\n \n test_expect_success 'ambiguously abbreviated option' '\n-\ttest-parse-options --strin 123;\n-\ttest $? = 129\n+\ttest_expect_code 129 test-parse-options --strin 123\n '\n \n cat > expect << EOF\ndiff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh\nindex e8ae2a0..461f4bb 100755\n--- a/t/t4035-diff-quiet.sh\n+++ b/t/t4035-diff-quiet.sh\n@@ -29,67 +29,65 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'git diff-tree HEAD^ HEAD' '\n-\tgit diff-tree --quiet HEAD^ HEAD >cnt\n-\ttest $? = 1 && test_line_count = 0 cnt\n+\ttest_expect_code 1 git diff-tree --quiet HEAD^ HEAD >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-tree HEAD^ HEAD -- a' '\n-\tgit diff-tree --quiet HEAD^ HEAD -- a >cnt\n-\ttest $? = 0 && test_line_count = 0 cnt\n+\ttest_expect_code 0 git diff-tree --quiet HEAD^ HEAD -- a >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-tree HEAD^ HEAD -- b' '\n-\tgit diff-tree --quiet HEAD^ HEAD -- b >cnt\n-\ttest $? = 1 && test_line_count = 0 cnt\n+\ttest_expect_code 1 git diff-tree --quiet HEAD^ HEAD -- b >cnt &&\n+\ttest_line_count = 0 cnt\n '\n # this diff outputs one line: sha1 of the given head\n test_expect_success 'echo HEAD | git diff-tree --stdin' '\n-\techo $(git rev-parse HEAD) | git diff-tree --quiet --stdin >cnt\n-\ttest $? = 1 && test_line_count = 1 cnt\n+\techo $(git rev-parse HEAD) |\n+\ttest_expect_code 1 git diff-tree --quiet --stdin >cnt &&\n+\ttest_line_count = 1 cnt\n '\n test_expect_success 'git diff-tree HEAD HEAD' '\n-\tgit diff-tree --quiet HEAD HEAD >cnt\n-\ttest $? = 0 && test_line_count = 0 cnt\n+\ttest_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-files' '\n-\tgit diff-files --quiet >cnt\n-\ttest $? = 0 && test_line_count = 0 cnt\n+\ttest_expect_code 0 git diff-files --quiet >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-index --cached HEAD' '\n-\tgit diff-index --quiet --cached HEAD >cnt\n-\ttest $? = 0 && test_line_count = 0 cnt\n+\ttest_expect_code 0 git diff-index --quiet --cached HEAD >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-index --cached HEAD^' '\n-\tgit diff-index --quiet --cached HEAD^ >cnt\n-\ttest $? = 1 && test_line_count = 0 cnt\n+\ttest_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-index --cached HEAD^' '\n \techo text >>b &&\n \techo 3 >c &&\n-\tgit add . && {\n-\t\tgit diff-index --quiet --cached HEAD^ >cnt\n-\t\ttest $? = 1 && test_line_count = 0 cnt\n-\t}\n+\tgit add . &&\n+\ttest_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-tree -Stext HEAD^ HEAD -- b' '\n-\tgit commit -m \"text in b\" && {\n-\t\tgit diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt\n-\t\ttest $? = 1 && test_line_count = 0 cnt\n-\t}\n+\tgit commit -m \"text in b\" &&\n+\ttest_expect_code 1 git diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-tree -Snot-found HEAD^ HEAD -- b' '\n-\tgit diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt\n-\ttest $? = 0 && test_line_count = 0 cnt\n+\ttest_expect_code 0 git diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt &&\n+\ttest_line_count = 0 cnt\n '\n test_expect_success 'git diff-files' '\n-\techo 3 >>c && {\n-\t\tgit diff-files --quiet >cnt\n-\t\ttest $? = 1 && test_line_count = 0 cnt\n-\t}\n+\techo 3 >>c &&\n+\ttest_expect_code 1 git diff-files --quiet >cnt &&\n+\ttest_line_count = 0 cnt\n '\n+\n test_expect_success 'git diff-index --cached HEAD' '\n-\tgit update-index c && {\n-\t\tgit diff-index --quiet --cached HEAD >cnt\n-\t\ttest $? = 1 && test_line_count = 0 cnt\n-\t}\n+\tgit update-index c &&\n+\ttest_expect_code 1 git diff-index --quiet --cached HEAD >cnt &&\n+\ttest_line_count = 0 cnt\n '\n \n test_expect_success 'git diff, one file outside repo' '\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 2ab3c48..075ece6 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -17,8 +17,8 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'git diff --no-index directories' '\n-\tgit diff --no-index a b >cnt\n-\ttest $? = 1 && test_line_count = 14 cnt\n+\ttest_expect_code 1 git diff --no-index a b >cnt &&\n+\ttest_line_count = 14 cnt\n '\n \n test_expect_success 'git diff --no-index relative path outside repo' '\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258108","messageId":"20150320101229.GK12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 11/25] t: wrap complicated expect_code users in a block","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:12:29Z","receivedAt":"2015-03-20T10:12:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we are expecting a command to produce a particular exit\ncode, we can use test_expect_code. However, some cases are\nmore complicated, and want to accept one of a range of exit\ncodes. For these, we end up with something like:\n\n cmd;\n case \"$?\" in\n ...\n\nThat unfortunately breaks the &&-chain and fools\n--chain-lint. Since these special cases are so few, we can\nwrap them in a block, like this:\n\n { cmd; ret=$?; } &&\n case \"$ret\" in\n ...\n\nThis accomplishes the same thing, and retains the &&-chain\n(the exit status fed to the && is that of the assignment,\nwhich should always be true). It's technically longer, but\nit is probably a good thing for unusual code like this to\nstand out.\n\nSigned-off-by: Jeff King \n---\n t/t0005-signals.sh | 4 ++--\n t/t4026-color.sh | 6 +++---\n t/t5004-archive-corner-cases.sh | 6 ++++--\n t/t5512-ls-remote.sh | 6 ++++--\n 4 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\nindex aeea50c..5c5707d 100755\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -10,8 +10,8 @@ one\n EOF\n \n test_expect_success 'sigchain works' '\n-\ttest-sigchain >actual\n-\tcase \"$?\" in\n+\t{ test-sigchain >actual; ret=$?; } &&\n+\tcase \"$ret\" in\n \t143) true ;; # POSIX w/ SIGTERM=15\n \t271) true ;; # ksh w/ SIGTERM=15\n \t 3) true ;; # Windows\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex 4d20fea..2b32c4f 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -111,9 +111,9 @@ test_expect_success 'unknown color slots are ignored (branch)' '\n '\n \n test_expect_success 'unknown color slots are ignored (status)' '\n-\tgit config color.status.nosuchslotwilleverbedefined white || exit\n-\tgit status\n-\tcase $? in 0|1) : ok ;; *) false ;; esac\n+\tgit config color.status.nosuchslotwilleverbedefined white &&\n+\t{ git status; ret=$?; } &&\n+\tcase $ret in 0|1) : ok ;; *) false ;; esac\n '\n \n test_done\ndiff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh\nindex 305bcac..654adda 100755\n--- a/t/t5004-archive-corner-cases.sh\n+++ b/t/t5004-archive-corner-cases.sh\n@@ -66,8 +66,10 @@ test_expect_success UNZIP 'zip archive of empty tree is empty' '\n \t# handle the empty repo at all, making our later check of its exit code\n \t# a no-op). But we cannot do anything reasonable except skip the test\n \t# on such platforms anyway, and this is the moral equivalent.\n-\t\"$GIT_UNZIP\" \"$TEST_DIRECTORY\"/t5004/empty.zip\n-\texpect_code=$?\n+\t{\n+\t\t\"$GIT_UNZIP\" \"$TEST_DIRECTORY\"/t5004/empty.zip\n+\t\texpect_code=$?\n+\t} &&\n \n \tgit archive --format=zip HEAD >empty.zip &&\n \tmake_dir extract &&\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 321c3e5..3bd9759 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -103,8 +103,10 @@ test_expect_success 'confuses pattern as remote when no remote specified' '\n '\n \n test_expect_success 'die with non-2 for wrong repository even with --exit-code' '\n-\tgit ls-remote --exit-code ./no-such-repository ;# not &&\n-\tstatus=$? &&\n+\t{\n+\t\tgit ls-remote --exit-code ./no-such-repository\n+\t\tstatus=$?\n+\t} &&\n \ttest $status != 2 && test $status != 0\n '\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258109","messageId":"20150320101237.GL12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 12/25] t: avoid using \":\" for comments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:12:37Z","receivedAt":"2015-03-20T10:12:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \":\" is not a comment marker, but rather a noop command.\nUsing it as a comment like:\n\n : do something\n cmd1 &&\n\n : something else\n cmd2\n\nbreaks the &&-chain, and we would fail to notice if \"cmd1\"\nfailed in this instance. We can just use regular \"#\"\ncomments instead.\n\nSigned-off-by: Jeff King \n---\n t/t4104-apply-boundary.sh | 18 +++++++++---------\n t/t5533-push-cas.sh | 2 +-\n 2 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh\nindex c97aad1..497afdc 100755\n--- a/t/t4104-apply-boundary.sh\n+++ b/t/t4104-apply-boundary.sh\n@@ -18,7 +18,7 @@ test_expect_success setup '\n \tcat victim >original &&\n \tgit update-index --add victim &&\n \n-\t: add to the head\n+\t# add to the head\n \tfor i in a b '\"$L\"' y\n \tdo\n \t\techo $i\n@@ -27,7 +27,7 @@ test_expect_success setup '\n \tgit diff victim >add-a-patch.with &&\n \tgit diff --unified=0 >add-a-patch.without &&\n \n-\t: insert at line two\n+\t# insert at line two\n \tfor i in b a '\"$L\"' y\n \tdo\n \t\techo $i\n@@ -36,7 +36,7 @@ test_expect_success setup '\n \tgit diff victim >insert-a-patch.with &&\n \tgit diff --unified=0 >insert-a-patch.without &&\n \n-\t: modify at the head\n+\t# modify at the head\n \tfor i in a '\"$L\"' y\n \tdo\n \t\techo $i\n@@ -45,16 +45,16 @@ test_expect_success setup '\n \tgit diff victim >mod-a-patch.with &&\n \tgit diff --unified=0 >mod-a-patch.without &&\n \n-\t: remove from the head\n+\t# remove from the head\n \tfor i in '\"$L\"' y\n \tdo\n \t\techo $i\n \tdone >victim &&\n \tcat victim >del-a-expect &&\n-\tgit diff victim >del-a-patch.with\n+\tgit diff victim >del-a-patch.with &&\n \tgit diff --unified=0 >del-a-patch.without &&\n \n-\t: add to the tail\n+\t# add to the tail\n \tfor i in b '\"$L\"' y z\n \tdo\n \t\techo $i\n@@ -63,7 +63,7 @@ test_expect_success setup '\n \tgit diff victim >add-z-patch.with &&\n \tgit diff --unified=0 >add-z-patch.without &&\n \n-\t: modify at the tail\n+\t# modify at the tail\n \tfor i in b '\"$L\"' z\n \tdo\n \t\techo $i\n@@ -72,7 +72,7 @@ test_expect_success setup '\n \tgit diff victim >mod-z-patch.with &&\n \tgit diff --unified=0 >mod-z-patch.without &&\n \n-\t: remove from the tail\n+\t# remove from the tail\n \tfor i in b '\"$L\"'\n \tdo\n \t\techo $i\n@@ -81,7 +81,7 @@ test_expect_success setup '\n \tgit diff victim >del-z-patch.with &&\n \tgit diff --unified=0 >del-z-patch.without\n \n-\t: done\n+\t# done\n '\n \n for with in with without\ndiff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh\nindex dccf8a6..c402d8d 100755\n--- a/t/t5533-push-cas.sh\n+++ b/t/t5533-push-cas.sh\n@@ -14,7 +14,7 @@ setup_srcdst_basic () {\n }\n \n test_expect_success setup '\n-\t: create template repository\n+\t# create template repository\n \ttest_commit A &&\n \ttest_commit B &&\n \ttest_commit C\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258110","messageId":"20150320101251.GM12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 13/25] t3600: fix &&-chain breakage for setup commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:12:51Z","receivedAt":"2015-03-20T10:12:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"As with the earlier patch to fix \"trivial\" &&-chain\nbreakage, these missing \"&&\" operators are not a serious\nproblem (e.g., we do not expect \"echo\" to fail).\n\nIronically, however, inserting them shows that some of the\ncommands _do_ fail. Specifically, some of the tests start by\nmaking sure we are at a commit with the string \"content\" in\nthe file \"foo\". However, running \"git commit\" may fail\nbecause the previous test left us in that state already, and\nthere is nothing to commit.\n\nWe could remove these commands entirely, but they serve to\ndocument the test's assumptions, as well as make it robust\nwhen an earlier test has failed. We could use test_might_fail\nto handle all cases, but that would miss an unrelated\nfailure to make the commit. Instead, we can just pass the\n--allow-empty flag to git-commit, which means that it will\nnot complain if our setup is a noop.\n\nSigned-off-by: Jeff King \n---\n t/t3600-rm.sh | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 1962989..9d90d2c 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -38,37 +38,37 @@ test_expect_success \\\n \n test_expect_success \\\n 'Test that git rm --cached foo succeeds if the index matches the file' \\\n- 'echo content > foo\n- git add foo\n+ 'echo content >foo &&\n+ git add foo &&\n git rm --cached foo'\n \n test_expect_success \\\n 'Test that git rm --cached foo succeeds if the index matches the file' \\\n- 'echo content > foo\n- git add foo\n- git commit -m foo\n- echo \"other content\" > foo\n+ 'echo content >foo &&\n+ git add foo &&\n+ git commit -m foo &&\n+ echo \"other content\" >foo &&\n git rm --cached foo'\n \n test_expect_success \\\n 'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' '\n- echo content > foo\n- git add foo\n- git commit -m foo\n- echo \"other content\" > foo\n- git add foo\n- echo \"yet another content\" > foo\n+ echo content >foo &&\n+ git add foo &&\n+ git commit -m foo --allow-empty &&\n+ echo \"other content\" >foo &&\n+ git add foo &&\n+ echo \"yet another content\" >foo &&\n test_must_fail git rm --cached foo\n '\n \n test_expect_success \\\n 'Test that git rm --cached -f foo works in case where --cached only did not' \\\n- 'echo content > foo\n- git add foo\n- git commit -m foo\n- echo \"other content\" > foo\n- git add foo\n- echo \"yet another content\" > foo\n+ 'echo content >foo &&\n+ git add foo &&\n+ git commit -m foo --allow-empty &&\n+ echo \"other content\" >foo &&\n+ git add foo &&\n+ echo \"yet another content\" >foo &&\n git rm --cached -f foo'\n \n test_expect_success \\\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258111","messageId":"20150320101255.GN12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 14/25] t7201: fix &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:12:55Z","receivedAt":"2015-03-20T10:12:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"One of these breakages is in setup, but one is more severe\nand may miss a real test failure. These are pulled out from\nthe rest, though, because we also clean up a few other\nanachronisms. The most interesting is the use of this\nhere-doc construct:\n\n (cat >... <\n---\n t/t7201-co.sh | 19 ++++++-------------\n 1 file changed, 6 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex a7fe4e6..8859236 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -88,14 +88,10 @@ test_expect_success \"checkout with unrelated dirty tree without -m\" '\n \n \tgit checkout -f master &&\n \tfill 0 1 2 3 4 5 6 7 8 >same &&\n-\tcp same kept\n+\tcp same kept &&\n \tgit checkout side >messages &&\n-\ttest_cmp same kept\n-\t(cat > messages.expect <messages.expect &&\n \ttest_cmp messages.expect messages\n '\n \n@@ -109,10 +105,7 @@ test_expect_success \"checkout -m with dirty tree\" '\n \n \ttest \"$(git symbolic-ref HEAD)\" = \"refs/heads/side\" &&\n \n-\t(cat >expect.messages <expect.messages &&\n \ttest_cmp expect.messages messages &&\n \n \tfill \"M\tone\" \"A\tthree\" \"D\ttwo\" >expect.master &&\n@@ -409,12 +402,12 @@ test_expect_success \\\n \n test_expect_success \\\n 'checkout w/autosetupmerge=always sets up tracking' '\n+ test_when_finished git config branch.autosetupmerge false &&\n git config branch.autosetupmerge always &&\n git checkout master &&\n git checkout -b track2 &&\n test \"$(git config branch.track2.remote)\" &&\n- test \"$(git config branch.track2.merge)\"\n- git config branch.autosetupmerge false'\n+ test \"$(git config branch.track2.merge)\"'\n \n test_expect_success 'checkout w/--track from non-branch HEAD fails' '\n git checkout master^0 &&\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258112","messageId":"20150320101301.GO12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 15/25] t9502: fix &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:01Z","receivedAt":"2015-03-20T10:13:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This script misses a trivial &&-chain in one of its tests,\nbut it also has a weird reverse: it includes an &&-chain\noutside of any test_expect block! This \"cat\" should never\nfail, but if it did, we would not notice, as it would cause\nus to skip the follow-on test entirely (which does not\nappear intentional; there are many later tests which rely on\nthis cat).\n\nLet's instead move the setup into its own test_expect_success\nblock, which is the standard practice nowadays.\n\nSigned-off-by: Jeff King \n---\n t/t9502-gitweb-standalone-parse-output.sh | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex 86dfee2..a93e159 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -145,9 +145,11 @@ test_expect_success 'forks: not skipped unless \"forks\" feature enabled' '\n \tgrep -q \">fork of .*<\" gitweb.body\n '\n \n-cat >>gitweb_config.perl <<\\EOF &&\n-$feature{'forks'}{'default'} = [1];\n-EOF\n+test_expect_success 'enable forks feature' '\n+\tcat >>gitweb_config.perl <<-\\EOF\n+\t$feature{'forks'}{'default'} = [1];\n+\tEOF\n+'\n \n test_expect_success 'forks: forks skipped if \"forks\" feature enabled' '\n \tgitweb_run \"a=project_list\" &&\n@@ -173,7 +175,7 @@ test_expect_success 'forks: can access forked repository' '\n '\n \n test_expect_success 'forks: project_index lists all projects (incl. forks)' '\n-\tcat >expected <<-\\EOF\n+\tcat >expected <<-\\EOF &&\n \t.git\n \tfoo.bar.git\n \tfoo.git\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258113","messageId":"20150320101304.GP12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 16/25] t6030: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:05Z","receivedAt":"2015-03-20T10:13:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can get rid of a lot of hand-rolled error messages by\nusing test_must_fail and test_expect_code. The existing code\nwas careful to use \"|| return 1\" when breaking the\n&&-chain, but it did fool --chain-lint; the new code is more\nidiomatic.\n\nWe also add some uses of test_when_finished, which is less\ncryptic and more robust than putting code at the end of a\ntest. In two cases we run \"git bisect reset\" from a\nsubshell, which is a problem for test_when_finished (it\nwould not run). However, in both of these cases, we are\nperforming the tests in one-off sub-repos, so we do not need\nto clean up at all (and in fact it is nicer not to if the\nuser wants to inspect the trash directory after a failure).\n\nSigned-off-by: Jeff King \n---\n t/t6030-bisect-porcelain.sh | 91 +++++++++++++++------------------------------\n 1 file changed, 31 insertions(+), 60 deletions(-)\n\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex e6abe65..06b4868 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -52,15 +52,8 @@ test_expect_success 'bisect starts with only one bad' '\n test_expect_success 'bisect does not start with only one good' '\n \tgit bisect reset &&\n \tgit bisect start &&\n-\tgit bisect good $HASH1 || return 1\n-\n-\tif git bisect next\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\t:\n-\tfi\n+\tgit bisect good $HASH1 &&\n+\ttest_must_fail git bisect next\n '\n \n test_expect_success 'bisect start with one bad and good' '\n@@ -191,34 +184,27 @@ test_expect_success 'bisect start: no \".git/BISECT_START\" if checkout error' '\n # but $HASH2 is bad,\n # so we should find $HASH2 as the first bad commit\n test_expect_success 'bisect skip: successful result' '\n+\ttest_when_finished git bisect reset &&\n \tgit bisect reset &&\n \tgit bisect start $HASH4 $HASH1 &&\n \tgit bisect skip &&\n \tgit bisect bad > my_bisect_log.txt &&\n-\tgrep \"$HASH2 is the first bad commit\" my_bisect_log.txt &&\n-\tgit bisect reset\n+\tgrep \"$HASH2 is the first bad commit\" my_bisect_log.txt\n '\n \n # $HASH1 is good, $HASH4 is bad, we skip $HASH3 and $HASH2\n # so we should not be able to tell the first bad commit\n # among $HASH2, $HASH3 and $HASH4\n test_expect_success 'bisect skip: cannot tell between 3 commits' '\n+\ttest_when_finished git bisect reset &&\n \tgit bisect start $HASH4 $HASH1 &&\n-\tgit bisect skip || return 1\n-\n-\tif git bisect skip > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH1 my_bisect_log.txt &&\n-\t\tgrep $HASH2 my_bisect_log.txt &&\n-\t\tgrep $HASH3 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgit bisect reset\n-\tfi\n+\tgit bisect skip &&\n+\ttest_expect_code 2 git bisect skip >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH1 my_bisect_log.txt &&\n+\tgrep $HASH2 my_bisect_log.txt &&\n+\tgrep $HASH3 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt\n '\n \n # $HASH1 is good, $HASH4 is bad, we skip $HASH3\n@@ -226,22 +212,15 @@ test_expect_success 'bisect skip: cannot tell between 3 commits' '\n # so we should not be able to tell the first bad commit\n # among $HASH3 and $HASH4\n test_expect_success 'bisect skip: cannot tell between 2 commits' '\n+\ttest_when_finished git bisect reset &&\n \tgit bisect start $HASH4 $HASH1 &&\n-\tgit bisect skip || return 1\n-\n-\tif git bisect good > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH1 my_bisect_log.txt &&\n-\t\t! grep $HASH2 my_bisect_log.txt &&\n-\t\tgrep $HASH3 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgit bisect reset\n-\tfi\n+\tgit bisect skip &&\n+\ttest_expect_code 2 git bisect good >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH1 my_bisect_log.txt &&\n+\t! grep $HASH2 my_bisect_log.txt &&\n+\tgrep $HASH3 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt\n '\n \n # $HASH1 is good, $HASH4 is both skipped and bad, we skip $HASH3\n@@ -249,24 +228,18 @@ test_expect_success 'bisect skip: cannot tell between 2 commits' '\n # so we should not be able to tell the first bad commit\n # among $HASH3 and $HASH4\n test_expect_success 'bisect skip: with commit both bad and skipped' '\n+\ttest_when_finished git bisect reset &&\n \tgit bisect start &&\n \tgit bisect skip &&\n \tgit bisect bad &&\n \tgit bisect good $HASH1 &&\n \tgit bisect skip &&\n-\tif git bisect good > my_bisect_log.txt\n-\tthen\n-\t\techo Oops, should have failed.\n-\t\tfalse\n-\telse\n-\t\ttest $? -eq 2 &&\n-\t\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n-\t\t! grep $HASH1 my_bisect_log.txt &&\n-\t\t! grep $HASH2 my_bisect_log.txt &&\n-\t\tgrep $HASH3 my_bisect_log.txt &&\n-\t\tgrep $HASH4 my_bisect_log.txt &&\n-\t\tgit bisect reset\n-\tfi\n+\ttest_expect_code 2 git bisect good >my_bisect_log.txt &&\n+\tgrep \"first bad commit could be any of\" my_bisect_log.txt &&\n+\t! grep $HASH1 my_bisect_log.txt &&\n+\t! grep $HASH2 my_bisect_log.txt &&\n+\tgrep $HASH3 my_bisect_log.txt &&\n+\tgrep $HASH4 my_bisect_log.txt\n '\n \n # We want to automatically find the commit that\n@@ -601,8 +574,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout specified' '\n \t\tgit bisect bad $HASH4 &&\n \t\tgit bisect run eval \\\n \t\t\t\"test \\$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0\" \\\n-\t\t\t>../nocheckout.log &&\n-\t\tgit bisect reset\n+\t\t\t>../nocheckout.log\n \t) &&\n \tgrep \"$HASH3 is the first bad commit\" nocheckout.log\n '\n@@ -617,8 +589,7 @@ test_expect_success 'test bisection on bare repo - --no-checkout defaulted' '\n \t\tgit bisect bad $HASH4 &&\n \t\tgit bisect run eval \\\n \t\t\t\"test \\$(git rev-list BISECT_HEAD ^$HASH2 --max-count=1 | wc -l) = 0\" \\\n-\t\t\t>../defaulted.log &&\n-\t\tgit bisect reset\n+\t\t\t>../defaulted.log\n \t) &&\n \tgrep \"$HASH3 is the first bad commit\" defaulted.log\n '\n@@ -642,14 +613,14 @@ test_expect_success 'broken branch creation' '\n \tmkdir missing &&\n \t:> missing/MISSING &&\n \tgit add missing/MISSING &&\n-\tgit commit -m \"6(broken): Added file that will be deleted\"\n+\tgit commit -m \"6(broken): Added file that will be deleted\" &&\n \tgit tag BROKEN_HASH6 &&\n \tadd_line_into_file \"7(broken): second line on a broken branch\" hello2 &&\n \tgit tag BROKEN_HASH7 &&\n \tadd_line_into_file \"8(broken): third line on a broken branch\" hello2 &&\n \tgit tag BROKEN_HASH8 &&\n \tgit rm missing/MISSING &&\n-\tgit commit -m \"9(broken): Remove missing file\"\n+\tgit commit -m \"9(broken): Remove missing file\" &&\n \tgit tag BROKEN_HASH9 &&\n \trm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d\n '\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258116","messageId":"20150320101308.GQ12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 17/25] t0020: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:08Z","receivedAt":"2015-03-20T10:13:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This test contains a lot of hand-rolled messages to show\nwhen the test fails. We can omit most of these by using\n\"verbose\" and \"test_must_fail\". A few of them are for\nupdate-index, but we can assume it produces reasonable error\nmessages when it fails.\n\nSigned-off-by: Jeff King \n---\n t/t0020-crlf.sh | 144 +++++++++++---------------------------------------------\n 1 file changed, 28 insertions(+), 116 deletions(-)\n\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex d2e51a8..9fa26df 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '\n \tfor f in one dir/two\n \tdo\n \t\tappend_cr <$f >tmp && mv -f tmp $f &&\n-\t\tgit update-index -- $f || {\n-\t\t\techo Oops\n-\t\t\tfalse\n-\t\t\tbreak\n-\t\t}\n+\t\tgit update-index -- $f ||\n+\t\tbreak\n \tdone &&\n \n \tdiffers=$(git diff-index --cached HEAD) &&\n-\ttest -z \"$differs\" || {\n-\t\techo Oops \"$differs\"\n-\t\tfalse\n-\t}\n+\tverbose test -z \"$differs\"\n \n '\n \n@@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '\n \tfor f in one dir/two\n \tdo\n \t\tappend_cr <$f >tmp && mv -f tmp $f &&\n-\t\tgit update-index -- $f || {\n-\t\t\techo \"Oops $f\"\n-\t\t\tfalse\n-\t\t\tbreak\n-\t\t}\n+\t\tgit update-index -- $f ||\n+\t\tbreak\n \tdone &&\n \n \tdiffers=$(git diff-index --cached HEAD) &&\n-\ttest -z \"$differs\" || {\n-\t\techo Oops \"$differs\"\n-\t\tfalse\n-\t}\n+\tverbose test -z \"$differs\"\n \n '\n \n@@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '\n \tfor f in one dir/two\n \tdo\n \t\tremove_cr <\"$f\" >tmp && mv -f tmp $f &&\n-\t\tgit update-index -- $f || {\n-\t\t\techo \"Eh? $f\"\n-\t\t\tfalse\n-\t\t\tbreak\n-\t\t}\n+\t\tverbose git update-index -- $f ||\n+\t\tbreak\n \tdone &&\n \ttest \"$one\" = $(git hash-object --stdin .gitattributes &&\n \tgit read-tree --reset -u HEAD &&\n \n-\tif has_cr dir/two\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Huh?\"\n-\t\tfalse\n-\tfi &&\n-\n-\tif has_cr three\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Huh?\"\n-\t\tfalse\n-\tfi\n+\tverbose has_cr dir/two &&\n+\tverbose has_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (1)' '\n@@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' '\n \trm -rf tmp one dir .gitattributes patch.file three &&\n \tgit read-tree --reset -u HEAD &&\n \n-\tif has_cr one\n-\tthen\n-\t\techo \"Eh? one should not have CRLF\"\n-\t\tfalse\n-\telse\n-\t\t: happy\n-\tfi &&\n-\thas_cr three || {\n-\t\techo \"Eh? three should still have CRLF\"\n-\t\tfalse\n-\t}\n+\ttest_must_fail has_cr one &&\n+\tverbose has_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (2)' '\n@@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' '\n \tgit read-tree --reset HEAD &&\n \tgit checkout-index -f -q -u -a &&\n \n-\tif has_cr one\n-\tthen\n-\t\techo \"Eh? one should not have CRLF\"\n-\t\tfalse\n-\telse\n-\t\t: happy\n-\tfi &&\n-\thas_cr three || {\n-\t\techo \"Eh? three should still have CRLF\"\n-\t\tfalse\n-\t}\n+\ttest_must_fail has_cr one &&\n+\tverbose has_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (3)' '\n@@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' '\n \tgit checkout-index -u .gitattributes &&\n \tgit checkout-index -u one dir/two three &&\n \n-\tif has_cr one\n-\tthen\n-\t\techo \"Eh? one should not have CRLF\"\n-\t\tfalse\n-\telse\n-\t\t: happy\n-\tfi &&\n-\thas_cr three || {\n-\t\techo \"Eh? three should still have CRLF\"\n-\t\tfalse\n-\t}\n+\ttest_must_fail has_cr one &&\n+\tverbose has_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (4)' '\n@@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' '\n \tgit checkout-index -u one dir/two three &&\n \tgit checkout-index -u .gitattributes &&\n \n-\tif has_cr one\n-\tthen\n-\t\techo \"Eh? one should not have CRLF\"\n-\t\tfalse\n-\telse\n-\t\t: happy\n-\tfi &&\n-\thas_cr three || {\n-\t\techo \"Eh? three should still have CRLF\"\n-\t\tfalse\n-\t}\n+\ttest_must_fail has_cr one &&\n+\tverbose has_cr three\n '\n \n test_expect_success 'checkout with existing .gitattributes' '\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258115","messageId":"20150320101311.GR12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 18/25] t1301: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:11Z","receivedAt":"2015-03-20T10:13:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This shortens the code and fixes some &&-chaining.\n\nSigned-off-by: Jeff King \n---\n t/t1301-shared-repo.sh | 20 +++++++-------------\n 1 file changed, 7 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 7eecfb8..ac10875 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -12,12 +12,11 @@ setfacl -k . 2>/dev/null\n \n # User must have read permissions to the repo -> failure on --shared=0400\n test_expect_success 'shared = 0400 (faulty permission u-w)' '\n+\ttest_when_finished \"rm -rf sub\" &&\n \tmkdir sub && (\n-\t\tcd sub && git init --shared=0400\n+\t\tcd sub &&\n+\t\ttest_must_fail git init --shared=0400\n \t)\n-\tret=\"$?\"\n-\trm -rf sub\n-\ttest $ret != \"0\"\n '\n \n modebits () {\n@@ -33,7 +32,7 @@ do\n \t\t\tgit init --shared=1 &&\n \t\t\ttest 1 = \"$(git config core.sharedrepository)\"\n \t\t) &&\n-\t\tactual=$(ls -l sub/.git/HEAD)\n+\t\tactual=$(ls -l sub/.git/HEAD) &&\n \t\tcase \"$actual\" in\n \t\t-rw-rw-r--*)\n \t\t\t: happy\n@@ -90,10 +89,8 @@ do\n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n \t\tactual=\"$(modebits .git/info/refs)\" &&\n-\t\ttest \"x$actual\" = \"x-$y\" || {\n-\t\t\tls -lt .git/info\n-\t\t\tfalse\n-\t\t}\n+\t\tverbose test \"x$actual\" = \"x-$y\"\n+\n \t'\n \n \tumask 077 &&\n@@ -102,10 +99,7 @@ do\n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n \t\tactual=\"$(modebits .git/info/refs)\" &&\n-\t\ttest \"x$actual\" = \"x-$x\" || {\n-\t\t\tls -lt .git/info\n-\t\t\tfalse\n-\t\t}\n+\t\tverbose test \"x$actual\" = \"x-$x\"\n \n \t'\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258118","messageId":"20150320101315.GS12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 19/25] t6034: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:15Z","receivedAt":"2015-03-20T10:13:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These say roughly the same thing as the hand-rolled\nmessages. We do lose the \"merge did not complete\" debug\nmessage, but merge and write-tree are prefectly capable of\nwriting useful error messages when they fail.\n\nSigned-off-by: Jeff King \n---\n t/t6034-merge-rename-nocruft.sh | 66 ++++++++---------------------------------\n 1 file changed, 13 insertions(+), 53 deletions(-)\n\ndiff --git a/t/t6034-merge-rename-nocruft.sh b/t/t6034-merge-rename-nocruft.sh\nindex 65be95f..89871aa 100755\n--- a/t/t6034-merge-rename-nocruft.sh\n+++ b/t/t6034-merge-rename-nocruft.sh\n@@ -73,33 +73,12 @@ test_expect_success 'merge white into red (A->B,M->N)' \\\n '\n \tgit checkout -b red-white red &&\n \tgit merge white &&\n-\tgit write-tree >/dev/null || {\n-\t\techo \"BAD: merge did not complete\"\n-\t\treturn 1\n-\t}\n-\n-\ttest -f B || {\n-\t\techo \"BAD: B does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest -f N || {\n-\t\techo \"BAD: N does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest -f R || {\n-\t\techo \"BAD: R does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\n-\ttest -f A && {\n-\t\techo \"BAD: A still exists in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest -f M && {\n-\t\techo \"BAD: M still exists in working directory\"\n-\t\treturn 1\n-\t}\n-\treturn 0\n+\tgit write-tree &&\n+\ttest_path_is_file B &&\n+\ttest_path_is_file N &&\n+\ttest_path_is_file R &&\n+\ttest_path_is_missing A &&\n+\ttest_path_is_missing M\n '\n \n # This test broke in 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9\n@@ -108,32 +87,13 @@ test_expect_success 'merge blue into white (A->B, mod A, A untracked)' \\\n \tgit checkout -b white-blue white &&\n \techo dirty >A &&\n \tgit merge blue &&\n-\tgit write-tree >/dev/null || {\n-\t\techo \"BAD: merge did not complete\"\n-\t\treturn 1\n-\t}\n-\n-\ttest -f A || {\n-\t\techo \"BAD: A does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest `cat A` = dirty || {\n-\t\techo \"BAD: A content is wrong\"\n-\t\treturn 1\n-\t}\n-\ttest -f B || {\n-\t\techo \"BAD: B does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest -f N || {\n-\t\techo \"BAD: N does not exist in working directory\"\n-\t\treturn 1\n-\t}\n-\ttest -f M && {\n-\t\techo \"BAD: M still exists in working directory\"\n-\t\treturn 1\n-\t}\n-\treturn 0\n+\tgit write-tree &&\n+\ttest_path_is_file A &&\n+\techo dirty >expect &&\n+\ttest_cmp expect A &&\n+\ttest_path_is_file B &&\n+\ttest_path_is_file N &&\n+\ttest_path_is_missing M\n '\n \n test_done\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258117","messageId":"20150320101318.GT12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 20/25] t4117: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:18Z","receivedAt":"2015-03-20T10:13:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can use test_must_fail and test_path_* to avoid some\nhand-rolled if statements. This makes the code shorter, and\nmakes it more obvious when we are breaking the &&-chain.\n\nSigned-off-by: Jeff King \n---\n t/t4117-apply-reject.sh | 66 ++++++++-----------------------------------------\n 1 file changed, 10 insertions(+), 56 deletions(-)\n\ndiff --git a/t/t4117-apply-reject.sh b/t/t4117-apply-reject.sh\nindex 8e15ecb..d80187d 100755\n--- a/t/t4117-apply-reject.sh\n+++ b/t/t4117-apply-reject.sh\n@@ -56,23 +56,13 @@ test_expect_success 'apply --reject is incompatible with --3way' '\n \n test_expect_success 'apply without --reject should fail' '\n \n-\tif git apply patch.1\n-\tthen\n-\t\techo \"Eh? Why?\"\n-\t\texit 1\n-\tfi\n-\n+\ttest_must_fail git apply patch.1 &&\n \ttest_cmp file1 saved.file1\n '\n \n test_expect_success 'apply without --reject should fail' '\n \n-\tif git apply --verbose patch.1\n-\tthen\n-\t\techo \"Eh? Why?\"\n-\t\texit 1\n-\tfi\n-\n+\ttest_must_fail git apply --verbose patch.1 &&\n \ttest_cmp file1 saved.file1\n '\n \n@@ -81,21 +71,11 @@ test_expect_success 'apply with --reject should fail but update the file' '\n \tcat saved.file1 >file1 &&\n \trm -f file1.rej file2.rej &&\n \n-\tif git apply --reject patch.1\n-\tthen\n-\t\techo \"succeeds with --reject?\"\n-\t\texit 1\n-\tfi\n-\n+\ttest_must_fail git apply --reject patch.1 &&\n \ttest_cmp file1 expected &&\n \n \tcat file1.rej &&\n-\n-\tif test -f file2.rej\n-\tthen\n-\t\techo \"file2 should not have been touched\"\n-\t\texit 1\n-\tfi\n+\ttest_path_is_missing file2.rej\n '\n \n test_expect_success 'apply with --reject should fail but update the file' '\n@@ -103,25 +83,12 @@ test_expect_success 'apply with --reject should fail but update the file' '\n \tcat saved.file1 >file1 &&\n \trm -f file1.rej file2.rej file2 &&\n \n-\tif git apply --reject patch.2 >rejects\n-\tthen\n-\t\techo \"succeeds with --reject?\"\n-\t\texit 1\n-\tfi\n-\n-\ttest -f file1 && {\n-\t\techo \"file1 still exists?\"\n-\t\texit 1\n-\t}\n+\ttest_must_fail git apply --reject patch.2 >rejects &&\n+\ttest_path_is_missing file1 &&\n \ttest_cmp file2 expected &&\n \n \tcat file2.rej &&\n-\n-\tif test -f file1.rej\n-\tthen\n-\t\techo \"file2 should not have been touched\"\n-\t\texit 1\n-\tfi\n+\ttest_path_is_missing file1.rej\n \n '\n \n@@ -130,25 +97,12 @@ test_expect_success 'the same test with --verbose' '\n \tcat saved.file1 >file1 &&\n \trm -f file1.rej file2.rej file2 &&\n \n-\tif git apply --reject --verbose patch.2 >rejects\n-\tthen\n-\t\techo \"succeeds with --reject?\"\n-\t\texit 1\n-\tfi\n-\n-\ttest -f file1 && {\n-\t\techo \"file1 still exists?\"\n-\t\texit 1\n-\t}\n+\ttest_must_fail git apply --reject --verbose patch.2 >rejects &&\n+\ttest_path_is_missing file1 &&\n \ttest_cmp file2 expected &&\n \n \tcat file2.rej &&\n-\n-\tif test -f file1.rej\n-\tthen\n-\t\techo \"file2 should not have been touched\"\n-\t\texit 1\n-\tfi\n+\ttest_path_is_missing file1.rej\n \n '\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258120","messageId":"20150320101321.GU12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 21/25] t9001: use test_when_finished","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:22Z","receivedAt":"2015-03-20T10:13:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The confirmation tests in t9001 all save the value of\nsendemail.confirm, do something to it, then restore it at\nthe end, in a way that breaks the &&-chain (they are not\nwrong, because they save the $? value, but it fools\n--chain-lint).\n\nInstead, they can all use test_when_finished, and we can\neven make the code simpler by factoring out the shared\nlines.\n\nNote that we can _almost_ use test_config here, except that:\n\n 1. We do not restore the config with test_unconfig, but by\n setting it back to some prior value.\n\n 2. We are not always setting a config variable. Sometimes\n the change to be undone is unsetting it entirely.\n\nWe could teach test_config to handle these cases, but it's\nnot worth the complexity for a single call-site.\n\nSigned-off-by: Jeff King \n---\n t/t9001-send-email.sh | 30 ++++++++++--------------------\n 1 file changed, 10 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 37caa18..c9f54d5 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' '\n \ttest_confirm --confirm=compose --compose\n '\n \n-test_expect_success $PREREQ 'confirm by default (due to cc)' '\n+save_confirm () {\n \tCONFIRM=$(git config --get sendemail.confirm) &&\n+\ttest_when_finished \"git config sendemail.confirm ${CONFIRM:-never}\"\n+}\n+\n+test_expect_success $PREREQ 'confirm by default (due to cc)' '\n+\tsave_confirm &&\n \tgit config --unset sendemail.confirm &&\n \ttest_confirm\n-\tret=\"$?\"\n-\tgit config sendemail.confirm ${CONFIRM:-never}\n-\ttest $ret = \"0\"\n '\n \n test_expect_success $PREREQ 'confirm by default (due to --compose)' '\n-\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tsave_confirm &&\n \tgit config --unset sendemail.confirm &&\n \ttest_confirm --suppress-cc=all --compose\n-\tret=\"$?\"\n-\tgit config sendemail.confirm ${CONFIRM:-never}\n-\ttest $ret = \"0\"\n '\n \n test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '\n-\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tsave_confirm &&\n \tgit config --unset sendemail.confirm &&\n \trm -fr outdir &&\n \tgit format-patch -2 -o outdir &&\n@@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '\n \t\t\t--to=nobody@example.com \\\n \t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t\toutdir/*.patch \n---\n t/t0050-filesystem.sh | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 988c392..b29d749 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -33,16 +33,20 @@ test_expect_success \"detection of case insensitive filesystem during repo init\"\n '\n else\n test_expect_success \"detection of case insensitive filesystem during repo init\" '\n-\ttest_must_fail git config --bool core.ignorecase >/dev/null ||\n-\ttest $(git config --bool core.ignorecase) = false\n+\t{\n+\t\ttest_must_fail git config --bool core.ignorecase >/dev/null ||\n+\t\t\ttest $(git config --bool core.ignorecase) = false\n+\t}\n '\n fi\n \n if test_have_prereq SYMLINKS\n then\n test_expect_success \"detection of filesystem w/o symlink support during repo init\" '\n-\ttest_must_fail git config --bool core.symlinks ||\n-\ttest \"$(git config --bool core.symlinks)\" = true\n+\t{\n+\t\ttest_must_fail git config --bool core.symlinks ||\n+\t\ttest \"$(git config --bool core.symlinks)\" = true\n+\t}\n '\n else\n test_expect_success \"detection of filesystem w/o symlink support during repo init\" '\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258122","messageId":"20150320101328.GW12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 23/25] t7004: fix embedded single-quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:29Z","receivedAt":"2015-03-20T10:13:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This test uses single quotes inside the single-quoted test\nsnippet, which effectively makes the contents unquoted.\nSince they don't need quoted anyway, this isn't a problem,\nbut let's switch them to double-quotes to make it more\nobviously correct.\n\nSigned-off-by: Jeff King \n---\n t/t7004-tag.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 347d3be..efb08c3 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1181,7 +1181,7 @@ test_expect_success \\\n \t'message in editor has initial comment: remainder' '\n \t# remove commented lines from the remainder -- should be empty\n \t>rest.expect &&\n-\tsed -e 1d -e '/^#/d' rest.actual &&\n+\tsed -e 1d -e \"/^#/d\" rest.actual &&\n \ttest_cmp rest.expect rest.actual\n '\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258121","messageId":"20150320101332.GX12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 24/25] t0005: fix broken &&-chains","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:32Z","receivedAt":"2015-03-20T10:13:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \":\" noop command always returns true, so it is fine to\ninclude these lines in an &&-chain (and it appeases\n--chain-lint).\n\nSigned-off-by: Jeff King \n---\n t/t0005-signals.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\nindex 5c5707d..e7f27eb 100755\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -40,12 +40,12 @@ test_expect_success 'create blob' '\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '\n-\tOUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 )\n+\tOUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n \ttest \"$OUT\" -eq 141\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '\n-\tOUT=$( ((trap \"\" PIPE; large_git; echo $? 1>&3) | :) 3>&1 )\n+\tOUT=$( ((trap \"\" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) &&\n \ttest \"$OUT\" -eq 141\n '\n \n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258119","messageId":"20150320101336.GY12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"[PATCH 25/25] t4104: drop hand-rolled error reporting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:13:36Z","receivedAt":"2015-03-20T10:13:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This use of \"||\" fools --chain-lint into thinking the\n&&-chain is broken (and indeed, it is somewhat broken; a\nfailure of update-index in these tests would show the patch\nfile, even if we never got to the part of the test where we\nfed the patch to git-apply).\n\nThe extra blocks were there to include more debugging\noutput, but it hardly seems worth it; the user should know\nwhich command failed (because git-apply will produce error\nmessages) and can look in the trash directory themselves.\n\nSigned-off-by: Jeff King \n---\n t/t4104-apply-boundary.sh | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh\nindex 497afdc..32e3b0e 100755\n--- a/t/t4104-apply-boundary.sh\n+++ b/t/t4104-apply-boundary.sh\n@@ -95,10 +95,7 @@ do\n \t\ttest_expect_success \"apply $kind-patch $with context\" '\n \t\t\tcat original >victim &&\n \t\t\tgit update-index victim &&\n-\t\t\tgit apply --index '\"$u$kind-patch.$with\"' || {\n-\t\t\t\tcat '\"$kind-patch.$with\"'\n-\t\t\t\t(exit 1)\n-\t\t\t} &&\n+\t\t\tgit apply --index '\"$u$kind-patch.$with\"' &&\n \t\t\ttest_cmp '\"$kind\"'-expect victim\n \t\t'\n \tdone\n@@ -113,10 +110,7 @@ do\n \ttest_expect_success \"apply non-git $kind-patch without context\" '\n \t\tcat original >victim &&\n \t\tgit update-index victim &&\n-\t\tgit apply --unidiff-zero --index '\"$kind-ng.without\"' || {\n-\t\t\tcat '\"$kind-ng.without\"'\n-\t\t\t(exit 1)\n-\t\t} &&\n+\t\tgit apply --unidiff-zero --index '\"$kind-ng.without\"' &&\n \t\ttest_cmp '\"$kind\"'-expect victim\n \t'\n done\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258123","messageId":"20150320102331.GZ12543@peff.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T10:23:32Z","receivedAt":"2015-03-20T10:23:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 06:04:30AM -0400, Jeff King wrote:\n\n> It's a lot of patches, and a few of them are long. I tried to group\n> them by functionality rather than file (though as you can see, some of\n> the tests were unique enough snowflakes that it made sense to discuss\n> their issues separately). I'm hoping it should be an easy review, if\n> not a short one.\n\nI hoped that would make it easier to review, but I was also curious\nabout the patterns I would see. Here are a few things I noticed:\n\n - the single most common place to forget an \"&&\" was at the start of a\n here-doc\n\n I complained earlier that I would prefer a way to put it at the end.\n And indeed, I found somebody who agreed with me and was more clever\n than I. They wrote in one test:\n\n (cat >expect < foo\" redirects when I was touching\n those particular lines :) ).\n\n Which is all to say, please don't mention ugly style issues you see\n in the context lines during review, unless it is to point me to your\n patch series that comes on top of mine and cleans them up. :)\n\n-Peff\n"},{"id":"258128","messageId":"550C2E7B.3030203@drmicha.warpmail.net","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-20T14:28:11Z","receivedAt":"2015-03-20T14:28:11Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 20.03.2015 11:04:\n> This is a cleanup of the &&-chain lint I posted earlier:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859\n> \n> I don't know who came up with the idea for it originally, but the\n> concept certainly was floating in the back of my mind from Jonathan's\n> earlier version that is referenced in that thread.\n> \n> The general idea is to detect &&-chain breakage that can lead to our\n> tests yielding false success. The first patch implements and discusses\n> the lint-check itself, which is quite simple. The bulk of the work was\n> fixing all of the issues in the existing tests. :)\n> \n> That didn't all need to happen immediately. I mainly wanted to start on\n> it to answer two questions:\n> \n> 1. Are &&-chain breakages actually preventing us from seeing any test\n> failures? Or is it mostly just pedantry, and we miss out only on\n> knowing whether \"cat >expect <<-\\EOF\" failed (which presumably it\n> never does).\n> \n> 2. How bad are the false positives? Both how common, and how bad to\n> work around.\n> \n> But after a few hours, I reached a zen state and just kept going. So at\n> the end of this series, the whole test suite is --chain-lint clean\n> (modulo any tests that are skipped on my platform). We could even switch\n> the checks on by default at the end of the series, but I didn't do that\n> here. I think it would be sane to run them all the time, though; in the\n> normal success case, they don't add any forks (the shell just runs\n> \"(exit) && ...\", and realizes that the whole thing is one big &&-chain).\n> I couldn't measure any time difference running the suite with and\n> without it.\n> \n> Anyway, to answer the questions: Yes, there were definitely tests whose\n> values were being thrown away, and we would not have noticed if they\n> failed. The good news is that all of them did pass once we started\n> checking their results. Hooray.\n> \n> There were a number of false positives, though as a percentage of the\n> test suite, probably not many (it's just that we have quite a lot of\n> tests). Most of them were in rather old tests, and IMHO the fixes I did\n> actually improved the readability of the result. So overall I think this\n> is a very positive change; I doubt it will get in people's way very\n> often, and I look forward to having one less thing to worry about\n> handling manually in review. The biggest downside is that I may have\n> automated Eric Sunshine out of a job. :)\n> \n> The patches are:\n> \n> [01/25]: t/test-lib: introduce --chain-lint option\n> [02/25]: t: fix severe &&-chain breakage\n> [03/25]: t: fix moderate &&-chain breakage\n> [04/25]: t: fix trivial &&-chain breakage\n> [05/25]: t: assume test_cmp produces verbose output\n> [06/25]: t: use verbose instead of hand-rolled errors\n> [07/25]: t: use test_must_fail instead of hand-rolled blocks\n> [08/25]: t: fix &&-chaining issues around setup which might fail\n> [09/25]: t: use test_might_fail for diff and grep\n> [10/25]: t: use test_expect_code instead of hand-rolled comparison\n> [11/25]: t: wrap complicated expect_code users in a block\n> [12/25]: t: avoid using \":\" for comments\n> [13/25]: t3600: fix &&-chain breakage for setup commands\n> [14/25]: t7201: fix &&-chain breakage\n> [15/25]: t9502: fix &&-chain breakage\n> [16/25]: t6030: use modern test_* helpers\n> [17/25]: t0020: use modern test_* helpers\n> [18/25]: t1301: use modern test_* helpers\n> [19/25]: t6034: use modern test_* helpers\n> [20/25]: t4117: use modern test_* helpers\n> [21/25]: t9001: use test_when_finished\n> [22/25]: t0050: appease --chain-lint\n> [23/25]: t7004: fix embedded single-quotes\n> [24/25]: t0005: fix broken &&-chains\n> [25/25]: t4104: drop hand-rolled error reporting\n> \n> It's a lot of patches, and a few of them are long. I tried to group\n> them by functionality rather than file (though as you can see, some of\n> the tests were unique enough snowflakes that it made sense to discuss\n> their issues separately). I'm hoping it should be an easy review, if\n> not a short one.\n> \n> I'll spare you the full per-file diffstat, but the total is a very\n> satisfying:\n> \n> 84 files changed, 397 insertions(+), 647 deletions(-)\n> \n> That's a lot of changes in a lot of hunks, but most of them are in files\n> that haven't been touched in a while. The series merges cleanly with\n> \"pu\", so I don't think I've stepped on anyone's topics in flight.\n> \n> -Peff\n\nNice series! In fact, it's a great place to learn many of the test suite\nfeatures that we have nowadays.\n\nWith 1/25 only, I get 163 dubious or failed on current next.\nWith 1/25 and only chain-lint without running the actual test loads, I\nget 213.\n\nSo, just as Jeff explained, we don't want a \"chain-lint-only mode\"\nbecause it does not give correct results.\n\nI reviewed the first 15 and they all look good.\nI skimmed through the rest and it appears good to (though this is no\nreview).\n\nYou run without git svn tests, obviously... I'll follow up with two\npatches for those.\n\nMichael\n"},{"id":"258130","messageId":"317e6b1e70f3e1c50d62207c53f4d038ad027c9d.1426861743.git.git@drmicha.warpmail.net","threadId":"38863","inReplyTo":"550C2E7B.3030203@drmicha.warpmail.net","subject":"[PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-20T14:32:55Z","receivedAt":"2015-03-20T14:32:55Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"All of these cases are moderate since they would most probably not\nlead to missed failing tests: Either they would fail otherwise,\nor fail a rm in test_when_finished only.\n\nSigned-off-by: Michael J Gruber \n---\n t/t2026-prune-linked-checkouts.sh | 4 ++--\n t/t9158-git-svn-mergeinfo.sh | 4 ++--\n t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----\n 3 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh\nindex 2936d52..e885baf 100755\n--- a/t/t2026-prune-linked-checkouts.sh\n+++ b/t/t2026-prune-linked-checkouts.sh\n@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '\n '\n \n test_expect_success 'not prune locked checkout' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir -p .git/worktrees/ghi &&\n \t: >.git/worktrees/ghi/locked &&\n \tgit prune --worktrees &&\n@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '\n '\n \n test_expect_success 'not prune recent checkouts' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir zz &&\n \tmkdir -p .git/worktrees/jlm &&\n \techo \"$(pwd)\"/zz >.git/worktrees/jlm/gitdir &&\ndiff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh\nindex 8c9539e..13f78f2 100755\n--- a/t/t9158-git-svn-mergeinfo.sh\n+++ b/t/t9158-git-svn-mergeinfo.sh\n@@ -34,7 +34,7 @@ test_expect_success 'change svn:mergeinfo' '\n '\n \n test_expect_success 'verify svn:mergeinfo' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk) &&\n \ttest \"$mergeinfo\" = \"/branches/foo:1-10\"\n '\n \n@@ -46,7 +46,7 @@ test_expect_success 'change svn:mergeinfo multiline' '\n '\n \n test_expect_success 'verify svn:mergeinfo multiline' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/trunk) &&\n \ttest \"$mergeinfo\" = \"/branches/bar:1-10\n /branches/other:3-5,8,10-11\"\n '\ndiff --git a/t/t9161-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh\nindex 6cb0909..f113aca 100755\n--- a/t/t9161-git-svn-mergeinfo-push.sh\n+++ b/t/t9161-git-svn-mergeinfo-push.sh\n@@ -24,7 +24,7 @@ test_expect_success 'propagate merge information' '\n \t'\n \n test_expect_success 'check svn:mergeinfo' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1) &&\n \ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\"\n \t'\n \n@@ -34,7 +34,7 @@ test_expect_success 'merge another branch' '\n \t'\n \n test_expect_success 'check primary parent mergeinfo respected' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1) &&\n \ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\n /branches/svnb3:4,9\"\n \t'\n@@ -45,7 +45,7 @@ test_expect_success 'merge existing merge' '\n \t'\n \n test_expect_success \"check both parents' mergeinfo respected\" '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1) &&\n \ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\n /branches/svnb3:4,9\n /branches/svnb4:5-6,10-12\n@@ -70,7 +70,7 @@ test_expect_success 'second forward merge' '\n \t'\n \n test_expect_success 'check new mergeinfo added' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1) &&\n \ttest \"$mergeinfo\" = \"/branches/svnb2:3,8,16-17\n /branches/svnb3:4,9\n /branches/svnb4:5-6,10-12\n@@ -84,7 +84,7 @@ test_expect_success 'reintegration merge' '\n \t'\n \n test_expect_success 'check reintegration mergeinfo' '\n-\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb4)\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb4) &&\n \ttest \"$mergeinfo\" = \"/branches/svnb1:2-4,7-9,13-18\n /branches/svnb2:3,8,16-17\n /branches/svnb3:4,9\n-- \n2.3.3.438.g7254779\n"},{"id":"258129","messageId":"43da457ddd8840ac5cf131e59d488fc41cdc8a52.1426861743.git.git@drmicha.warpmail.net","threadId":"38863","inReplyTo":"550C2E7B.3030203@drmicha.warpmail.net","subject":"[PATCH 27/27] t9104: fix test for following larger parents","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-20T14:32:56Z","receivedAt":"2015-03-20T14:32:56Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"This test is special for several reasons:\nIt ends with a \"true\" statement, which should be a no-op.\nIt is not because the &&-chain is broken right before it.\n\nAlso, looking at what the test intended to test according to\n7f578c5 (git-svn: --follow-parent now works on sub-directories of larger\nbranches, 2007-01-24)\nit is not clear how it would achieve that with the given steps.\n\nAmend the test to include the second svn id to be tested for, and\nchange the tested refs to the ones which are to be expected, and which\nmake the test pass.\n\nSigned-off-by: Michael J Gruber \n---\nIn fact, these merge-base tests look kind of ugly, too.\nBut they appear in many more places here and should best be rewritten\nseparately.\n\n t/t9104-git-svn-follow-parent.sh | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh\nindex 13b179e..1bd1022 100755\n--- a/t/t9104-git-svn-follow-parent.sh\n+++ b/t/t9104-git-svn-follow-parent.sh\n@@ -72,16 +72,18 @@ test_expect_success 'follow larger parent' '\n svn import -m \"import a larger parent\" import \"$svnrepo\"/larger-parent &&\n svn cp -m \"hi\" \"$svnrepo\"/larger-parent \"$svnrepo\"/another-larger &&\n git svn init --minimize-url -i larger \\\n- \"$svnrepo\"/another-larger/trunk/thunk/bump/thud &&\n+ \"$svnrepo\"/larger-parent/trunk/thunk/bump/thud &&\n git svn fetch -i larger &&\n+ git svn init --minimize-url -i larger-parent \\\n+ \"$svnrepo\"/another-larger/trunk/thunk/bump/thud &&\n+ git svn fetch -i larger-parent &&\n git rev-parse --verify refs/remotes/larger &&\n git rev-parse --verify \\\n- refs/remotes/larger-parent/trunk/thunk/bump/thud &&\n+ refs/remotes/larger-parent &&\n test \"`git merge-base \\\n- refs/remotes/larger-parent/trunk/thunk/bump/thud \\\n+ refs/remotes/larger-parent \\\n refs/remotes/larger`\" = \\\n \"`git rev-parse refs/remotes/larger`\"\n- true\n '\n \n test_expect_success 'follow higher-level parent' '\n-- \n2.3.3.438.g7254779\n"},{"id":"258141","messageId":"xmqqegojftsj.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-20T17:44:12Z","receivedAt":"2015-03-20T17:44:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. They applied cleanly on 'master' and all looked sensible.\n\nI found 2026 and 5312 to be broken (there may be others that are\nexcluded in my usual test set) in 'pu'. As to these topics in \"git\nlog --first-parent master..pu\", my preference is to queue fixups on\nthe broken-out topics (available at http://github.com/gitster/git)\nindependently.\n\nFor example, this will go on top of nd/multiple-work-trees.\n\ndiff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh\nindex 2936d52..e885baf 100755\n--- a/t/t2026-prune-linked-checkouts.sh\n+++ b/t/t2026-prune-linked-checkouts.sh\n@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '\n '\n \n test_expect_success 'not prune locked checkout' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir -p .git/worktrees/ghi &&\n \t: >.git/worktrees/ghi/locked &&\n \tgit prune --worktrees &&\n@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '\n '\n \n test_expect_success 'not prune recent checkouts' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir zz &&\n \tmkdir -p .git/worktrees/jlm &&\n \techo \"$(pwd)\"/zz >.git/worktrees/jlm/gitdir &&\n"},{"id":"258142","messageId":"550C5D73.4080901@kdbg.org","threadId":"38863","inReplyTo":"20150320101301.GO12543@peff.net","subject":"Re: [PATCH 15/25] t9502: fix &&-chain breakage","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-03-20T17:48:35Z","receivedAt":"2015-03-20T17:48:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.03.2015 um 11:13 schrieb Jeff King:\n> This script misses a trivial &&-chain in one of its tests,\n> but it also has a weird reverse: it includes an &&-chain\n> outside of any test_expect block! This \"cat\" should never\n> fail, but if it did, we would not notice, as it would cause\n> us to skip the follow-on test entirely (which does not\n> appear intentional; there are many later tests which rely on\n> this cat).\n>\n> Let's instead move the setup into its own test_expect_success\n> block, which is the standard practice nowadays.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t9502-gitweb-standalone-parse-output.sh | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\n> index 86dfee2..a93e159 100755\n> --- a/t/t9502-gitweb-standalone-parse-output.sh\n> +++ b/t/t9502-gitweb-standalone-parse-output.sh\n> @@ -145,9 +145,11 @@ test_expect_success 'forks: not skipped unless \"forks\" feature enabled' '\n> \tgrep -q \">fork of .*<\" gitweb.body\n> '\n>\n> -cat >>gitweb_config.perl <<\\EOF &&\n> -$feature{'forks'}{'default'} = [1];\n> -EOF\n> +test_expect_success 'enable forks feature' '\n> +\tcat >>gitweb_config.perl <<-\\EOF\n> +\t$feature{'forks'}{'default'} = [1];\n> +\tEOF\n> +'\n\nThis loses the single-quotes in the generated perl script, doesn't it? \nMost likely, it does not matter.\n\n-- Hannes\n"},{"id":"258144","messageId":"20150320175703.GA15054@peff.net","threadId":"38863","inReplyTo":"550C2E7B.3030203@drmicha.warpmail.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T17:57:03Z","receivedAt":"2015-03-20T17:57:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 03:28:11PM +0100, Michael J Gruber wrote:\n\n> With 1/25 only, I get 163 dubious or failed on current next.\n> With 1/25 and only chain-lint without running the actual test loads, I\n> get 213.\n> \n> So, just as Jeff explained, we don't want a \"chain-lint-only mode\"\n> because it does not give correct results.\n\nThanks for checking. I had assumed there would be some weirdness, but I\ndidn't actually try a lint-only mode.\n\nI only ran the lint against master earlier. There are two trivial broken\nchains in pu. Patch is below (against nd/multiple-work-trees). Looks\nlike that is in 'next', so we can't just squash it in.\n\n-- >8 --\nSubject: t2026: fix broken &&-chains\n\nThese are totally trivial (test_when_finished should never\nfail), but being complete with our &&-chaining makes the new\n--chain-lint feature more useful.\n\nSigned-off-by: Jeff King \n---\n t/t2026-prune-linked-checkouts.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh\nindex 2936d52..e885baf 100755\n--- a/t/t2026-prune-linked-checkouts.sh\n+++ b/t/t2026-prune-linked-checkouts.sh\n@@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' '\n '\n \n test_expect_success 'not prune locked checkout' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir -p .git/worktrees/ghi &&\n \t: >.git/worktrees/ghi/locked &&\n \tgit prune --worktrees &&\n@@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' '\n '\n \n test_expect_success 'not prune recent checkouts' '\n-\ttest_when_finished rm -r .git/worktrees\n+\ttest_when_finished rm -r .git/worktrees &&\n \tmkdir zz &&\n \tmkdir -p .git/worktrees/jlm &&\n \techo \"$(pwd)\"/zz >.git/worktrees/jlm/gitdir &&\n-- \n2.3.3.520.g3cfbb5d\n"},{"id":"258146","messageId":"xmqq384zft22.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"xmqqegojftsj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-20T18:00:05Z","receivedAt":"2015-03-20T18:00:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I found 2026 and 5312 to be broken (there may be others that are\n> excluded in my usual test set) in 'pu'. As to these topics in \"git\n> log --first-parent master..pu\", my preference is to queue fixups on\n> the broken-out topics (available at http://github.com/gitster/git)\n> independently.\n>\n> For example, this will go on top of nd/multiple-work-trees.\n\n... and this goes on top of jk/prune-with-corrupt-refs until it is\nrerolled.\n\n-- >8 --\nSubject: [PATCH] SQUASH??? t5312: fix broken &&-chain\n\nSigned-off-by: Junio C Hamano \n---\n t/t5312-prune-corruption.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh\nindex 8b54d16..9633d97 100755\n--- a/t/t5312-prune-corruption.sh\n+++ b/t/t5312-prune-corruption.sh\n@@ -80,7 +80,7 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '\n # skip processing a broken ref\n test_expect_success 'create packed-refs file with broken ref' '\n \trm -f .git/refs/heads/master &&\n-\tcat >.git/packed-refs <<-EOF\n+\tcat >.git/packed-refs <<-EOF &&\n \t$missing refs/heads/master\n \t$recoverable refs/heads/other\n \tEOF\n-- \n2.3.3-401-g402122f\n"},{"id":"258147","messageId":"20150320180301.GB11844@peff.net","threadId":"38863","inReplyTo":"550C5D73.4080901@kdbg.org","subject":"Re: [PATCH 15/25] t9502: fix &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T18:03:01Z","receivedAt":"2015-03-20T18:03:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 06:48:35PM +0100, Johannes Sixt wrote:\n\n> >-cat >>gitweb_config.perl <<\\EOF &&\n> >-$feature{'forks'}{'default'} = [1];\n> >-EOF\n> >+test_expect_success 'enable forks feature' '\n> >+\tcat >>gitweb_config.perl <<-\\EOF\n> >+\t$feature{'forks'}{'default'} = [1];\n> >+\tEOF\n> >+'\n> \n> This loses the single-quotes in the generated perl script, doesn't it? Most\n> likely, it does not matter.\n\nEek, you're right. Thanks for noticing.\n\nI agree it should not matter (it is OK in perl to use barewords as hash\nkeys, but it would probably be more obvious to omit them entirely, or to\njust use double-quotes. I'll squash this in:\n\ndiff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh\nindex a93e159..0796a43 100755\n--- a/t/t9502-gitweb-standalone-parse-output.sh\n+++ b/t/t9502-gitweb-standalone-parse-output.sh\n@@ -147,7 +147,7 @@ test_expect_success 'forks: not skipped unless \"forks\" feature enabled' '\n \n test_expect_success 'enable forks feature' '\n \tcat >>gitweb_config.perl <<-\\EOF\n-\t$feature{'forks'}{'default'} = [1];\n+\t$feature{\"forks\"}{\"default\"} = [1];\n \tEOF\n '\n \n\n-Peff\n"},{"id":"258149","messageId":"xmqqy4mree9x.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"317e6b1e70f3e1c50d62207c53f4d038ad027c9d.1426861743.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-20T18:04:42Z","receivedAt":"2015-03-20T18:04:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> All of these cases are moderate since they would most probably not\n> lead to missed failing tests: Either they would fail otherwise,\n> or fail a rm in test_when_finished only.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> t/t2026-prune-linked-checkouts.sh | 4 ++--\n> t/t9158-git-svn-mergeinfo.sh | 4 ++--\n> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----\n> 3 files changed, 9 insertions(+), 9 deletions(-)\n\nAhh, it seems that I should have read everything in my inbox before\nstarting my day X-<. I already queued an identical patch for 2026\non nd/multiple-work-trees, and its new tip is in 'next' now.\n\nWhich branches are the git-svn ones meant to apply? Are they meant\nto fix an existing bug already in master, or are they new ones added\nby still-in-flight topics? Can you split if necessary and mark them\nfor individual topios in flight if that is the case, so that we can\napply them independently from GIT_TEST_CHAIN_LINT series?\n\nThanks.\n"},{"id":"258148","messageId":"20150320180448.GC11844@peff.net","threadId":"38863","inReplyTo":"xmqq384zft22.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T18:04:48Z","receivedAt":"2015-03-20T18:04:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 11:00:05AM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > I found 2026 and 5312 to be broken (there may be others that are\n> > excluded in my usual test set) in 'pu'. As to these topics in \"git\n> > log --first-parent master..pu\", my preference is to queue fixups on\n> > the broken-out topics (available at http://github.com/gitster/git)\n> > independently.\n> >\n> > For example, this will go on top of nd/multiple-work-trees.\n\nHeh, I just crossed emails with you over the same patch.\n\n> ... and this goes on top of jk/prune-with-corrupt-refs until it is\n> rerolled.\n\nI'm actually about to send out a re-roll of that, as I think all of the\nreview comments have been addressed.\n\n-Peff\n"},{"id":"258153","messageId":"xmqqlhirecxh.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"20150320180448.GC11844@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-20T18:33:46Z","receivedAt":"2015-03-20T18:33:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm actually about to send out a re-roll of that, as I think all of the\n> review comments have been addressed.\n\nThanks.\n"},{"id":"258178","messageId":"xmqqh9tfea1v.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"xmqqy4mree9x.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-20T19:35:56Z","receivedAt":"2015-03-20T19:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>> All of these cases are moderate since they would most probably not\n>> lead to missed failing tests: Either they would fail otherwise,\n>> or fail a rm in test_when_finished only.\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n>> t/t2026-prune-linked-checkouts.sh | 4 ++--\n>> t/t9158-git-svn-mergeinfo.sh | 4 ++--\n>> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----\n>> 3 files changed, 9 insertions(+), 9 deletions(-)\n>\n> Ahh, it seems that I should have read everything in my inbox before\n> starting my day X-<. I already queued an identical patch for 2026\n> on nd/multiple-work-trees, and its new tip is in 'next' now.\n>\n> Which branches are the git-svn ones meant to apply? Are they meant\n> to fix an existing bug already in master, or are they new ones added\n> by still-in-flight topics? Can you split if necessary and mark them\n> for individual topios in flight if that is the case, so that we can\n> apply them independently from GIT_TEST_CHAIN_LINT series?\n>\n> Thanks.\n\nOK, I think I figured it out. All the git-svn bits in 26 and 27 are\nfor 'master', and should be part of Peff's original 25-patch series.\nI'll drop 2026 from your 26/25 and queue it together with your 27/25\non top off it.\n"},{"id":"258183","messageId":"20150320200239.GA25506@peff.net","threadId":"38863","inReplyTo":"xmqqh9tfea1v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T20:02:39Z","receivedAt":"2015-03-20T20:02:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 12:35:56PM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Michael J Gruber writes:\n> >\n> >> All of these cases are moderate since they would most probably not\n> >> lead to missed failing tests: Either they would fail otherwise,\n> >> or fail a rm in test_when_finished only.\n> >>\n> >> Signed-off-by: Michael J Gruber \n> >> ---\n> >> t/t2026-prune-linked-checkouts.sh | 4 ++--\n> >> t/t9158-git-svn-mergeinfo.sh | 4 ++--\n> >> t/t9161-git-svn-mergeinfo-push.sh | 10 +++++-----\n> >> 3 files changed, 9 insertions(+), 9 deletions(-)\n> >\n> > Ahh, it seems that I should have read everything in my inbox before\n> > starting my day X-<. I already queued an identical patch for 2026\n> > on nd/multiple-work-trees, and its new tip is in 'next' now.\n> >\n> > Which branches are the git-svn ones meant to apply? Are they meant\n> > to fix an existing bug already in master, or are they new ones added\n> > by still-in-flight topics? Can you split if necessary and mark them\n> > for individual topios in flight if that is the case, so that we can\n> > apply them independently from GIT_TEST_CHAIN_LINT series?\n> >\n> > Thanks.\n> \n> OK, I think I figured it out. All the git-svn bits in 26 and 27 are\n> for 'master', and should be part of Peff's original 25-patch series.\n> I'll drop 2026 from your 26/25 and queue it together with your 27/25\n> on top off it.\n\nYeah, that was my impression, too. I don't have svn installed on my\nsystem, so I missed those ones. I don't have CVS either. That might be\nworth following up on.\n\n-Peff\n"},{"id":"258184","messageId":"20150320201321.GA21945@peff.net","threadId":"38863","inReplyTo":"20150320200239.GA25506@peff.net","subject":"Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-20T20:13:21Z","receivedAt":"2015-03-20T20:13:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 04:02:39PM -0400, Jeff King wrote:\n\n> Yeah, that was my impression, too. I don't have svn installed on my\n> system, so I missed those ones. I don't have CVS either. That might be\n> worth following up on.\n\nHmm, that turned out rather easy. No breakages at all in the cvs tests.\nIt almost makes me think I ran the tests wrong. ;)\n\n-Peff\n"},{"id":"258200","messageId":"CAPig+cRCbhMR58_PSFnsWoyo_aZoTOVZM2YeYC6Tvo7iXMZwBA@mail.gmail.com","threadId":"38863","inReplyTo":"20150320100429.GA17354@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-20T23:18:47Z","receivedAt":"2015-03-20T23:18:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 20, 2015 at 6:04 AM, Jeff King wrote:\n> [...]\n> There were a number of false positives, though as a percentage of the\n> test suite, probably not many (it's just that we have quite a lot of\n> tests). Most of them were in rather old tests, and IMHO the fixes I did\n> actually improved the readability of the result. So overall I think this\n> is a very positive change; I doubt it will get in people's way very\n> often, and I look forward to having one less thing to worry about\n> handling manually in review. The biggest downside is that I may have\n> automated Eric Sunshine out of a job. :)\n\nHeh. I won't mind. Thanks for doing a thorough job.\n\nIronically, one of the broken here-doc &&-links you detected with\n--chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb\n(t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17).\n"},{"id":"258237","messageId":"20150321081909.GA8221@peff.net","threadId":"38863","inReplyTo":"CAPig+cRCbhMR58_PSFnsWoyo_aZoTOVZM2YeYC6Tvo7iXMZwBA@mail.gmail.com","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-21T08:19:09Z","receivedAt":"2015-03-21T08:19:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2015 at 07:18:47PM -0400, Eric Sunshine wrote:\n\n> Ironically, one of the broken here-doc &&-links you detected with\n> --chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb\n> (t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17).\n\nHeh. I was afraid to look at my own.\n\nI wondered if we had a good way of finding who wrote the specific lines\nthat were changed in a patch. I wrote the script below before realizing\nthat contrib/contacts does something similar (though I think it blames\nwhole hunks, including context, and I really want to care specifically\nonly about touched lines; I wonder if that is a distinction git-contacts\nshould make).\n\nRunning:\n\n git diff origin origin/jk/test-chain-lint |\n perl diff-blame.pl jk/test-chain-lint |\n grep EOF\n\nwas fun. At least I am not the only one. :)\n\nNor the worst in the \"severe\" category. I will leave using the script to\nread the list of \"severe\" names as an exercise to the reader; reading\nsuch output is probably not overly healthy (and besides, to be fair it\nreally should be normalized over the number of contributions).\n\n-- >8 --\n# diff-blame.pl\nuse warnings FATAL => 'all';\nuse strict;\n\nmy $head = shift\n or die \"usage: git diff | $0 [blame-opts]\";\n\nmy $file;\nmy @lines;\nmy $line_nr;\n\nwhile () {\n if (m{^--- .*?/(.*)}) {\n flush();\n $file = $1;\n @lines = ();\n }\n elsif (m{^@@ -(\\d+)}) {\n $line_nr = $1;\n }\n elsif (m{^ }) {\n $line_nr++;\n }\n elsif (m{^-}) {\n push @lines, $line_nr++;\n }\n}\nflush();\nexit 0;\n\nsub flush {\n return unless defined $file && @lines > 0;\n\n # XXX coalesce blocks of adjacent lines into ranges?\n system(qw(git --no-pager blame), @ARGV,\n (map { \"-L$_,$_\" } @lines), $head, $file);\n}\n"},{"id":"258250","messageId":"xmqq384ydybc.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"20150321081909.GA8221@peff.net","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-21T18:01:43Z","receivedAt":"2015-03-21T18:01:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Running:\n>\n> git diff origin origin/jk/test-chain-lint |\n> perl diff-blame.pl jk/test-chain-lint |\n> grep EOF\n>\n> was fun. At least I am not the only one. :)\n\nThe parameter to diff-blame.pl should be origin, instead of\njk/test-chain-lint, I presume? You are grabbing the preimage line\nnumbers and asking blame to find out who wrote them.\n\n> Nor the worst in the \"severe\" category.\n\nI do not quite get what this means---the script does not seem to\njudge what is severe and what is not, so I presume that this is to\nbe judged by whoever is reading the output from the above pipeline\nafter replacing \"grep EOF\" with \"less\" or something?\n\n> # diff-blame.pl\n> use warnings FATAL => 'all';\n> use strict;\n>\n> my $head = shift\n> or die \"usage: git diff | $0 [blame-opts]\";\n>\n> my $file;\n> my @lines;\n> my $line_nr;\n>\n> while () {\n> if (m{^--- .*?/(.*)}) {\n\nThis may match a removal of a line that begins with \"^-- something/\" ;-)\n\n> flush();\n> $file = $1;\n> @lines = ();\n> }\n> elsif (m{^@@ -(\\d+)}) {\n> $line_nr = $1;\n> }\n> elsif (m{^ }) {\n> $line_nr++;\n> }\n> elsif (m{^-}) {\n> push @lines, $line_nr++;\n> }\n> }\n> flush();\n> exit 0;\n>\n> sub flush {\n> return unless defined $file && @lines > 0;\n>\n> # XXX coalesce blocks of adjacent lines into ranges?\n> system(qw(git --no-pager blame), @ARGV,\n\nYou may want to pass an option to always show the filename here.\n\n> (map { \"-L$_,$_\" } @lines), $head, $file);\n> }\n"},{"id":"258260","messageId":"20150321222304.GA27850@peff.net","threadId":"38863","inReplyTo":"xmqq384ydybc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/25] detecting &&-chain breakage","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-21T22:23:04Z","receivedAt":"2015-03-21T22:23:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 21, 2015 at 11:01:43AM -0700, Junio C Hamano wrote:\n\n> > Running:\n> >\n> > git diff origin origin/jk/test-chain-lint |\n> > perl diff-blame.pl jk/test-chain-lint |\n> > grep EOF\n> >\n> > was fun. At least I am not the only one. :)\n> \n> The parameter to diff-blame.pl should be origin, instead of\n> jk/test-chain-lint, I presume? You are grabbing the preimage line\n> numbers and asking blame to find out who wrote them.\n\nYes, sorry, that was an error translating from what I actually ran in\nthe shell into the email. It should be \"origin\". And if the script\nreally wanted to be user-friendly, it should probably take two endpoints\nand just run the diff itself (when I started it, I assume that you could\nprocess any diff, but of course you must know the start point to get a\nreasonable blame).\n\n> > Nor the worst in the \"severe\" category.\n> \n> I do not quite get what this means---the script does not seem to\n> judge what is severe and what is not, so I presume that this is to\n> be judged by whoever is reading the output from the above pipeline\n> after replacing \"grep EOF\" with \"less\" or something?\n\nThat was the exercise I left to the reader. :) In this case, it is\npossible because I have already split the patches into \"severe\",\n\"moderate\", and \"trivial\" cases, so you can blame only the severe patch\n(using its parent as the start-point).\n\n> > while () {\n> > if (m{^--- .*?/(.*)}) {\n> \n> This may match a removal of a line that begins with \"^-- something/\" ;-)\n\nTrue. I was trying to avoid being stateful in my diff parsing. I guess\nit would be enough to parse the hunk headers to know how many lines are\nin the hunk. Not worth it for this one-off, but a good thing if somebody\nwanted to pick this idea up for a \"real\" tool.\n\n> > # XXX coalesce blocks of adjacent lines into ranges?\n> > system(qw(git --no-pager blame), @ARGV,\n> \n> You may want to pass an option to always show the filename here.\n\nI left that to the user. I actually found \"--line-porcelain\" useful for\ngathering statistics (e.g., piped to \"grep '^author ' | sort | uniq -c\").\n\n-Peff\n"},{"id":"258351","messageId":"550FDEB6.8050809@drmicha.warpmail.net","threadId":"38863","inReplyTo":"20150320201321.GA21945@peff.net","subject":"Re: [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-23T09:36:54Z","receivedAt":"2015-03-23T09:36:54Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 20.03.2015 21:13:\n> On Fri, Mar 20, 2015 at 04:02:39PM -0400, Jeff King wrote:\n> \n>> Yeah, that was my impression, too. I don't have svn installed on my\n>> system, so I missed those ones. I don't have CVS either. That might be\n>> worth following up on.\n> \n> Hmm, that turned out rather easy. No breakages at all in the cvs tests.\n> It almost makes me think I ran the tests wrong. ;)\n> \n> -Peff\n> \n\nYes, sorry, just catching up.\n\nI applied to Jeff's series to next at 9c5cf4d, and I run with svn and\ncvs tests. Full suite except for:\n\nexpensive, ext. cred. helper, mac fs, svnserve, svn-info, git-cvsserver, p4.\n\nI guess I should recheck some prerequisites and adjust the svn-info test\nto newer svn...\n\nMichael\n"},{"id":"258461","messageId":"20150325004334.Horde.frKSaf3PFu150aHhx6lRlw3@webmail.informatik.kit.edu","threadId":"38863","inReplyTo":"20150320101315.GS12543@peff.net","subject":"Re: [PATCH 19/25] t6034: use modern test_* helpers","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-24T23:43:34Z","receivedAt":"2015-03-24T23:43:34Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King :\n\n> These say roughly the same thing as the hand-rolled\n> messages. We do lose the \"merge did not complete\" debug\n> message, but merge and write-tree are prefectly capable of\n\ns/prefectly/perfectly/\n\n> writing useful error messages when they fail.\n>\n> Signed-off-by: Jeff King \n> ---\n"},{"id":"258463","messageId":"20150325005120.Horde.ClYI1WJflnvTzVgcL6i8Sg8@webmail.informatik.kit.edu","threadId":"38863","inReplyTo":"20150320101311.GR12543@peff.net","subject":"Re: [PATCH 18/25] t1301: use modern test_* helpers","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-24T23:51:20Z","receivedAt":"2015-03-24T23:51:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King :\n\n> This shortens the code and fixes some &&-chaining.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t1301-shared-repo.sh | 20 +++++++-------------\n> 1 file changed, 7 insertions(+), 13 deletions(-)\n>\n> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\n> index 7eecfb8..ac10875 100755\n> --- a/t/t1301-shared-repo.sh\n> +++ b/t/t1301-shared-repo.sh\n> @@ -12,12 +12,11 @@ setfacl -k . 2>/dev/null\n>\n> # User must have read permissions to the repo -> failure on --shared=0400\n> test_expect_success 'shared = 0400 (faulty permission u-w)' '\n> +\ttest_when_finished \"rm -rf sub\" &&\n> \tmkdir sub && (\n> -\t\tcd sub && git init --shared=0400\n> +\t\tcd sub &&\n> +\t\ttest_must_fail git init --shared=0400\n> \t)\n> -\tret=\"$?\"\n> -\trm -rf sub\n> -\ttest $ret != \"0\"\n> '\n>\n> modebits () {\n> @@ -33,7 +32,7 @@ do\n> \t\t\tgit init --shared=1 &&\n> \t\t\ttest 1 = \"$(git config core.sharedrepository)\"\n> \t\t) &&\n> -\t\tactual=$(ls -l sub/.git/HEAD)\n> +\t\tactual=$(ls -l sub/.git/HEAD) &&\n> \t\tcase \"$actual\" in\n> \t\t-rw-rw-r--*)\n> \t\t\t: happy\n\nThis hunk could go into the \"moderate &&-chain breakage\" patch.\nDoesn't really matter, what matters most is that it's fixed, but I \nreally liked your classification of missing &&s in the early patches.\n\n> @@ -90,10 +89,8 @@ do\n> \t\trm -f .git/info/refs &&\n> \t\tgit update-server-info &&\n> \t\tactual=\"$(modebits .git/info/refs)\" &&\n> -\t\ttest \"x$actual\" = \"x-$y\" || {\n> -\t\t\tls -lt .git/info\n> -\t\t\tfalse\n> -\t\t}\n> +\t\tverbose test \"x$actual\" = \"x-$y\"\n> +\n> \t'\n>\n> \tumask 077 &&\n> @@ -102,10 +99,7 @@ do\n> \t\trm -f .git/info/refs &&\n> \t\tgit update-server-info &&\n> \t\tactual=\"$(modebits .git/info/refs)\" &&\n> -\t\ttest \"x$actual\" = \"x-$x\" || {\n> -\t\t\tls -lt .git/info\n> -\t\t\tfalse\n> -\t\t}\n> +\t\tverbose test \"x$actual\" = \"x-$x\"\n>\n> \t'\n>\n> --\n> 2.3.3.520.g3cfbb5d\n"},{"id":"258464","messageId":"20150325012323.Horde.zCWvV1mF8OBE1PxYPuuEFg8@webmail.informatik.kit.edu","threadId":"38863","inReplyTo":"20150320101308.GQ12543@peff.net","subject":"Re: [PATCH 17/25] t0020: use modern test_* helpers","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-25T00:23:23Z","receivedAt":"2015-03-25T00:23:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King :\n\n> This test contains a lot of hand-rolled messages to show\n> when the test fails. We can omit most of these by using\n> \"verbose\" and \"test_must_fail\". A few of them are for\n> update-index, but we can assume it produces reasonable error\n> messages when it fails.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t0020-crlf.sh | 144 \n> +++++++++++---------------------------------------------\n> 1 file changed, 28 insertions(+), 116 deletions(-)\n>\n> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> index d2e51a8..9fa26df 100755\n> --- a/t/t0020-crlf.sh\n> +++ b/t/t0020-crlf.sh\n> @@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '\n> \tfor f in one dir/two\n> \tdo\n> \t\tappend_cr <$f >tmp && mv -f tmp $f &&\n> -\t\tgit update-index -- $f || {\n> -\t\t\techo Oops\n> -\t\t\tfalse\n> -\t\t\tbreak\n> -\t\t}\n> +\t\tgit update-index -- $f ||\n> +\t\tbreak\n> \tdone &&\n\nAh, these tests are evil, I remember them from the time when I was \nfiddling with Jonathan's patch. They can fail silently without \ntesting what they were supposed to test.\n\nIf something in the loop fails, the break will leave the loop but it \nwill do so with zero return value and, consequently, the test will \ncontinue as if everything were OK.\nAnd unless it was 'git update-index' that failed in a way that left a \nborked index behind, the 'git diff-index --cached' below will not \nerror out or produce some output that would cause the test to fail. \ni.e. I tried e.g.\n\n append_cr <$f >tmp && mv -f tmp $f && false &&\n\nin the loop and the test succeeded.\n\nI think the best fix would be to unroll the loop: after this patch the \nloop body consists of only two significant lines and we iterate \nthrough the loop only twice, so the test would be even shorter.\n\n> \tdiffers=$(git diff-index --cached HEAD) &&\n> -\ttest -z \"$differs\" || {\n> -\t\techo Oops \"$differs\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test -z \"$differs\"\n>\n> '\n>\n> @@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '\n> \tfor f in one dir/two\n> \tdo\n> \t\tappend_cr <$f >tmp && mv -f tmp $f &&\n> -\t\tgit update-index -- $f || {\n> -\t\t\techo \"Oops $f\"\n> -\t\t\tfalse\n> -\t\t\tbreak\n> -\t\t}\n> +\t\tgit update-index -- $f ||\n> +\t\tbreak\n> \tdone &&\n>\n> \tdiffers=$(git diff-index --cached HEAD) &&\n> -\ttest -z \"$differs\" || {\n> -\t\techo Oops \"$differs\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test -z \"$differs\"\n>\n> '\n>\n> @@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '\n> \tfor f in one dir/two\n> \tdo\n> \t\tremove_cr <\"$f\" >tmp && mv -f tmp $f &&\n> -\t\tgit update-index -- $f || {\n> -\t\t\techo \"Eh? $f\"\n> -\t\t\tfalse\n> -\t\t\tbreak\n> -\t\t}\n> +\t\tverbose git update-index -- $f ||\n> +\t\tbreak\n> \tdone &&\n> \ttest \"$one\" = $(git hash-object --stdin \ttest \"$two\" = $(git hash-object --stdin \tdiffers=$(git diff-index --cached HEAD) &&\n> -\ttest -z \"$differs\" || {\n> -\t\techo Oops \"$differs\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test -z \"$differs\"\n> '\n>\n> test_expect_success 'checkout with autocrlf=input' '\n> @@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '\n> \ttest \"$one\" = $(git hash-object --stdin \ttest \"$two\" = $(git hash-object --stdin \tdiffers=$(git diff-index --cached HEAD) &&\n> -\ttest -z \"$differs\" || {\n> -\t\techo Oops \"$differs\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test -z \"$differs\"\n> '\n>\n> test_expect_success 'apply patch (autocrlf=input)' '\n> @@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply patch.file &&\n> -\ttest \"$patched\" = \"$(git hash-object --stdin -\t\techo \"Eh? apply without index\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = \"$(git hash-object --stdin '\n>\n> test_expect_success 'apply patch --cached (autocrlf=input)' '\n> @@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached\n> (autocrlf=input)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply --cached patch.file &&\n> -\ttest \"$patched\" = $(git rev-parse :one) || {\n> -\t\techo \"Eh? apply with --cached\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = $(git rev-parse :one)\n> '\n>\n> test_expect_success 'apply patch --index (autocrlf=input)' '\n> @@ -226,11 +199,8 @@ test_expect_success 'apply patch --index\n> (autocrlf=input)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply --index patch.file &&\n> -\ttest \"$patched\" = $(git rev-parse :one) &&\n> -\ttest \"$patched\" = $(git hash-object --stdin -\t\techo \"Eh? apply with --index\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = $(git rev-parse :one) &&\n> +\tverbose test \"$patched\" = $(git hash-object --stdin '\n>\n> test_expect_success 'apply patch (autocrlf=true)' '\n> @@ -240,10 +210,7 @@ test_expect_success 'apply patch (autocrlf=true)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply patch.file &&\n> -\ttest \"$patched\" = \"$(remove_cr -\t\techo \"Eh? apply without index\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = \"$(remove_cr '\n>\n> test_expect_success 'apply patch --cached (autocrlf=true)' '\n> @@ -253,10 +220,7 @@ test_expect_success 'apply patch --cached\n> (autocrlf=true)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply --cached patch.file &&\n> -\ttest \"$patched\" = $(git rev-parse :one) || {\n> -\t\techo \"Eh? apply without index\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = $(git rev-parse :one)\n> '\n>\n> test_expect_success 'apply patch --index (autocrlf=true)' '\n> @@ -266,11 +230,8 @@ test_expect_success 'apply patch --index\n> (autocrlf=true)' '\n> \tgit read-tree --reset -u HEAD &&\n>\n> \tgit apply --index patch.file &&\n> -\ttest \"$patched\" = $(git rev-parse :one) &&\n> -\ttest \"$patched\" = \"$(remove_cr -\t\techo \"Eh? apply with --index\"\n> -\t\tfalse\n> -\t}\n> +\tverbose test \"$patched\" = $(git rev-parse :one) &&\n> +\tverbose test \"$patched\" = \"$(remove_cr '\n>\n> test_expect_success '.gitattributes says two is binary' '\n> @@ -326,21 +287,8 @@ test_expect_success '.gitattributes says two and\n> three are text' '\n> \techo \"t* crlf\" >.gitattributes &&\n> \tgit read-tree --reset -u HEAD &&\n>\n> -\tif has_cr dir/two\n> -\tthen\n> -\t\t: happy\n> -\telse\n> -\t\techo \"Huh?\"\n> -\t\tfalse\n> -\tfi &&\n> -\n> -\tif has_cr three\n> -\tthen\n> -\t\t: happy\n> -\telse\n> -\t\techo \"Huh?\"\n> -\t\tfalse\n> -\tfi\n> +\tverbose has_cr dir/two &&\n> +\tverbose has_cr three\n> '\n>\n> test_expect_success 'in-tree .gitattributes (1)' '\n> @@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' '\n> \trm -rf tmp one dir .gitattributes patch.file three &&\n> \tgit read-tree --reset -u HEAD &&\n>\n> -\tif has_cr one\n> -\tthen\n> -\t\techo \"Eh? one should not have CRLF\"\n> -\t\tfalse\n> -\telse\n> -\t\t: happy\n> -\tfi &&\n> -\thas_cr three || {\n> -\t\techo \"Eh? three should still have CRLF\"\n> -\t\tfalse\n> -\t}\n> +\ttest_must_fail has_cr one &&\n> +\tverbose has_cr three\n> '\n>\n> test_expect_success 'in-tree .gitattributes (2)' '\n> @@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' '\n> \tgit read-tree --reset HEAD &&\n> \tgit checkout-index -f -q -u -a &&\n>\n> -\tif has_cr one\n> -\tthen\n> -\t\techo \"Eh? one should not have CRLF\"\n> -\t\tfalse\n> -\telse\n> -\t\t: happy\n> -\tfi &&\n> -\thas_cr three || {\n> -\t\techo \"Eh? three should still have CRLF\"\n> -\t\tfalse\n> -\t}\n> +\ttest_must_fail has_cr one &&\n> +\tverbose has_cr three\n> '\n>\n> test_expect_success 'in-tree .gitattributes (3)' '\n> @@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' '\n> \tgit checkout-index -u .gitattributes &&\n> \tgit checkout-index -u one dir/two three &&\n>\n> -\tif has_cr one\n> -\tthen\n> -\t\techo \"Eh? one should not have CRLF\"\n> -\t\tfalse\n> -\telse\n> -\t\t: happy\n> -\tfi &&\n> -\thas_cr three || {\n> -\t\techo \"Eh? three should still have CRLF\"\n> -\t\tfalse\n> -\t}\n> +\ttest_must_fail has_cr one &&\n> +\tverbose has_cr three\n> '\n>\n> test_expect_success 'in-tree .gitattributes (4)' '\n> @@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' '\n> \tgit checkout-index -u one dir/two three &&\n> \tgit checkout-index -u .gitattributes &&\n>\n> -\tif has_cr one\n> -\tthen\n> -\t\techo \"Eh? one should not have CRLF\"\n> -\t\tfalse\n> -\telse\n> -\t\t: happy\n> -\tfi &&\n> -\thas_cr three || {\n> -\t\techo \"Eh? three should still have CRLF\"\n> -\t\tfalse\n> -\t}\n> +\ttest_must_fail has_cr one &&\n> +\tverbose has_cr three\n> '\n>\n> test_expect_success 'checkout with existing .gitattributes' '\n> --\n> 2.3.3.520.g3cfbb5d\n"},{"id":"258469","messageId":"20150325030022.Horde.yvJScxCdU4w4HDIV_xUwrw1@webmail.informatik.kit.edu","threadId":"38863","inReplyTo":"20150320101321.GU12543@peff.net","subject":"Re: [PATCH 21/25] t9001: use test_when_finished","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-25T02:00:22Z","receivedAt":"2015-03-25T02:00:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King :\n\n> The confirmation tests in t9001 all save the value of\n> sendemail.confirm, do something to it, then restore it at\n> the end, in a way that breaks the &&-chain (they are not\n> wrong, because they save the $? value, but it fools\n> --chain-lint).\n>\n> Instead, they can all use test_when_finished, and we can\n> even make the code simpler by factoring out the shared\n> lines.\n\nI think that saving the value of 'sendemail.confirm' is not necessary.\n\nThere are two blocks of confirmation tests, this patch concerns only tests\nof the second block. The first block of confirmation tests is nearly at\nthe beginning of the file in order to check the \"no confirm\" cases early.\nIf any of those fails the remainig tests in the file are skipped because\nthey might hang. The last of those tests sets 'sendemail.confirm' to\n'never' and leaves it so to avoid unintentional prompting in the remaining\ntests and then its value is not modified until that second block of\nconfirm tests are reached. This means that when those tests save the\nvalue of 'sendemail.confirm' they always save 'never'. Then why save it,\njust use test_when_finished to restore it to 'never' and all is well.\n\n\n> Note that we can _almost_ use test_config here, except that:\n>\n> 1. We do not restore the config with test_unconfig, but by\n> setting it back to some prior value.\n>\n> 2. We are not always setting a config variable. Sometimes\n> the change to be undone is unsetting it entirely.\n>\n> We could teach test_config to handle these cases, but it's\n> not worth the complexity for a single call-site.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t9001-send-email.sh | 30 ++++++++++--------------------\n> 1 file changed, 10 insertions(+), 20 deletions(-)\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 37caa18..c9f54d5 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' '\n> \ttest_confirm --confirm=compose --compose\n> '\n>\n> -test_expect_success $PREREQ 'confirm by default (due to cc)' '\n> +save_confirm () {\n> \tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\ttest_when_finished \"git config sendemail.confirm ${CONFIRM:-never}\"\n> +}\n> +\n> +test_expect_success $PREREQ 'confirm by default (due to cc)' '\n> +\tsave_confirm &&\n> \tgit config --unset sendemail.confirm &&\n> \ttest_confirm\n> -\tret=\"$?\"\n> -\tgit config sendemail.confirm ${CONFIRM:-never}\n> -\ttest $ret = \"0\"\n> '\n>\n> test_expect_success $PREREQ 'confirm by default (due to --compose)' '\n> -\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tsave_confirm &&\n> \tgit config --unset sendemail.confirm &&\n> \ttest_confirm --suppress-cc=all --compose\n> -\tret=\"$?\"\n> -\tgit config sendemail.confirm ${CONFIRM:-never}\n> -\ttest $ret = \"0\"\n> '\n>\n> test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '\n> -\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tsave_confirm &&\n> \tgit config --unset sendemail.confirm &&\n> \trm -fr outdir &&\n> \tgit format-patch -2 -o outdir &&\n> @@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF\n> (inform assumes y)' '\n> \t\t\t--to=nobody@example.com \\\n> \t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> \t\t\toutdir/*.patch -\tret=\"$?\"\n> -\tgit config sendemail.confirm ${CONFIRM:-never}\n> -\ttest $ret = \"0\"\n> '\n>\n> test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '\n> -\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tsave_confirm &&\n> \tgit config sendemail.confirm auto &&\n> \tGIT_SEND_EMAIL_NOTTY=1 &&\n> \texport GIT_SEND_EMAIL_NOTTY &&\n> @@ -861,13 +857,10 @@ test_expect_success $PREREQ 'confirm detects EOF\n> (auto causes failure)' '\n> \t\t\t--to=nobody@example.com \\\n> \t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> \t\t\t$patches -\tret=\"$?\"\n> -\tgit config sendemail.confirm ${CONFIRM:-never}\n> -\ttest $ret = \"0\"\n> '\n>\n> test_expect_success $PREREQ 'confirm does not loop forever' '\n> -\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tsave_confirm &&\n> \tgit config sendemail.confirm auto &&\n> \tGIT_SEND_EMAIL_NOTTY=1 &&\n> \texport GIT_SEND_EMAIL_NOTTY &&\n> @@ -876,9 +869,6 @@ test_expect_success $PREREQ 'confirm does not loop\n> forever' '\n> \t\t\t--to=nobody@example.com \\\n> \t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> \t\t\t$patches\n> -\tret=\"$?\"\n> -\tgit config sendemail.confirm ${CONFIRM:-never}\n> -\ttest $ret = \"0\"\n> '\n>\n> test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n> --\n> 2.3.3.520.g3cfbb5d\n"},{"id":"258470","messageId":"20150325024502.GA15051@peff.net","threadId":"38863","inReplyTo":"20150325005120.Horde.ClYI1WJflnvTzVgcL6i8Sg8@webmail.informatik.kit.edu","subject":"Re: [PATCH 18/25] t1301: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T02:45:03Z","receivedAt":"2015-03-25T02:45:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 12:51:20AM +0100, SZEDER Gábor wrote:\n\n> >@@ -33,7 +32,7 @@ do\n> > \t\t\tgit init --shared=1 &&\n> > \t\t\ttest 1 = \"$(git config core.sharedrepository)\"\n> > \t\t) &&\n> >-\t\tactual=$(ls -l sub/.git/HEAD)\n> >+\t\tactual=$(ls -l sub/.git/HEAD) &&\n> > \t\tcase \"$actual\" in\n> > \t\t-rw-rw-r--*)\n> > \t\t\t: happy\n> \n> This hunk could go into the \"moderate &&-chain breakage\" patch.\n> Doesn't really matter, what matters most is that it's fixed, but I really\n> liked your classification of missing &&s in the early patches.\n\nYeah, these later ones are a mish-mash of real fixes and just quieting\n--chain-lint. I pulled out ones that I felt needed a little more\nexplanation, and generally kept changes for a file together (though I am\nnot sure not always). I'm not sure it's worth the effort to go through\nanother round of classifying.\n\n-Peff\n"},{"id":"258471","messageId":"20150325024710.GB15051@peff.net","threadId":"38863","inReplyTo":"20150325030022.Horde.yvJScxCdU4w4HDIV_xUwrw1@webmail.informatik.kit.edu","subject":"Re: [PATCH 21/25] t9001: use test_when_finished","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T02:47:10Z","receivedAt":"2015-03-25T02:47:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 03:00:22AM +0100, SZEDER Gábor wrote:\n\n> >Instead, they can all use test_when_finished, and we can\n> >even make the code simpler by factoring out the shared\n> >lines.\n> \n> I think that saving the value of 'sendemail.confirm' is not necessary.\n> \n> There are two blocks of confirmation tests, this patch concerns only tests\n> of the second block. The first block of confirmation tests is nearly at\n> the beginning of the file in order to check the \"no confirm\" cases early.\n> If any of those fails the remainig tests in the file are skipped because\n> they might hang. The last of those tests sets 'sendemail.confirm' to\n> 'never' and leaves it so to avoid unintentional prompting in the remaining\n> tests and then its value is not modified until that second block of\n> confirm tests are reached. This means that when those tests save the\n> value of 'sendemail.confirm' they always save 'never'. Then why save it,\n> just use test_when_finished to restore it to 'never' and all is well.\n\nYeah, I suspected this while writing it the patch, but I preferred to\nkeep it more obvious that there would be no accidental regression, since\nthe series was already so long (and also because calling save_confirm is\nnot any worse than test_when_finished).\n\nI don't mind a patch on top simplifying out save_confirm, if you're\nconfident that's what we're always saving.\n\n-Peff\n"},{"id":"258472","messageId":"20150325035352.Horde.wlh5PImm_EGVzNTHeSEb7g1@webmail.informatik.kit.edu","threadId":"38863","inReplyTo":"20150320100548.GA12543@peff.net","subject":"Re: [PATCH 01/25] t/test-lib: introduce --chain-lint option","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-03-25T02:53:52Z","receivedAt":"2015-03-25T02:53:52Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jeff King :\n\n> However, there are a number of places it cannot reach:\n>\n> - it cannot find a failure to break out of loops on error,\n> like:\n>\n> cmd1 &&\n> for i in a b c; do\n> \t cmd2 $i\n> done &&\n> cmd3\n>\n> which will not notice failures of \"cmd2 a\" or \"cmd b\"\n\ns/cmd b/cmd2 b/ ?\n\n> - it cannot find a missing &&-chain inside a block or\n> subfunction, like:\n>\n> foo () {\n> \t cmd1\n> \t cmd2\n> }\n>\n> foo &&\n> bar\n>\n> which will not notice a failure of cmd1.\n\nAnd inside subshells. I think it's worth mentioning, too, because\nsubshells are used frequently when setting environment variables\n\n ( GIT_FOO=bar && export GIT_FOO && cmd1 && ... ) && test_cmp\n\nor changing directory\n\n ( cd subdir && cmd1 && ... ) && test_cmp\n\nI was wondering whether we could do better here with helper functions,\nsomething along the lines of:\n\n # Set and export environment variable, automatically unsetting it after\n # the test is over\n test_setenv () {\n eval \"$1=\\$2\" &&\n export \"$1\" &&\n # sane_unset, to allow unsetting during the test\n test_when_finished \"sane_unset '$1'\"\n }\n\n # Change to given directory, automatically return to current working\n # directory after the test is over\n test_cd () {\n test_when_finished \"cd '$PWD'\" &&\n cd \"$1\"\n }\n\nWith these the above examples would become\n\n test_setenv GIT_FOO bar && cmd1 && ... && test_cmp\n\nand\n\n test_cd subdir && cmd1 && ... && test_cmp\n\nwhich means increased coverage for --chain-lint.\n\n\nThanks for working on this. I looked into this after seeing Jonathan's\npatch back then, got quite far but never reached a chain-lint-clean\nstate, and only sent patches for the two most amusing breakages\n(ddeaf7ef0d, 056f34bbcd).\nI'm glad it's off my TODO list and I don't have to rebase a 1.5 year old\nbranch to current master :)\n\nGábor\n\n> - it only checks tests that you run; every platform will\n> have some tests skipped due to missing prequisites,\n> so it's impossible to say from one run that the test\n> suite is free of broken &&-chains. However, all tests get\n> run by _somebody_, so eventually we will notice problems.\n>\n> - it does not operate on test_when_finished or prerequisite\n> blocks. It could, but these tends to be much shorter and\n> less of a problem, so I punted on them in this patch.\n>\n> This patch was inspired by an earlier patch by Jonathan\n> Nieder:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/235913\n>\n> This implementation and all bugs are mine.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/README | 10 ++++++++++\n> t/test-lib.sh | 16 ++++++++++++++++\n> 2 files changed, 26 insertions(+)\n>\n> diff --git a/t/README b/t/README\n> index d5bb0c9..35438bc 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -168,6 +168,16 @@ appropriately before running \"make\".\n> \tUsing this option with a RAM-based filesystem (such as tmpfs)\n> \tcan massively speed up the test suite.\n>\n> +--chain-lint::\n> +--no-chain-lint::\n> +\tIf --chain-lint is enabled, the test harness will check each\n> +\ttest to make sure that it properly \"&&-chains\" all commands (so\n> +\tthat a failure in the middle does not go unnoticed by the final\n> +\texit code of the test). This check is performed in addition to\n> +\trunning the tests themselves. You may also enable or disable\n> +\tthis feature by setting the GIT_TEST_CHAIN_LINT environment\n> +\tvariable to \"1\" or \"0\", respectively.\n> +\n> You can also set the GIT_TEST_INSTALLED environment variable to\n> the bindir of an existing git installation to test that installation.\n> You still need to have built this git sandbox, from which various\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index c096778..50b3d3f 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -232,6 +232,12 @@ do\n> \t--root=*)\n> \t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n> \t\tshift ;;\n> +\t--chain-lint)\n> +\t\tGIT_TEST_CHAIN_LINT=1\n> +\t\tshift ;;\n> +\t--no-chain-lint)\n> +\t\tGIT_TEST_CHAIN_LINT=0\n> +\t\tshift ;;\n> \t-x)\n> \t\ttrace=t\n> \t\tverbose=t\n> @@ -524,6 +530,16 @@ test_eval_ () {\n> test_run_ () {\n> \ttest_cleanup=:\n> \texpecting_failure=$2\n> +\n> +\tif test \"${GIT_TEST_CHAIN_LINT:-0}\" != 0; then\n> +\t\t# 117 is magic because it is unlikely to match the exit\n> +\t\t# code of other programs\n> +\t\ttest_eval_ \"(exit 117) && $1\"\n> +\t\tif test \"$?\" != 117; then\n> +\t\t\terror \"bug in the test script: broken &&-chain: $1\"\n> +\t\tfi\n> +\tfi\n> +\n> \tsetup_malloc_check\n> \ttest_eval_ \"$1\"\n> \teval_ret=$?\n> --\n> 2.3.3.520.g3cfbb5d\n"},{"id":"258473","messageId":"20150325025635.GC15051@peff.net","threadId":"38863","inReplyTo":"20150325012323.Horde.zCWvV1mF8OBE1PxYPuuEFg8@webmail.informatik.kit.edu","subject":"Re: [PATCH 17/25] t0020: use modern test_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T02:56:35Z","receivedAt":"2015-03-25T02:56:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 01:23:23AM +0100, SZEDER Gábor wrote:\n\n> > \tfor f in one dir/two\n> > \tdo\n> > \t\tappend_cr <$f >tmp && mv -f tmp $f &&\n> >-\t\tgit update-index -- $f || {\n> >-\t\t\techo Oops\n> >-\t\t\tfalse\n> >-\t\t\tbreak\n> >-\t\t}\n> >+\t\tgit update-index -- $f ||\n> >+\t\tbreak\n> > \tdone &&\n> \n> Ah, these tests are evil, I remember them from the time when I was fiddling\n> with Jonathan's patch. They can fail silently without testing what they\n> were supposed to test.\n> \n> If something in the loop fails, the break will leave the loop but it will do\n> so with zero return value and, consequently, the test will continue as if\n> everything were OK.\n> And unless it was 'git update-index' that failed in a way that left a borked\n> index behind, the 'git diff-index --cached' below will not error out or\n> produce some output that would cause the test to fail. i.e. I tried e.g.\n> \n> append_cr <$f >tmp && mv -f tmp $f && false &&\n> \n> in the loop and the test succeeded.\n\nUgh, you're right. I remembered that for loops were tricky in &&-chains,\nbut for some reason was thinking that \"break\" would give you the last\nexit code, But I just re-tested, and of course it does not work.\n\n> I think the best fix would be to unroll the loop: after this patch the loop\n> body consists of only two significant lines and we iterate through the loop\n> only twice, so the test would be even shorter.\n\nYeah, unrolling may be the best thing here, given the size of the loops.\nAs a general rule, I think it has to be a subshell with an exit, like:\n\n (\n\tfor i in one two three; do\n\t\techo $i &&\n\t\ttest $i = one ||\n\t\texit 1\n\tdone\n )\n echo exit=$?\n\nwhich should yield one, two, and exit=1. 7b1732c (t7510: use consistent\n&&-chains in loop, 2014-06-16) deals with this in another test.\n\n-Peff\n"},{"id":"258474","messageId":"20150325030544.GD15051@peff.net","threadId":"38863","inReplyTo":"20150325035352.Horde.wlh5PImm_EGVzNTHeSEb7g1@webmail.informatik.kit.edu","subject":"Re: [PATCH 01/25] t/test-lib: introduce --chain-lint option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T03:05:44Z","receivedAt":"2015-03-25T03:05:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 03:53:52AM +0100, SZEDER Gábor wrote:\n\n> > cmd1 &&\n> > for i in a b c; do\n> >\t cmd2 $i\n> > done &&\n> > cmd3\n> >\n> > which will not notice failures of \"cmd2 a\" or \"cmd b\"\n> \n> s/cmd b/cmd2 b/ ?\n\nYes, but the patches are already in next, so it is sadly too late for\ncommit message fixups.\n\n> > - it cannot find a missing &&-chain inside a block or\n> > subfunction, like:\n> [...]\n> And inside subshells. [...]\n\nYeah, I had mentally filed them with \"block\", but true subshells are\nprobably the most common place. However, I'd suspect a good portion of\nthem are going to be the \"trivial\" type, especially if they involve\nsetting up the sub-environment at the top of the subshell. E.g.,\nsomething like this:\n\n cmd1 &&\n (\n FOO=bar; export FOO\n cmd2\n ) &&\n cmd3\n\ndoes not break the outer chain (which is what --chain-lint checks). It\ndoes break the chain inside the subshell, but we never expect variable\nassignment or export to fail (it is nice to fix it, of course, but the\nmain purpose in fixing the ones in my \"trivial\" patch was more about\nshutting up --chain-lint to make the real breakages more obvious).\n\n-Peff\n"},{"id":"258475","messageId":"20150325052456.GA19394@peff.net","threadId":"38863","inReplyTo":"20150325025635.GC15051@peff.net","subject":"[PATCH 0/8] more &&-chaining test fixups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:24:56Z","receivedAt":"2015-03-25T05:24:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's what I found looking for loops like:\n\n for i in a b c; do\n something_important $i || break\n done &&\n something_else\n\nwhich presumably expect the chain to stop when something_important fails\nfor any loop element. The solutions are one of (depending on the\nsurrounding code):\n\n 1. Switch the break to \"return 1\". The tests are all &&-chained, so\n the effect of a failed command is to exit the test immediately\n anyway. And we wrap our eval'd test snippets inside an extra layer\n of function call to explicitly allow early returns like this.\n\n 2. Switch the break to \"exit 1\". Calling \"return\" from a subshell\n inside a function is a bit weird. It doesn't exit the function at\n all, but rather just the subshell (in both bash and dash). But if\n you are not in a function, calling \"return\" at all is an error in\n bash (subshell or no), and OK in dash (where it acts like \"exit\").\n POSIX explicitly marks the \"outside of a function\" behavior as\n unspecified, but I couldn't find anything about the subshell\n behavior.\n\n So I'm loathe to depend on it, even though it does seem to do what\n we want, as I do not want to even think what some more obscure\n shells might do with it. And especially because we know that \"exit\n 1\" portably does what we want (the only downside is that you have\n to recognize which situation you are in and use \"exit\" versus\n \"return\").\n\n 3. Unroll the loops. In some cases the result is actually shorter, and\n (IMHO) more readable.\n\nThese sites were all found with:\n\n git grep -E '(^|\\|\\|)[ \t]*break' t/t*.sh\n\n(that's a space and a tab in the brackets). There are some matches\nthere that I did not touch, because they were already fine. E.g., t5302\nand t7700 use loops to assign a value to a variable and break out early,\nand then check the value of the variable.\n\nThat's just the tip of the iceberg, though. Searching for\n\n git grep 'for .* in '\n\nyields hundreds of hits. Most of which are probably fine (quite a few\nare outside &&-chains entirely). I focused on the ones that called\nbreak, because that indicated to me that the author was trying to\naddress the &&-chain. Certainly anybody else is welcome to take a stab\nat the rest, but I'm also happy to fix them up as we touch nearby code\nand notice them. Most of the loops are in setup code that we do not\nexpect to fail anyway, so examining them is a lot of work for a little\ngain.\n\nThere were a few legitimate problems, though. I've ordered the patches\nbelow by descending severity. These apply on top of jk/test-chain-lint.\n\n [1/8]: perf-lib: fix ignored exit code inside loop\n [2/8]: t0020: fix ignored exit code inside loops\n [3/8]: t3305: fix ignored exit code inside loop\n [4/8]: t7701: fix ignored exit code inside loop\n\n These four are actual bugs.\n\n [5/8]: t: fix some trivial cases of ignored exit codes in loops\n\n These ones are in setup code, and so would almost certainly never\n fail.\n\n [6/8]: t: simplify loop exit-code status variables\n [7/8]: t0020: use test_* helpers instead of hand-rolled messages\n [8/8]: t9001: drop save_confirm helper\n\n These last three are pure cleanup, no behavior changes. The last two\n are not even strictly related to the same topic, but I noticed them\n while in the area.\n\n-Peff\n"},{"id":"258476","messageId":"20150325052555.GA31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 1/8] perf-lib: fix ignored exit code inside loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:25:55Z","receivedAt":"2015-03-25T05:25:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When copying the test repository, we try to detect whether\nthe copy succeeded. However, most of the heavy lifting is\ndone inside a for loop, where our \"break\" will lose the exit\ncode of the failing \"cp\". We can take advantage of the fact\nthat we are in a subshell, and just \"exit 1\" to break out\nwith a code.\n\nSigned-off-by: Jeff King \n---\n t/perf/perf-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\nindex a8c9574..5cf74ed 100644\n--- a/t/perf/perf-lib.sh\n+++ b/t/perf/perf-lib.sh\n@@ -91,7 +91,7 @@ test_perf_create_repo_from () {\n \t\t\t\t*/objects|*/hooks|*/config)\n \t\t\t\t\t;;\n \t\t\t\t*)\n-\t\t\t\t\tcp -R \"$stuff\" . || break\n+\t\t\t\t\tcp -R \"$stuff\" . || exit 1\n \t\t\t\t\t;;\n \t\t\tesac\n \t\tdone &&\n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258477","messageId":"20150325052844.GB31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 2/8] t0020: fix ignored exit code inside loops","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:28:44Z","receivedAt":"2015-03-25T05:28:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"A loop like:\n\n for f in one two; do\n\t something $f ||\n\t break\n done\n\nwill correctly break out of the loop when we see a failure\nof one item, but the resulting exit code will always be\nzero. We can fix that by putting the loop into a function or\nsubshell, but in this case it is simpler still to just\nunroll the loop. We do add a helper function, which\nhopefully makes the end result even more readable (in\naddition to being shorter).\n\nReported-by: SZEDER Gábor \nSigned-off-by: Jeff King \n---\nYou can make each call site a one-liner that does the loop inside the\nfunction (including the update-index call). But I tried to shoot for\nreadability rather than absolute minimum characters.\n\n t/t0020-crlf.sh | 54 +++++++++++++++++++-----------------------------------\n 1 file changed, 19 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 9fa26df..144fdcd 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -8,6 +8,13 @@ has_cr() {\n \ttr '\\015' Q <\"$1\" | grep Q >/dev/null\n }\n \n+# add or remove CRs to disk file in-place\n+# usage: munge_cr \n+munge_cr () {\n+\t\"${1}_cr\" <\"$2\" >tmp &&\n+\tmv tmp \"$2\"\n+}\n+\n test_expect_success setup '\n \n \tgit config core.autocrlf false &&\n@@ -100,14 +107,9 @@ test_expect_success 'update with autocrlf=input' '\n \trm -f tmp one dir/two three &&\n \tgit read-tree --reset -u HEAD &&\n \tgit config core.autocrlf input &&\n-\n-\tfor f in one dir/two\n-\tdo\n-\t\tappend_cr <$f >tmp && mv -f tmp $f &&\n-\t\tgit update-index -- $f ||\n-\t\tbreak\n-\tdone &&\n-\n+\tmunge_cr append one &&\n+\tmunge_cr append dir/two &&\n+\tgit update-index -- one dir/two &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n \tverbose test -z \"$differs\"\n \n@@ -118,14 +120,9 @@ test_expect_success 'update with autocrlf=true' '\n \trm -f tmp one dir/two three &&\n \tgit read-tree --reset -u HEAD &&\n \tgit config core.autocrlf true &&\n-\n-\tfor f in one dir/two\n-\tdo\n-\t\tappend_cr <$f >tmp && mv -f tmp $f &&\n-\t\tgit update-index -- $f ||\n-\t\tbreak\n-\tdone &&\n-\n+\tmunge_cr append one &&\n+\tmunge_cr append dir/two &&\n+\tgit update-index -- one dir/two &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n \tverbose test -z \"$differs\"\n \n@@ -136,13 +133,9 @@ test_expect_success 'checkout with autocrlf=true' '\n \trm -f tmp one dir/two three &&\n \tgit config core.autocrlf true &&\n \tgit read-tree --reset -u HEAD &&\n-\n-\tfor f in one dir/two\n-\tdo\n-\t\tremove_cr <\"$f\" >tmp && mv -f tmp $f &&\n-\t\tverbose git update-index -- $f ||\n-\t\tbreak\n-\tdone &&\n+\tmunge_cr remove one &&\n+\tmunge_cr remove dir/two &&\n+\tgit update-index -- one dir/two &&\n \ttest \"$one\" = $(git hash-object --stdin \n---\n t/t3305-notes-fanout.sh | 9 +++------\n 1 file changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh\nindex b1ea64b..54460be 100755\n--- a/t/t3305-notes-fanout.sh\n+++ b/t/t3305-notes-fanout.sh\n@@ -51,15 +51,12 @@ test_expect_success 'deleting most notes with git-notes' '\n \tnum_notes=250 &&\n \ti=0 &&\n \tgit rev-list HEAD |\n-\twhile read sha1\n+\twhile test $i -lt $num_notes && read sha1\n \tdo\n \t\ti=$(($i + 1)) &&\n-\t\tif test $i -gt $num_notes\n-\t\tthen\n-\t\t\tbreak\n-\t\tfi &&\n \t\ttest_tick &&\n-\t\tgit notes remove \"$sha1\"\n+\t\tgit notes remove \"$sha1\" ||\n+\t\texit 1\n \tdone\n '\n \n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258479","messageId":"20150325052909.GD31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 4/8] t7701: fix ignored exit code inside loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:29:10Z","receivedAt":"2015-03-25T05:29:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When checking a list of file mtimes, we use a loop and break\nout early from the loop if any entry does not match.\nHowever, the exit code of a loop exited via break is always\n0, meaning that the test will fail to notice we had a\nmismatch. Since the loop is inside a function, we can fix\nthis by doing an early \"return 1\".\n\nSigned-off-by: Jeff King \n---\n t/t7701-repack-unpack-unreachable.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex aad8a9c..b66e383 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -57,7 +57,7 @@ compare_mtimes ()\n {\n \tread tref rest &&\n \twhile read t rest; do\n-\t\ttest \"$tref\" = \"$t\" || break\n+\t\ttest \"$tref\" = \"$t\" || return 1\n \tdone\n }\n \n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258480","messageId":"20150325052952.GE31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:29:52Z","receivedAt":"2015-03-25T05:29:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are all cases where we do a setup step of the form:\n\n for i in $foo; do\n\t set_up $i || break\n done &&\n more_setup\n\nwould not notice a failure in set_up (because break always\nreturns a 0 exit code). These are just setup steps that we\ndo not expect to fail, but it does not hurt to be defensive.\n\nMost can be fixed by converting the \"break\" to a \"return 1\"\n(since we eval our tests inside a function for just this\npurpose). A few of the loops are inside subshells, so we can\nuse just \"exit 1\" to break out of the subshell. And a few\ncan actually be made shorter by just unrolling the loop.\n\nSigned-off-by: Jeff King \n---\n t/t3010-ls-files-killed-modified.sh | 11 ++++-------\n t/t3031-merge-criscross.sh | 2 +-\n t/t3202-show-branch-octopus.sh | 2 +-\n t/t4024-diff-optimize-common.sh | 2 +-\n t/t4046-diff-unmerged.sh | 8 ++++----\n t/t4151-am-abort.sh | 2 +-\n t/t5505-remote.sh | 8 ++++----\n t/t5514-fetch-multiple.sh | 4 ++--\n t/t6026-merge-attr.sh | 6 +++---\n t/t6040-tracking-info.sh | 7 +++----\n 10 files changed, 24 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh\nindex 62fce10..580e158 100755\n--- a/t/t3010-ls-files-killed-modified.sh\n+++ b/t/t3010-ls-files-killed-modified.sh\n@@ -55,13 +55,10 @@ test_expect_success 'git update-index --add to add various paths.' '\n \t: >path9 &&\n \tdate >path10 &&\n \tgit update-index --add -- path0 path?/file? pathx/ju path7 path8 path9 path10 &&\n-\tfor i in 1 2\n-\tdo\n-\t\tgit init submod$i &&\n-\t\t(\n-\t\t\tcd submod$i && git commit --allow-empty -m \"empty $i\"\n-\t\t) || break\n-\tdone &&\n+\tgit init submod1 &&\n+\tgit -C submod1 commit --allow-empty -m \"empty 1\" &&\n+\tgit init submod2 &&\n+\tgit -C submod2 commit --allow-empty -m \"empty 2\" &&\n \tgit update-index --add submod[12] &&\n \t(\n \t\tcd submod1 &&\ndiff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh\nindex 7f41607..e59b0a3 100755\n--- a/t/t3031-merge-criscross.sh\n+++ b/t/t3031-merge-criscross.sh\n@@ -32,7 +32,7 @@ test_expect_success 'setup repo with criss-cross history' '\n \tdo\n \t\techo $n > data/$n &&\n \t\tn=$(($n+1)) ||\n-\t\tbreak\n+\t\treturn 1\n \tdone &&\n \n \t# check them in\ndiff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh\nindex 0a5d5e6..6adf478 100755\n--- a/t/t3202-show-branch-octopus.sh\n+++ b/t/t3202-show-branch-octopus.sh\n@@ -19,7 +19,7 @@ test_expect_success 'setup' '\n \t\t> file$i &&\n \t\tgit add file$i &&\n \t\ttest_tick &&\n-\t\tgit commit -m branch$i || break\n+\t\tgit commit -m branch$i || return 1\n \tdone\n \n '\ndiff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh\nindex c4d733f..7e76018 100755\n--- a/t/t4024-diff-optimize-common.sh\n+++ b/t/t4024-diff-optimize-common.sh\n@@ -139,7 +139,7 @@ test_expect_success setup '\n \t\t( printf C; zs $n ) >file-c$n &&\n \t\t( echo D; zs $n ) >file-d$n &&\n \n-\t\texpect_pattern $n || break\n+\t\texpect_pattern $n || return 1\n \n \tdone >expect\n '\ndiff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh\nindex 25d50a6..d0f1447 100755\n--- a/t/t4046-diff-unmerged.sh\n+++ b/t/t4046-diff-unmerged.sh\n@@ -8,7 +8,7 @@ test_expect_success setup '\n \tdo\n \t\tblob=$(echo $i | git hash-object --stdin) &&\n \t\teval \"blob$i=$blob\" &&\n-\t\teval \"m$i=\\\"100644 \\$blob$i $i\\\"\" || break\n+\t\teval \"m$i=\\\"100644 \\$blob$i $i\\\"\" || return 1\n \tdone &&\n \tpaths= &&\n \tfor b in o x\n@@ -24,9 +24,9 @@ test_expect_success setup '\n \t\t\t\tcase \"$b\" in x) echo \"$m1$p\" ;; esac &&\n \t\t\t\tcase \"$o\" in x) echo \"$m2$p\" ;; esac &&\n \t\t\t\tcase \"$t\" in x) echo \"$m3$p\" ;; esac ||\n-\t\t\t\tbreak\n-\t\t\tdone || break\n-\t\tdone || break\n+\t\t\t\treturn 1\n+\t\t\tdone\n+\t\tdone\n \tdone >ls-files-s.expect &&\n \tgit update-index --index-info ls-files-s.actual &&\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex 1176bcc..8d90634 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -20,7 +20,7 @@ test_expect_success setup '\n \t\techo $i >otherfile-$i &&\n \t\tgit add otherfile-$i &&\n \t\ttest_tick &&\n-\t\tgit commit -a -m $i || break\n+\t\tgit commit -a -m $i || return 1\n \tdone &&\n \tgit format-patch --no-numbered initial &&\n \tgit checkout -b side initial &&\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 17c6330..7a8499c 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -579,7 +579,7 @@ test_expect_success 'update with arguments' '\n \t\tcd one &&\n \t\tfor b in $(git branch -r)\n \t\tdo\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t\tdone &&\n \t\tgit remote add manduca ../mirror &&\n \t\tgit remote add megaloprepus ../mirror &&\n@@ -622,7 +622,7 @@ test_expect_success 'update default' '\n \t\tcd one &&\n \t\tfor b in $(git branch -r)\n \t\tdo\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t\tdone &&\n \t\tgit config remote.drosophila.skipDefaultUpdate true &&\n \t\tgit remote update default &&\n@@ -642,7 +642,7 @@ test_expect_success 'update default (overridden, with funny whitespace)' '\n \t\tcd one &&\n \t\tfor b in $(git branch -r)\n \t\tdo\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t\tdone &&\n \t\tgit config remotes.default \"$(printf \"\\t drosophila \\n\")\" &&\n \t\tgit remote update default &&\n@@ -656,7 +656,7 @@ test_expect_success 'update (with remotes.default defined)' '\n \t\tcd one &&\n \t\tfor b in $(git branch -r)\n \t\tdo\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t\tdone &&\n \t\tgit config remotes.default \"drosophila\" &&\n \t\tgit remote update &&\ndiff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh\nindex 0f81409..4b4b667 100755\n--- a/t/t5514-fetch-multiple.sh\n+++ b/t/t5514-fetch-multiple.sh\n@@ -120,7 +120,7 @@ test_expect_success 'git fetch --all (skipFetchAll)' '\n \t(cd test4 &&\n \t for b in $(git branch -r)\n \t do\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t done &&\n \t git remote add three ../three &&\n \t git config remote.three.skipFetchAll true &&\n@@ -144,7 +144,7 @@ test_expect_success 'git fetch --multiple (ignoring skipFetchAll)' '\n \t(cd test4 &&\n \t for b in $(git branch -r)\n \t do\n-\t\tgit branch -r -d $b || break\n+\t\tgit branch -r -d $b || exit 1\n \t done &&\n \t git fetch --multiple one two three &&\n \t git branch -r > output &&\ndiff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh\nindex 5e43997..3c21938 100755\n--- a/t/t6026-merge-attr.sh\n+++ b/t/t6026-merge-attr.sh\n@@ -11,7 +11,7 @@ test_expect_success setup '\n \n \tfor f in text binary union\n \tdo\n-\t\techo Initial >$f && git add $f || break\n+\t\techo Initial >$f && git add $f || return 1\n \tdone &&\n \ttest_tick &&\n \tgit commit -m Initial &&\n@@ -19,7 +19,7 @@ test_expect_success setup '\n \tgit branch side &&\n \tfor f in text binary union\n \tdo\n-\t\techo Master >>$f && git add $f || break\n+\t\techo Master >>$f && git add $f || return 1\n \tdone &&\n \ttest_tick &&\n \tgit commit -m Master &&\n@@ -27,7 +27,7 @@ test_expect_success setup '\n \tgit checkout side &&\n \tfor f in text binary union\n \tdo\n-\t\techo Side >>$f && git add $f || break\n+\t\techo Side >>$f && git add $f || return 1\n \tdone &&\n \ttest_tick &&\n \tgit commit -m Side &&\ndiff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh\nindex 7ac8fd0..3d5c238 100755\n--- a/t/t6040-tracking-info.sh\n+++ b/t/t6040-tracking-info.sh\n@@ -12,10 +12,9 @@ advance () {\n }\n \n test_expect_success setup '\n-\tfor i in a b c;\n-\tdo\n-\t\tadvance $i || break\n-\tdone &&\n+\tadvance a &&\n+\tadvance b &&\n+\tadvance c &&\n \tgit clone . test &&\n \t(\n \t\tcd test &&\n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258481","messageId":"20150325053017.GF31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 6/8] t: simplify loop exit-code status variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:30:17Z","receivedAt":"2015-03-25T05:30:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since shell loops may drop the exit code of failed commands\ninside the loop, some tests try to keep track of the status\nby setting a variable. This can end up cumbersome and hard\nto read; it is much simpler to just exit directly from the\nloop using \"return 1\" (since each case is either in a helper\nfunction or inside a test snippet).\n\nSigned-off-by: Jeff King \n---\n t/t3060-ls-files-with-tree.sh | 13 ++++---------\n t/t3901-i18n-patch.sh | 8 ++------\n 2 files changed, 6 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\nindex 61c1f53..36b10f7 100755\n--- a/t/t3060-ls-files-with-tree.sh\n+++ b/t/t3060-ls-files-with-tree.sh\n@@ -25,15 +25,10 @@ test_expect_success setup '\n \t\tdo\n \t\t\tnum=00$n$m &&\n \t\t\t>sub/file-$num &&\n-\t\t\techo file-$num >>expected || {\n-\t\t\t\tbad=t\n-\t\t\t\tbreak\n-\t\t\t}\n-\t\tdone && test -z \"$bad\" || {\n-\t\t\tbad=t\n-\t\t\tbreak\n-\t\t}\n-\tdone && test -z \"$bad\" &&\n+\t\t\techo file-$num >>expected ||\n+\t\t\treturn 1\n+\t\tdone\n+\tdone &&\n \tgit add . &&\n \tgit commit -m \"add a bunch of files\" &&\n \ndiff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh\nindex a392f3d..75cf3ff 100755\n--- a/t/t3901-i18n-patch.sh\n+++ b/t/t3901-i18n-patch.sh\n@@ -9,7 +9,7 @@ test_description='i18n settings and format-patch | am pipe'\n \n check_encoding () {\n \t# Make sure characters are not corrupted\n-\tcnt=\"$1\" header=\"$2\" i=1 j=0 bad=0\n+\tcnt=\"$1\" header=\"$2\" i=1 j=0\n \twhile test \"$i\" -le $cnt\n \tdo\n \t\tgit format-patch --encoding=UTF-8 --stdout HEAD~$i..HEAD~$j |\n@@ -20,14 +20,10 @@ check_encoding () {\n \t\t\tgrep \"^encoding ISO8859-1\" ;;\n \t\t*)\n \t\t\tgrep \"^encoding ISO8859-1\"; test \"$?\" != 0 ;;\n-\t\tesac || {\n-\t\t\tbad=1\n-\t\t\tbreak\n-\t\t}\n+\t\tesac || return 1\n \t\tj=$i\n \t\ti=$(($i+1))\n \tdone\n-\t(exit $bad)\n }\n \n test_expect_success setup '\n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258482","messageId":"20150325053141.GG31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:31:41Z","receivedAt":"2015-03-25T05:31:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These tests are not wrong, but it is much shorter and more\nidiomatic to say \"verbose\" or \"test_must_fail\" rather than\nprinting our own messages on failure. Likewise, there is no\nneed to say \"happy\" at the end of a test; the test suite\ntakes care of that.\n\nSigned-off-by: Jeff King \n---\nI somehow missed these when doing 9157c5c in the earlier series.\n\n t/t0020-crlf.sh | 38 +++++---------------------------------\n 1 file changed, 5 insertions(+), 33 deletions(-)\n\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 144fdcd..f94120a 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -35,9 +35,7 @@ test_expect_success setup '\n \tfor w in Some extra lines here; do echo $w; done >>one &&\n \tgit diff >patch.file &&\n \tpatched=$(git hash-object --stdin .gitattributes &&\n \tgit read-tree --reset -u HEAD &&\n \n-\tif has_cr dir/two\n-\tthen\n-\t\techo \"Huh?\"\n-\t\tfalse\n-\telse\n-\t\t: happy\n-\tfi\n+\ttest_must_fail has_cr dir/two\n '\n \n test_expect_success '.gitattributes says two and three are text' '\n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258483","messageId":"20150325053220.GH31924@peff.net","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"[PATCH 8/8] t9001: drop save_confirm helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T05:32:20Z","receivedAt":"2015-03-25T05:32:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The idea of this helper is that we want to save the current\nvalue of a config variable and then restore it again after\nthe test completes. However, there's no point in actually\nsaving the value; it should always be restored to the string\n\"never\" (which you can confirm by instrumenting\nsave_confirm to print the value it finds).\n\nLet's just replace it with a single test_when_finished call.\n\nSuggested-by: SZEDER Gábor \nSigned-off-by: Jeff King \n---\n\n t/t9001-send-email.sh | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex c9f54d5..7be14a4 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -817,25 +817,20 @@ test_expect_success $PREREQ '--confirm=compose' '\n \ttest_confirm --confirm=compose --compose\n '\n \n-save_confirm () {\n-\tCONFIRM=$(git config --get sendemail.confirm) &&\n-\ttest_when_finished \"git config sendemail.confirm ${CONFIRM:-never}\"\n-}\n-\n test_expect_success $PREREQ 'confirm by default (due to cc)' '\n-\tsave_confirm &&\n+\ttest_when_finished git config sendemail.confirm never &&\n \tgit config --unset sendemail.confirm &&\n \ttest_confirm\n '\n \n test_expect_success $PREREQ 'confirm by default (due to --compose)' '\n-\tsave_confirm &&\n+\ttest_when_finished git config sendemail.confirm never &&\n \tgit config --unset sendemail.confirm &&\n \ttest_confirm --suppress-cc=all --compose\n '\n \n test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '\n-\tsave_confirm &&\n+\ttest_when_finished git config sendemail.confirm never &&\n \tgit config --unset sendemail.confirm &&\n \trm -fr outdir &&\n \tgit format-patch -2 -o outdir &&\n@@ -848,7 +843,7 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' '\n '\n \n test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '\n-\tsave_confirm &&\n+\ttest_when_finished git config sendemail.confirm never &&\n \tgit config sendemail.confirm auto &&\n \tGIT_SEND_EMAIL_NOTTY=1 &&\n \texport GIT_SEND_EMAIL_NOTTY &&\n@@ -860,7 +855,7 @@ test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '\n '\n \n test_expect_success $PREREQ 'confirm does not loop forever' '\n-\tsave_confirm &&\n+\ttest_when_finished git config sendemail.confirm never &&\n \tgit config sendemail.confirm auto &&\n \tGIT_SEND_EMAIL_NOTTY=1 &&\n \texport GIT_SEND_EMAIL_NOTTY &&\n-- \n2.3.4.635.gd6ffcfe\n"},{"id":"258496","messageId":"CALKQrge+KKHg3GtGz=rg3YQEnzN_tthqW4hrTDjFt96vnuGk9Q@mail.gmail.com","threadId":"38863","inReplyTo":"20150325052856.GC31924@peff.net","subject":"Re: [PATCH 3/8] t3305: fix ignored exit code inside loop","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2015-03-25T08:40:47Z","receivedAt":"2015-03-25T08:40:47Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Mar 25, 2015 at 6:28 AM, Jeff King wrote:\n> When we test deleting notes, we run \"git notes remove\" in a\n> loop. However, the exit value of the loop will only reflect\n> the final note we process. We should break out of the loop\n> with a failing exit code as soon as we see a problem.\n>\n> Note that we can call \"exit 1\" here without explicitly\n> creating a subshell, because the while loop on the\n> right-hand side of a pipe executes in its own implicit\n> subshell.\n>\n> Note also that the \"break\" above does not suffer the same\n> problem; it is meant to exit the loop early at a certain\n> number of iterations. We can bump it into the conditional of\n> the loop to make this more obvious.\n>\n> Signed-off-by: Jeff King \n\nAcked-by: Johan Herland \n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"258506","messageId":"xmqqfv8t2dii.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"20150325053017.GF31924@peff.net","subject":"Re: [PATCH 6/8] t: simplify loop exit-code status variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-25T17:27:49Z","receivedAt":"2015-03-25T17:27:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Since shell loops may drop the exit code of failed commands\n> inside the loop, some tests try to keep track of the status\n> by setting a variable. This can end up cumbersome and hard\n> to read; it is much simpler to just exit directly from the\n> loop using \"return 1\" (since each case is either in a helper\n> function or inside a test snippet).\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t3060-ls-files-with-tree.sh | 13 ++++---------\n> t/t3901-i18n-patch.sh | 8 ++------\n> 2 files changed, 6 insertions(+), 15 deletions(-)\n>\n> diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\n> index 61c1f53..36b10f7 100755\n> --- a/t/t3060-ls-files-with-tree.sh\n> +++ b/t/t3060-ls-files-with-tree.sh\n> @@ -25,15 +25,10 @@ test_expect_success setup '\n> \t\tdo\n> \t\t\tnum=00$n$m &&\n> \t\t\t>sub/file-$num &&\n> -\t\t\techo file-$num >>expected || {\n> -\t\t\t\tbad=t\n> -\t\t\t\tbreak\n> -\t\t\t}\n> -\t\tdone && test -z \"$bad\" || {\n> -\t\t\tbad=t\n> -\t\t\tbreak\n> -\t\t}\n> -\tdone && test -z \"$bad\" &&\n> +\t\t\techo file-$num >>expected ||\n> +\t\t\treturn 1\n> +\t\tdone\n> +\tdone &&\n> \tgit add . &&\n> \tgit commit -m \"add a bunch of files\" &&\n\nThe empty initialization for $bad can also go for this one, right?\n"},{"id":"258507","messageId":"xmqqbnjh2dfe.fsf@gitster.dls.corp.google.com","threadId":"38863","inReplyTo":"20150325052456.GA19394@peff.net","subject":"Re: [PATCH 0/8] more &&-chaining test fixups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-25T17:29:41Z","receivedAt":"2015-03-25T17:29:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> There were a few legitimate problems, though. I've ordered the patches\n> below by descending severity. These apply on top of jk/test-chain-lint.\n>\n> [1/8]: perf-lib: fix ignored exit code inside loop\n> [2/8]: t0020: fix ignored exit code inside loops\n> [3/8]: t3305: fix ignored exit code inside loop\n> [4/8]: t7701: fix ignored exit code inside loop\n>\n> These four are actual bugs.\n>\n> [5/8]: t: fix some trivial cases of ignored exit codes in loops\n>\n> These ones are in setup code, and so would almost certainly never\n> fail.\n>\n> [6/8]: t: simplify loop exit-code status variables\n> [7/8]: t0020: use test_* helpers instead of hand-rolled messages\n> [8/8]: t9001: drop save_confirm helper\n>\n> These last three are pure cleanup, no behavior changes. The last two\n> are not even strictly related to the same topic, but I noticed them\n> while in the area.\n\nThanks. All looked sensible.\n"},{"id":"258508","messageId":"20150325174319.GA15096@peff.net","threadId":"38863","inReplyTo":"xmqqfv8t2dii.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 6/8] t: simplify loop exit-code status variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-25T17:43:19Z","receivedAt":"2015-03-25T17:43:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 25, 2015 at 10:27:49AM -0700, Junio C Hamano wrote:\n\n> > diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh\n> > index 61c1f53..36b10f7 100755\n> > --- a/t/t3060-ls-files-with-tree.sh\n> > +++ b/t/t3060-ls-files-with-tree.sh\n> > @@ -25,15 +25,10 @@ test_expect_success setup '\n> > \t\tdo\n> > \t\t\tnum=00$n$m &&\n> > \t\t\t>sub/file-$num &&\n> > -\t\t\techo file-$num >>expected || {\n> > -\t\t\t\tbad=t\n> > -\t\t\t\tbreak\n> > -\t\t\t}\n> > -\t\tdone && test -z \"$bad\" || {\n> > -\t\t\tbad=t\n> > -\t\t\tbreak\n> > -\t\t}\n> > -\tdone && test -z \"$bad\" &&\n> > +\t\t\techo file-$num >>expected ||\n> > +\t\t\treturn 1\n> > +\t\tdone\n> > +\tdone &&\n> > \tgit add . &&\n> > \tgit commit -m \"add a bunch of files\" &&\n> \n> The empty initialization for $bad can also go for this one, right?\n\nYeah, it can.\n\n-Peff\n"}]}