This is a cleanup of the &&-chain lint I posted earlier: http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859 I don't know who came up with the idea for it originally, but the concept certainly was floating in the back of my mind from Jonathan's earlier version that is referenced in that thread. The general idea is to detect &&-chain breakage that can lead to our tests yielding false success. The first patch implements and discusses the lint-check itself, which is quite simple. The bulk of the work was fixing all of the issues in the existing tests. :) That didn't all need to happen immediately. I mainly wanted to start on it to answer two questions: 1. Are &&-chain breakages actually preventing us from seeing any test failures? Or is it mostly just pedantry, and we miss out only on knowing whether "cat >expect <<-\EOF" failed (which presumably it never does). 2. How bad are the false positives? Both how common, and how bad to work around. But after a few hours, I reached a zen state and just kept going. So at the end of this series, the whole test suite is --chain-lint clean (modulo any tests that are skipped on my platform). We could even switch the checks on by default at the end of the series, but I didn't do that here. I think it would be sane to run them all the time, though; in the normal success case, they don't add any forks (the shell just runs "(exit) && ...", and realizes that the whole thing is one big &&-chain). I couldn't measure any time difference running the suite with and without it. Anyway, to answer the questions: Yes, there were definitely tests whose values were being thrown away, and we would not have noticed if they failed. The good news is that all of them did pass once we started checking their results. Hooray. There were a number of false positives, though as a percentage of the test suite, probably not many (it's just that we have quite a lot of tests). Most of them were in rather old tests, and IMHO the fixes I did actually improved the readability of the result. So overall I think this is a very positive change; I doubt it will get in people's way very often, and I look forward to having one less thing to worry about handling manually in review. The biggest downside is that I may have automated Eric Sunshine out of a job. :) The patches are: [01/25]: t/test-lib: introduce --chain-lint option [02/25]: t: fix severe &&-chain breakage [03/25]: t: fix moderate &&-chain breakage [04/25]: t: fix trivial &&-chain breakage [05/25]: t: assume test_cmp produces verbose output [06/25]: t: use verbose instead of hand-rolled errors [07/25]: t: use test_must_fail instead of hand-rolled blocks [08/25]: t: fix &&-chaining issues around setup which might fail [09/25]: t: use test_might_fail for diff and grep [10/25]: t: use test_expect_code instead of hand-rolled comparison [11/25]: t: wrap complicated expect_code users in a block [12/25]: t: avoid using ":" for comments [13/25]: t3600: fix &&-chain breakage for setup commands [14/25]: t7201: fix &&-chain breakage [15/25]: t9502: fix &&-chain breakage [16/25]: t6030: use modern test_* helpers [17/25]: t0020: use modern test_* helpers [18/25]: t1301: use modern test_* helpers [19/25]: t6034: use modern test_* helpers [20/25]: t4117: use modern test_* helpers [21/25]: t9001: use test_when_finished [22/25]: t0050: appease --chain-lint [23/25]: t7004: fix embedded single-quotes [24/25]: t0005: fix broken &&-chains [25/25]: t4104: drop hand-rolled error reporting It's a lot of patches, and a few of them are long. I tried to group them by functionality rather than file (though as you can see, some of the tests were unique enough snowflakes that it made sense to discuss their issues separately). I'm hoping it should be an easy review, if not a short one. I'll spare you the full per-file diffstat, but the total is a very satisfying: 84 files changed, 397 insertions(+), 647 deletions(-) That's a lot of changes in a lot of hunks, but most of them are in files that haven't been touched in a while. The series merges cleanly with "pu", so I don't think I've stepped on anyone's topics in flight. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html