test: Reliably don't start itself (lint-all.py runs all tests twice) #25288

pull martinus wants to merge 2 commits into bitcoin:master from martinus:2022-06-fix-lint-all-starting-itself changing 4 files +8 −9

martinus commented at 5:18 PM on June 6, 2022: contributor

When running ./test/lint/lint-all.py, the script runs all tests but also calls itself because the comparison with __file__ doesn't work.

Comparing resolved paths gives reliable comparison, and lint-all.py doesn't call itself any more
martinus renamed this:
~~test: Reliably don't start itself in lint-all.py runs all tests twice~~
test: Reliably don't start itself (lint-all.py runs all tests twice)
on Jun 6, 2022
DrahtBot added the label Tests on Jun 6, 2022
theStack commented at 7:44 PM on June 6, 2022: member

Concept ACK. Good catch!
MarcoFalke commented at 6:08 AM on June 7, 2022: member

It doesn't run twice in CI, right? For example, looking at https://cirrus-ci.com/task/4523243527733248?logs=lint#L849 I can only find the error once.
martinus commented at 7:48 AM on June 7, 2022: contributor

Ah, in 06_script.sh lint-all.py is started with test/lint/lint-all.py, but I am starting it locally with ./test/lint/lint-all.py. I think the leading ./ is the difference
laanwj commented at 7:53 AM on June 7, 2022: member

May I propose a different solution: to rename the launcher so that it doesn't match the glob. This is 100% reliable and imo more elegant.
MarcoFalke commented at 8:09 AM on June 7, 2022: member

Yeah, could rename to all-lint.py and then also address #24982 (review) while touching this file?

test: rename lint-all.py to all-lint.py

That way it is impossible for the script to call itself.

64d72c4c87

test: clean up all-lint.py

Removed th check against __file__ which is not necessary any more after the rename to all-lint.py.

Changed glob to find only `lint-*.py` scripts.

f26a496dfd

martinus force-pushed on Jun 7, 2022
laanwj commented at 8:31 AM on June 7, 2022: member

Code review ACK f26a496dfd0a7ce3833a10075027d7d5b0345e32
MarcoFalke merged this on Jun 7, 2022
MarcoFalke closed this on Jun 7, 2022
martinus deleted the branch on Jun 7, 2022
jonatack commented at 2:50 PM on June 7, 2022: member

Post-merge ACK
sidhujag referenced this in commit 57f019aee2 on Jun 8, 2022
DrahtBot locked this on Jun 7, 2023

Contributors

Labels