lint: Convert Python dead code linter test to Python #24778

pull fjahr wants to merge 1 commits into bitcoin:master from fjahr:202204-dead-py changing 3 files +42 −22

fjahr commented at 11:00 PM on April 5, 2022: contributor

The new python version should produce the exact same output as the bash version but be easier to maintain.
lint: Convert Python dead code linter to Python 076cd6835f
DrahtBot added the label Docs on Apr 5, 2022
DrahtBot added the label Tests on Apr 5, 2022
in test/lint/lint-python-dead-code.py:33 in 076cd6835f
```
  28 | +    # --min-confidence 100 will only report code that is guaranteed to be unused within the analyzed files.
  29 | +    # Any value below 100 introduces the risk of false positives, which would create an unacceptable maintenance burden.
  30 | +    vulture_args = ['vulture', '--min-confidence=100'] + files
  31 | +
  32 | +    try:
  33 | +        check_output(vulture_args, stderr=STDOUT)
```
MarcoFalke commented at 6:09 AM on April 6, 2022:

Will this print the error? I'd guess no
fjahr commented at 6:49 AM on April 6, 2022:

Hm, for me it does. It doesn't for you?

(faked to get an alert)

$ test/lint/lint-python-dead-code.py /Users/FJ/projects/clones/bitcoin/test/lint/lint-python-dead-code.py:26: unsatisfiable 'if' condition (100% confidence) Python dead code detection found some issues
MarcoFalke commented at 7:01 AM on April 6, 2022:

Thanks for testing and confirming. I just assumed that redirecting stderr to stdout and then capturing stdout, which is done by check_output, which discards the return value here, would also drop the stderr.

This is also what the documentation says:

https://docs.python.org/3/library/subprocess.html#subprocess.check_output

To also capture standard error in the result, use stderr=subprocess.STDOUT

Not sure what to do now. Are you trying to trick me into installing vulture? :thinking:

MarcoFalke commented at 7:02 AM on April 6, 2022:

Oh, sorry. I missed the e.output.decode two lines below. :man_facepalming:
MarcoFalke approved
fjahr cross-referenced this on Apr 6, 2022 from issue doc: Add bash dependency of lint tests by fjahr
MarcoFalke commented at 7:05 AM on April 6, 2022: member

review ACK 076cd6835fd97a62bfd6912b80addfcb5342ea8e

The three (3) CI failures are unrelated and can be ignored (:smiling_face_with_tear:)
MarcoFalke merged this on Apr 6, 2022
MarcoFalke closed this on Apr 6, 2022
laanwj cross-referenced this on Apr 6, 2022 from issue Port lint scripts to Python by laanwj
sidhujag referenced this in commit 459274b683 on Apr 6, 2022
bitcoin locked this on Apr 6, 2023

Contributors

fjahr

MarcoFalke

Labels

Docs Tests

Linked (view graph)

#24750 doc: Add bash dependency of lint tests #24783 Port lint scripts to Python