[PATCH 0/8] more &&-chaining test fixups

Here's what I found looking for loops like:

  for i in a b c; do
     something_important $i || break
  done &&
  something_else

which presumably expect the chain to stop when something_important fails for any loop element. The solutions are one of (depending on the surrounding code):

  1. Switch the break to "return 1". The tests are all &&-chained, so
     the effect of a failed command is to exit the test immediately
     anyway. And we wrap our eval'd test snippets inside an extra layer
     of function call to explicitly allow early returns like this.

  2. Switch the break to "exit 1". Calling "return" from a subshell
     inside a function is a bit weird. It doesn't exit the function at
     all, but rather just the subshell (in both bash and dash). But if
     you are not in a function, calling "return" at all is an error in
     bash (subshell or no), and OK in dash (where it acts like "exit").
     POSIX explicitly marks the "outside of a function" behavior as
     unspecified, but I couldn't find anything about the subshell
     behavior.

     So I'm loathe to depend on it, even though it does seem to do what
     we want, as I do not want to even think what some more obscure
     shells might do with it. And especially because we know that "exit
     1" portably does what we want (the only downside is that you have
     to recognize which situation you are in and use "exit" versus
     "return").

  3. Unroll the loops. In some cases the result is actually shorter, and
     (IMHO) more readable.

These sites were all found with:

  git grep -E '(^|\|\|)[ 	]*break' t/t*.sh

(that's a space and a tab in the brackets). There are some matches there that I did not touch, because they were already fine. E.g., t5302 and t7700 use loops to assign a value to a variable and break out early, and then check the value of the variable.

That's just the tip of the iceberg, though. Searching for

  git grep 'for .* in '

yields hundreds of hits. Most of which are probably fine (quite a few are outside &&-chains entirely). I focused on the ones that called break, because that indicated to me that the author was trying to address the &&-chain. Certainly anybody else is welcome to take a stab at the rest, but I'm also happy to fix them up as we touch nearby code and notice them. Most of the loops are in setup code that we do not expect to fail anyway, so examining them is a lot of work for a little gain.

There were a few legitimate problems, though. I've ordered the patches below by descending severity. These apply on top of jk/test-chain-lint.

  [1/8]: perf-lib: fix ignored exit code inside loop
  [2/8]: t0020: fix ignored exit code inside loops
  [3/8]: t3305: fix ignored exit code inside loop
  [4/8]: t7701: fix ignored exit code inside loop

    These four are actual bugs.

  [5/8]: t: fix some trivial cases of ignored exit codes in loops

    These ones are in setup code, and so would almost certainly never
    fail.

  [6/8]: t: simplify loop exit-code status variables
  [7/8]: t0020: use test_* helpers instead of hand-rolled messages
  [8/8]: t9001: drop save_confirm helper

    These last three are pure cleanup, no behavior changes. The last two
    are not even strictly related to the same topic, but I noticed them
    while in the area.

-Peff