| Message ID | 20211209051115.52629-7-sunshine@sunshineco.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | efe47c83b266b3ca12d646f5a8f1c6a71f0d6e44 |
| Headers | show |
| Series | tests: fix broken &&-chains & abort loops on error | expand |
On Thu, Dec 09, 2021 at 12:11:02AM -0500, Eric Sunshine wrote: > This test populates its `expect` file solely by appending content but > fails to ensure that the file starts out empty. The test succeeds only > because no earlier test populated a file of the exact same name, however > this is an accident waiting to happen. Make the test more robust by > ensuring that it contains exactly the intended content. Agreed. > While at it, simplify the implementation via a straightforward `sed` > application and by avoiding dropping out of the single-quote context > within the test body (thus eliminating a hard-to-digest combination of > apostrophes and backslashes). I find them equally ugly. :) The most confusing thing is that we are not doing any shell quoting at all, just adding single-quote wrappers. But that is OK because %(raw:size) should never need quoting. Your solution seems fine to me. -Peff
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 9f2c706c12..aae57908f8 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -955,10 +955,7 @@ test_expect_success '%(raw) with --shell and --sort=raw must fail' ' ' test_expect_success '%(raw:size) with --shell' ' - git for-each-ref --format="%(raw:size)" | while read line - do - echo "'\''$line'\''" >>expect - done && + git for-each-ref --format="%(raw:size)" | sed "s/^/$SQ/;s/$/$SQ/" >expect && git for-each-ref --format="%(raw:size)" --shell >actual && test_cmp expect actual '
This test populates its `expect` file solely by appending content but fails to ensure that the file starts out empty. The test succeeds only because no earlier test populated a file of the exact same name, however this is an accident waiting to happen. Make the test more robust by ensuring that it contains exactly the intended content. While at it, simplify the implementation via a straightforward `sed` application and by avoiding dropping out of the single-quote context within the test body (thus eliminating a hard-to-digest combination of apostrophes and backslashes). Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- Notes: An alternative simple fix would be to capture the output of the `while` loop itself rather than the output of `echo`: git for-each-ref ... | while read line do echo ... done >expect && however, I prefer the conciseness of the `sed` approach, This patch makes no attempt to address `git` being upstream in a pipe. There are enough other such instances in this script that fixing them warrants a separate patch (if someone wants to tackle it). t/t6300-for-each-ref.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)