diff options
author | Eric Sunshine <sunshine@sunshineco.com> | 2022-09-01 02:29:49 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-09-01 19:07:41 +0200 |
commit | 832c68b3c210267c93e1dcb2f2763372339ca36c (patch) | |
tree | 3141f27c3958b96cbb05ddd3f7feafcc4328f79e | |
parent | chainlint.pl: don't flag broken &&-chain if `$?` handled explicitly (diff) | |
download | git-832c68b3c210267c93e1dcb2f2763372339ca36c.tar.xz git-832c68b3c210267c93e1dcb2f2763372339ca36c.zip |
chainlint.pl: don't flag broken &&-chain if failure indicated explicitly
There are quite a few tests which print an error messages and then
explicitly signal failure with `false`, `return 1`, or `exit 1` as the
final command in an `if` branch. In these cases, the tests don't bother
maintaining the &&-chain between `echo` and the explicit "test failed"
indicator. Since such constructs are manually signaling failure, their
&&-chain breakage is legitimate and safe -- both for the command
immediately preceding `false`, `return`, or `exit`, as well as for all
preceding commands in the `if` branch. Therefore, stop flagging &&-chain
breakage in these sorts of cases.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rwxr-xr-x | t/chainlint.pl | 8 | ||||
-rw-r--r-- | t/chainlint/chain-break-false.expect | 9 | ||||
-rw-r--r-- | t/chainlint/chain-break-false.test | 10 | ||||
-rw-r--r-- | t/chainlint/chain-break-return-exit.expect | 15 | ||||
-rw-r--r-- | t/chainlint/chain-break-return-exit.test | 18 | ||||
-rw-r--r-- | t/chainlint/if-in-loop.expect | 2 | ||||
-rw-r--r-- | t/chainlint/if-in-loop.test | 2 |
7 files changed, 62 insertions, 2 deletions
diff --git a/t/chainlint.pl b/t/chainlint.pl index 14e1db3519..a76a09ecf5 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -503,6 +503,14 @@ sub accumulate { goto DONE if $token =~ /\$\?/; } + # if this command is "false", "return 1", or "exit 1" (which signal + # failure explicitly), then okay for all preceding commands to be + # missing "&&" + if ($$cmd[0] =~ /^(?:false|return|exit)$/) { + @$tokens = grep(!/^\?!AMP\?!$/, @$tokens); + goto DONE; + } + # flag missing "&&" at end of previous command my $n = find_non_nl($tokens); splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0; diff --git a/t/chainlint/chain-break-false.expect b/t/chainlint/chain-break-false.expect new file mode 100644 index 0000000000..989766fb85 --- /dev/null +++ b/t/chainlint/chain-break-false.expect @@ -0,0 +1,9 @@ +if condition not satisified +then + echo it did not work... + echo failed! + false +else + echo it went okay ?!AMP?! + congratulate user +fi diff --git a/t/chainlint/chain-break-false.test b/t/chainlint/chain-break-false.test new file mode 100644 index 0000000000..a5aaff8c8a --- /dev/null +++ b/t/chainlint/chain-break-false.test @@ -0,0 +1,10 @@ +# LINT: broken &&-chain okay if explicit "false" signals failure +if condition not satisified +then + echo it did not work... + echo failed! + false +else + echo it went okay + congratulate user +fi diff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect index dba292ee89..1732d221c3 100644 --- a/t/chainlint/chain-break-return-exit.expect +++ b/t/chainlint/chain-break-return-exit.expect @@ -1,3 +1,18 @@ +case "$(git ls-files)" in +one ) echo pass one ;; +* ) echo bad one ; return 1 ;; +esac && +( + case "$(git ls-files)" in + two ) echo pass two ;; + * ) echo bad two ; exit 1 ;; +esac +) && +case "$(git ls-files)" in +dir/two"$LF"one ) echo pass both ;; +* ) echo bad ; return 1 ;; +esac && + for i in 1 2 3 4 ; do git checkout main -b $i || return $? test_commit $i $i $i tag$i || return $? diff --git a/t/chainlint/chain-break-return-exit.test b/t/chainlint/chain-break-return-exit.test index e2b059933a..46542edf88 100644 --- a/t/chainlint/chain-break-return-exit.test +++ b/t/chainlint/chain-break-return-exit.test @@ -1,3 +1,21 @@ +case "$(git ls-files)" in +one) echo pass one ;; +# LINT: broken &&-chain okay if explicit "return 1" signals failuire +*) echo bad one; return 1 ;; +esac && +( + case "$(git ls-files)" in + two) echo pass two ;; +# LINT: broken &&-chain okay if explicit "exit 1" signals failuire + *) echo bad two; exit 1 ;; + esac +) && +case "$(git ls-files)" in +dir/two"$LF"one) echo pass both ;; +# LINT: broken &&-chain okay if explicit "return 1" signals failuire +*) echo bad; return 1 ;; +esac && + for i in 1 2 3 4 ; do # LINT: broken &&-chain okay if explicit "return $?" signals failure git checkout main -b $i || return $? diff --git a/t/chainlint/if-in-loop.expect b/t/chainlint/if-in-loop.expect index 03b82a3e58..d6514ae749 100644 --- a/t/chainlint/if-in-loop.expect +++ b/t/chainlint/if-in-loop.expect @@ -3,7 +3,7 @@ do if false then - echo "err" ?!AMP?! + echo "err" exit 1 fi ?!AMP?! foo diff --git a/t/chainlint/if-in-loop.test b/t/chainlint/if-in-loop.test index f0cf19cfad..90c23976fe 100644 --- a/t/chainlint/if-in-loop.test +++ b/t/chainlint/if-in-loop.test @@ -3,7 +3,7 @@ do if false then -# LINT: missing "&&" on "echo" +# LINT: missing "&&" on "echo" okay since "exit 1" signals error explicitly echo "err" exit 1 # LINT: missing "&&" on "fi" |