lint: convert lint-tests.sh to python #24815

pull KevinMusgrave wants to merge 1 commits into bitcoin:master from KevinMusgrave:lint-tests-python-port changing 2 files +87 −35

KevinMusgrave commented at 10:05 PM on April 9, 2022: contributor

Reference issue: https://github.com/bitcoin/bitcoin/issues/24783
KevinMusgrave renamed this:
~~Converted lint-tests.sh to python~~
lint: converted lint-tests.sh to python
on Apr 9, 2022
KevinMusgrave force-pushed on Apr 9, 2022
KevinMusgrave renamed this:
~~lint: converted lint-tests.sh to python~~
lint: convert lint-tests.sh to python
on Apr 9, 2022
DrahtBot added the label Tests on Apr 9, 2022
KevinMusgrave commented at 11:59 AM on April 11, 2022: contributor

Maybe I should replace grep, cut, sort, and uniq with python functions
KevinMusgrave force-pushed on Apr 11, 2022
KevinMusgrave force-pushed on Apr 11, 2022
KevinMusgrave commented at 10:19 PM on April 11, 2022: contributor

Ok I converted those functions to python, so the only non-python call is git grep.
KevinMusgrave force-pushed on Apr 11, 2022
KevinMusgrave force-pushed on Apr 11, 2022

in test/lint/lint-tests.py:3 in 285ee4b00c outdated

   0 | @@ -0,0 +1,76 @@
   1 | +#!/usr/bin/env python3
   2 | +#
   3 | +# Copyright (c) 2018 The Bitcoin Core developers

vincenzopalazzo commented at 11:33 PM on April 11, 2022:

# Copyright (c) 2018-2022 The Bitcoin Core developers

KevinMusgrave commented at 2:12 PM on April 13, 2022:

Fixed

vincenzopalazzo commented at 11:35 PM on April 11, 2022: none

utACK https://github.com/bitcoin/bitcoin/pull/24815/commits/285ee4b00cd847068eff7b6ff5500d7c180b3d3c
KevinMusgrave force-pushed on Apr 13, 2022

KevinMusgrave commented at 2:37 PM on April 13, 2022: contributor

Below are example outputs, before and after the change. The only difference is that the new version adds a newline after the "matching names" test, which I think improves readability.

Before (lint-tests.sh):

The test suite in file src/test/foo_tests.cpp should be named
"foo_tests". Please make sure the following test suites follow
that convention:

src/test/banman_tests.cpp:BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
src/test/bip32_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/blockchain_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/blockencodings_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, RegTestingSetup)
Test suite names must be unique. The following test suite names
appear to be used more than once:

addrman_tests
base58_tests

After (lint-tests.py):

The test suite in file src/test/foo_tests.cpp should be named
"foo_tests". Please make sure the following test suites follow
that convention:

src/test/banman_tests.cpp:BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
src/test/bip32_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/blockchain_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/blockencodings_tests.cpp:BOOST_FIXTURE_TEST_SUITE(base58_tests, RegTestingSetup)

Test suite names must be unique. The following test suite names
appear to be used more than once:

addrman_tests
base58_tests

in test/lint/lint-tests.py:26 in b4125057ba outdated
```
  21 | +        r"^BOOST_FIXTURE_TEST_SUITE\(",
  22 | +        "--",
  23 | +        "src/test/**.cpp",
  24 | +        "src/wallet/test/**.cpp",
  25 | +    ]
  26 | +    return subprocess.run(command, stdout=subprocess.PIPE, universal_newlines=True)
```
laanwj commented at 5:45 PM on April 13, 2022:

Would prefer to use check_output here. This would make the script automatically fail if the git grep returns an error status. It also autmatically does stdout=PIPE and returns stdout.

KevinMusgrave commented at 6:24 PM on April 13, 2022:

Done
KevinMusgrave force-pushed on Apr 13, 2022
KevinMusgrave force-pushed on Apr 13, 2022
Eunoia1729 commented at 10:22 PM on April 13, 2022: contributor

utACk Not sure but is it a good practise to leave links in the code ?
KevinMusgrave commented at 10:28 PM on April 13, 2022: contributor

When I grab a snippet from stackoverflow, I like to leave a link to credit the author but also in case I want to see the original answer again.

Converted lint-tests.sh to python

Use raw string

Use re.search instead of grep in check_matching_test_names

Replaced bash commands in check_unique_test_names with python commands

Use set and sort output

Use set comprehension

Use .splitlines()

Call grep_boost_fixture_test_suite once

splitlines() once

Fixed copyright date

Use check_output() instead of run()

add encoding='utf8'

Use clearer code for getting duplicates

ae0e06a439

in test/lint/lint-tests.py:51 in 21b967a67d outdated
```
  46 | +
  47 | +
  48 | +def check_unique_test_names(test_suite_list):
  49 | +    output = [re.search(r"\((.*?),", x) for x in test_suite_list]
  50 | +    output = [x.group(1) for x in output if x is not None]
  51 | +    # find duplicates, see: https://stackoverflow.com/a/9835819
```
MarcoFalke commented at 11:11 AM on April 15, 2022:

This is pretty hard to read and the answer actually recommends against using the snippet.

MarcoFalke commented at 11:11 AM on April 15, 2022:

This is pretty hard to read and the answer actually recommends against using the snippet.

KevinMusgrave commented at 1:41 PM on April 15, 2022:

I assume you mean this output = {x for x in output if x in seen or seen.add(x)}. I thought the comment makes it clear what it does, but I can convert it to a for loop anyway.

KevinMusgrave commented at 2:16 PM on April 15, 2022:

I changed it now

KevinMusgrave commented at 2:16 PM on April 15, 2022:

Looks like an accidental duplicate review
KevinMusgrave force-pushed on Apr 15, 2022
laanwj commented at 5:27 PM on April 25, 2022: member

Tested ACK ae0e06a439e09a7e24dd6198591a588c8df2d529
laanwj merged this on Apr 25, 2022
laanwj closed this on Apr 25, 2022
sidhujag referenced this in commit ea3961c0ac on Apr 26, 2022
DrahtBot locked this on Apr 25, 2023

Contributors

Labels

Linked (view graph)

#24783 Port lint scripts to Python