test: Add missing set -ex to ci/lint/06_script.sh
#28103
Conversation
(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.
This is needed for the container-entrypoint.sh Also, remove unused `source` from ci/lint_run_all.sh, since it is the last step.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
DrahtBot
changed the title
set -ex to ci/lint/06_script.sh set -ex to ci/lint/06_script.sh
maflcko
changed the title
set -ex to ci/lint/06_script.shset -ex to ci/lint/06_script.sh
|
|
||
| export LC_ALL=C | ||
|
|
||
| set -ex |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Nah, this is a step in the right direction I think.
jamesob
left a comment
jamesob
left a comment
There was a problem hiding this comment.
ACK ffff4b5 (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.
ffff4b5 lint: Add missing `set -ex` to ci/lint/06_script.sh (MarcoFalke) fadc523 doc: Add doc comment to ci/test_imagefile (MarcoFalke) Pull request description: Requested in bitcoin#28083 (review). Also, one doc commit. ACKs for top commit: fanquake: ACK ffff4b5 jamesob: ACK ffff4b5 ([`jamesob/ackr/28103.1.MarcoFalke.test_add_missing_set_ex`](https://github.com/jamesob/bitcoin/tree/ackr/28103.1.MarcoFalke.test_add_missing_set_ex)) Tree-SHA512: 99e67aeaae460319c2c428eab5297dbe1f1dc7f162f6592380bc5d2005308300c391cc187959cb2ace486dfe3411a8b0477f703ff11b5fe33944942c210a2d32
…early exit fa01f88 ci: Add missing COPY for ./test/lint/test_runner (MarcoFalke) faff3e3 lint: Report all lint errors instead of early exit (MarcoFalke) Pull request description: `all-lint.py` currently collects all failures. However, the `06_script.sh` does not, since July this year (bitcoin/bitcoin#28103 (comment)). Fix this by printing all failures before exiting. Can be tested by modifying (for example) two subtrees in the same commit and then running the linters. ACKs for top commit: kevkevinpal: ACK [fa01f88](bitcoin/bitcoin@fa01f88) TheCharlatan: lgtm ACK fa01f88 Tree-SHA512: c0f3110f2907d87e29c755e3b77a67dfae1f8a25833fe6ef8f2f2c58cfecf1aa46f1a20881576b62252b04930140a9e416c78b4edba0780d3c4fa7aaebabba81
4 participants
Requested in #28083 (review).
Also, one doc commit.