On Sun, Jul 1, 2018 at 5:24 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> This series fixes several buggy tests which went undetected due to
> broken &&-chains in subshells, modernizes some tests to take advantage
> of test functions (test_might_fail(), test_write_lines(), etc.), and
> fixes a lot of broken &&-chains in subshells. It applies atop
> 'master'. Happily, there are no broken &&-chains in subshells in any
> in-flight topic.
>
> It is split out from an earlier series[1] which additionally extended
> --chain-lint to work within subshells. I decided to make this an
> independent series because these (hopefully) non-controversial changes
> all stand on their own merit, and because I don't want to flood the list
> repeatedly with this lengthy series as I re-roll the "extend
> --chain-lint to work in subshells" functionality from [1].
>
> To ease review burden, I largely avoided noisy modernizations and
> cleanups, and limited changes to merely adding "&&" even when some other
> transformation would have made the fix nicer overall. (For example, I
> resisted the urge to replace a series of 'echo' statements with a simple
> here-doc.)
>
> Changes since [1]:
>
> * Found and fixed more &&-chain breakage, and converted a couple more
> 'unset' instances (which were hidden behind a MINGW prerequisite) to
> sane_unset().
>
> * Rewrote commit messages to sell changes on their own merit rather than
> leaning on --chain-lint as a crutch. (junio, jrnieder)
>
> * Changed a modernization/cleanup to use "! non-git-command" rather than
> test_must_fail(), moving it to its own patch in the process. (j6t)
>
> * Changed "printf '%s\n'" idiom to test_write_lines(). (junio)
>
> * Incorporated Stefan's fix[2] for a broken 't/lib-submodule-update'
> test since my interpretation of the problem was incorrect.
>
> * Converted a subshell 'echo' sequence to write_script().
>
> * Dropped patches which existed primarily to pacify --chain-lint; they
> are no longer needed since I re-wrote the "linter" to detect &&-chain
> breakage itself (by pure textual inspection) rather than by
> incorporating subshell bodies into the main &&-chain:
>
> t0001: use "{...}" block around "||" expression rather than subshell
> t3303: use standard here-doc tag "EOF" to avoid fooling --chain-lint
> t9104: use "{...}" block around "||" expression rather than subshell
> t9401: drop unnecessary nested subshell
>
> * Dropped a patch which pretty much duplicated Junio's 037714252f
> (tests: clean after SANITY tests, 2018-06-15), which graduated to
> 'master':
>
> t7508: use test_when_finished() instead of managing exit code manually
>
> An interdiff against [1] is below, although I stripped out all the noisy
> "printf '%s\n'" to test_write_lines() differences, of which there were a
> lot, since they drowned out the other more significant changes.
>
> Thanks to Elijah, Hannes, Jonathan Nieder, Jonathan Tan, Junio, Luke,
> Peff, and Stefan for comments on [1].
Thanks for this series,
I think it is good to include it as is and build on top if needed. I had some
comments on the earlier part of the series, but that is really just the cherry
on top of the cake.
Thanks,
Stefan
