test: Add missing `set -ex` to ci/lint/06_script.sh #28103

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2307-lint-ex- changing 5 files +20 −2

maflcko commented at 9:43 AM on July 19, 2023: member

Requested in #28083#pullrequestreview-1535304219.

Also, one doc commit.

doc: Add doc comment to ci/test_imagefile

(Similar to the doc comment in ci/lint_imagefile)

Also, rename docker-entrypoint.sh to container-entrypoint.sh

Also, add copyright header to touched files.

fadc5232f4

lint: Add missing `set -ex` to ci/lint/06_script.sh

This is needed for the container-entrypoint.sh

Also, remove unused `source` from ci/lint_run_all.sh, since it is the
last step.

ffff4b5dc5

DrahtBot commented at 9:43 AM on July 19, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK fanquake, jamesob

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot renamed this:
~~lint: Add missing `set -ex` to ci/lint/06_script.sh~~
lint: Add missing `set -ex` to ci/lint/06_script.sh
on Jul 19, 2023
maflcko renamed this:
~~lint: Add missing `set -ex` to ci/lint/06_script.sh~~
test: Add missing `set -ex` to ci/lint/06_script.sh
on Jul 19, 2023
DrahtBot added the label Tests on Jul 19, 2023
fanquake requested review from jamesob on Jul 19, 2023
fanquake approved
fanquake commented at 11:33 AM on July 19, 2023: member

ACK ffff4b5dc57c32bf759b705530c1368de4aa787e
in ci/lint/06_script.sh:9 in ffff4b5dc5
```
   5 | @@ -6,6 +6,8 @@
   6 |  
   7 |  export LC_ALL=C
   8 |  
   9 | +set -ex
```
jamesob commented at 1:56 PM on July 19, 2023:

Only downside with this is that CI won't report all lint failures, rather will bail on the first one. Not sure if this is going to be annoying for people. Maybe we could have a FAIL_FAST envvar that gates this, and right now is only enabled in the container entrypoint script?

maflcko commented at 2:13 PM on July 19, 2023:

A fail-fast setting would also need to be picked up by lint-all to abort early, because this one does collect the failures.

But I won't be working on that soon, so should I just close this pull?

jamesob commented at 2:20 PM on July 20, 2023:

Nah, this is a step in the right direction I think.
jamesob approved
jamesob commented at 3:54 PM on July 20, 2023: contributor

ACK ffff4b5dc57c32bf759b705530c1368de4aa787e (jamesob/ackr/28103.1.MarcoFalke.test_add_missing_set_ex)

Tested locally; lint errors surfaced through the lint container give non-zero error code as expected, which will allow local git hooks to e.g. stop pushes on error.
fanquake merged this on Jul 20, 2023
fanquake closed this on Jul 20, 2023
maflcko deleted the branch on Jul 20, 2023
sidhujag referenced this in commit 632295996c on Jul 21, 2023
fanquake referenced this in commit 172cd92620 on Nov 22, 2023
bitcoin locked this on Dec 5, 2024

Type	Reviewers
ACK	fanquake, jamesob

Contributors

Labels

Linked (view graph)

#28083 ci: Use DOCKER_BUILDKIT for lint image #28862 lint: Report all lint errors instead of early exit

test: Add missing `set -ex` to ci/lint/06_script.sh #28103

Reviews