lint: fix lint-tests.py failing on correct names #26725

willcl-ark commented at 10:54 AM on December 19, 2022: contributor

lint-tests.py is currently failing to detect that filenames are correct and therefore returning incorrect result and error code from the script. This fixes the matching.

Ouput before:

❯ test/lint/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/addrman_tests.cpp:61:BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
src/test/argsman_tests.cpp:22:BOOST_FIXTURE_TEST_SUITE(argsman_tests, BasicTestingSetup)
src/test/banman_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(banman_tests, BasicTestingSetup)
src/test/base58_tests.cpp:21:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/bip32_tests.cpp:160:BOOST_FIXTURE_TEST_SUITE(bip32_tests, BasicTestingSetup)
...
etc.

Output after (all tests ok):

❯ test/lint/lint-tests.py; echo $status
0

Output after (with an intentially renamed test):

❯ git mv src/test/addrman_tests.cpp src/test/addrman_tests_incorrect.cpp

will@ubuntu in bitcoin on  lint-name-test [$»] via 🐍 v3.6.15
❯ test/lint/lint-tests.py; and $status
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/addrman_tests_incorrect.cpp

1

lint: fix lint-tests.py failing on correct names

lint-tests.py is currently failing to detect that filenames are correct
and therefore returning incorrect result and error code from the script.

7536ff2e06

DrahtBot commented at 10:54 AM on December 19, 2022: 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. A summary of reviews will appear here.

willcl-ark commented at 10:55 AM on December 19, 2022: contributor

With this, #26721 and #26257 the usefulness of linter would be greatly improved.

maflcko commented at 11:09 AM on December 19, 2022: member

CI is green on master, so it is unclear to me what this is fixing.

willcl-ark commented at 12:10 PM on December 19, 2022: contributor

Closing because:

My fix is incorrect for the intended test case.
It seems you are correct and that this is a local issue with my setup in some way, although I cannot figure out why https://github.com/bitcoin/bitcoin/blob/65f5cfda656c150dd0df40369845c53fa387b25f/test/lint/lint-tests.py#L30-L34 does not succeed in its regex matching locally for me, which I can see that it does on CI.

I am running with python 3.6.15 as per .python-version and have packages matching CI versions installed:

❯ pip freeze
codespell==2.2.1
flake8==4.0.1
importlib-metadata==4.2.0
mccabe==0.6.1
mypy==0.942
mypy-extensions==0.4.3
pycodestyle==2.8.0
pyflakes==2.4.0
pyzmq==22.3.0
toml==0.10.2
tomli==1.2.3
typed-ast==1.5.4
typing_extensions==4.1.1
vulture==2.3
zipp==3.6.0

vs

https://github.com/bitcoin/bitcoin/blob/65f5cfda656c150dd0df40369845c53fa387b25f/ci/lint/04_install.sh#L19-L23

When I run lint-tests.py on master I see the following output which I don't see in CI:

will@ubuntu in bitcoin on  master [$⇡] via 🐍 v3.6.15
❯ ./test/lint/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/addrman_tests.cpp:61:BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup)
src/test/argsman_tests.cpp:22:BOOST_FIXTURE_TEST_SUITE(argsman_tests, BasicTestingSetup)
src/test/banman_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(banman_tests, BasicTestingSetup)
src/test/base58_tests.cpp:21:BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)
src/test/bip32_tests.cpp:160:BOOST_FIXTURE_TEST_SUITE(bip32_tests, BasicTestingSetup)
src/test/blockchain_tests.cpp:50:BOOST_FIXTURE_TEST_SUITE(blockchain_tests, BasicTestingSetup)
src/test/blockencodings_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(blockencodings_tests, RegTestingSetup)
src/test/blockmanager_tests.cpp:17:BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
src/test/bloom_tests.cpp:24:BOOST_FIXTURE_TEST_SUITE(bloom_tests, BasicTestingSetup)
src/test/checkqueue_tests.cpp:35:BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, NoLockLoggingTestingSetup)
src/test/coins_tests.cpp:99:BOOST_FIXTURE_TEST_SUITE(coins_tests, BasicTestingSetup)
src/test/compress_tests.cpp:25:BOOST_FIXTURE_TEST_SUITE(compress_tests, BasicTestingSetup)
src/test/crypto_tests.cpp:27:BOOST_FIXTURE_TEST_SUITE(crypto_tests, BasicTestingSetup)
src/test/dbwrapper_tests.cpp:23:BOOST_FIXTURE_TEST_SUITE(dbwrapper_tests, BasicTestingSetup)
src/test/denialofservice_tests.cpp:36:BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup)
src/test/descriptor_tests.cpp:371:BOOST_FIXTURE_TEST_SUITE(descriptor_tests, BasicTestingSetup)
src/test/flatfile_tests.cpp:13:BOOST_FIXTURE_TEST_SUITE(flatfile_tests, BasicTestingSetup)
src/test/fs_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup)
src/test/getarg_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(getarg_tests, BasicTestingSetup)
src/test/headers_sync_chainwork_tests.cpp:58:BOOST_FIXTURE_TEST_SUITE(headers_sync_chainwork_tests, HeadersGeneratorSetup)
src/test/httpserver_tests.cpp:10:BOOST_FIXTURE_TEST_SUITE(httpserver_tests, BasicTestingSetup)
src/test/i2p_tests.cpp:19:BOOST_FIXTURE_TEST_SUITE(i2p_tests, BasicTestingSetup)
src/test/interfaces_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup)
src/test/key_io_tests.cpp:20:BOOST_FIXTURE_TEST_SUITE(key_io_tests, BasicTestingSetup)
src/test/key_tests.cpp:32:BOOST_FIXTURE_TEST_SUITE(key_tests, BasicTestingSetup)
src/test/logging_tests.cpp:20:BOOST_FIXTURE_TEST_SUITE(logging_tests, BasicTestingSetup)
src/test/mempool_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup)
src/test/merkle_tests.cpp:10:BOOST_FIXTURE_TEST_SUITE(merkle_tests, TestingSetup)
src/test/miner_tests.cpp:54:BOOST_FIXTURE_TEST_SUITE(miner_tests, MinerTestingSetup)
src/test/miniscript_tests.cpp:159:BOOST_FIXTURE_TEST_SUITE(miniscript_tests, BasicTestingSetup)
src/test/multisig_tests.cpp:19:BOOST_FIXTURE_TEST_SUITE(multisig_tests, BasicTestingSetup)
src/test/net_peer_eviction_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(net_peer_eviction_tests, BasicTestingSetup)
src/test/net_tests.cpp:36:BOOST_FIXTURE_TEST_SUITE(net_tests, RegTestingSetup)
src/test/netbase_tests.cpp:23:BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
src/test/orphanage_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(orphanage_tests, TestingSetup)
src/test/pmt_tests.cpp:28:BOOST_FIXTURE_TEST_SUITE(pmt_tests, BasicTestingSetup)
src/test/policyestimator_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, ChainTestingSetup)
src/test/pow_tests.cpp:12:BOOST_FIXTURE_TEST_SUITE(pow_tests, BasicTestingSetup)
src/test/prevector_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(prevector_tests, TestingSetup)
src/test/raii_event_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup)
src/test/random_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(random_tests, BasicTestingSetup)
src/test/rbf_tests.cpp:17:BOOST_FIXTURE_TEST_SUITE(rbf_tests, TestingSetup)
src/test/rest_tests.cpp:12:BOOST_FIXTURE_TEST_SUITE(rest_tests, BasicTestingSetup)
src/test/rpc_tests.cpp:83:BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup)
src/test/sanity_tests.cpp:11:BOOST_FIXTURE_TEST_SUITE(sanity_tests, BasicTestingSetup)
src/test/script_p2sh_tests.cpp:51:BOOST_FIXTURE_TEST_SUITE(script_p2sh_tests, BasicTestingSetup)
src/test/script_segwit_tests.cpp:10:BOOST_FIXTURE_TEST_SUITE(script_segwit_tests, BasicTestingSetup)
src/test/script_standard_tests.cpp:20:BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup)
src/test/script_tests.cpp:126:BOOST_FIXTURE_TEST_SUITE(script_tests, BasicTestingSetup)
src/test/scriptnum_tests.cpp:13:BOOST_FIXTURE_TEST_SUITE(scriptnum_tests, BasicTestingSetup)
src/test/serfloat_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(serfloat_tests, BasicTestingSetup)
src/test/serialize_tests.cpp:15:BOOST_FIXTURE_TEST_SUITE(serialize_tests, BasicTestingSetup)
src/test/settings_tests.cpp:50:BOOST_FIXTURE_TEST_SUITE(settings_tests, BasicTestingSetup)
src/test/sighash_tests.cpp:118:BOOST_FIXTURE_TEST_SUITE(sighash_tests, BasicTestingSetup)
src/test/sigopcount_tests.cpp:27:BOOST_FIXTURE_TEST_SUITE(sigopcount_tests, BasicTestingSetup)
src/test/skiplist_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(skiplist_tests, BasicTestingSetup)
src/test/sock_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(sock_tests, BasicTestingSetup)
src/test/streams_tests.cpp:13:BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup)
src/test/system_tests.cpp:24:BOOST_FIXTURE_TEST_SUITE(system_tests, BasicTestingSetup)
src/test/timedata_tests.cpp:19:BOOST_FIXTURE_TEST_SUITE(timedata_tests, BasicTestingSetup)
src/test/transaction_tests.cpp:188:BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup)
src/test/txreconciliation_tests.cpp:11:BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup)
src/test/txrequest_tests.cpp:17:BOOST_FIXTURE_TEST_SUITE(txrequest_tests, BasicTestingSetup)
src/test/util_tests.cpp:54:BOOST_FIXTURE_TEST_SUITE(util_tests, BasicTestingSetup)
src/test/validation_block_tests.cpp:34:BOOST_FIXTURE_TEST_SUITE(validation_block_tests, MinerTestingSetup)
src/test/validation_chainstate_tests.cpp:19:BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, ChainTestingSetup)
src/test/validation_chainstatemanager_tests.cpp:26:BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, ChainTestingSetup)
src/test/validation_flush_tests.cpp:11:BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, TestingSetup)
src/test/validation_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(validation_tests, TestingSetup)
src/test/validationinterface_tests.cpp:13:BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
src/test/versionbits_tests.cpp:185:BOOST_FIXTURE_TEST_SUITE(versionbits_tests, BasicTestingSetup)
src/wallet/test/availablecoins_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(availablecoins_tests, WalletTestingSetup)
src/wallet/test/coinselector_tests.cpp:23:BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup)
src/wallet/test/db_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup)
src/wallet/test/feebumper_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(feebumper_tests, WalletTestingSetup)
src/wallet/test/init_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup)
src/wallet/test/ismine_tests.cpp:18:BOOST_FIXTURE_TEST_SUITE(ismine_tests, BasicTestingSetup)
src/wallet/test/psbt_wallet_tests.cpp:15:BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup)
src/wallet/test/scriptpubkeyman_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(scriptpubkeyman_tests, BasicTestingSetup)
src/wallet/test/spend_tests.cpp:16:BOOST_FIXTURE_TEST_SUITE(spend_tests, WalletTestingSetup)
src/wallet/test/wallet_crypto_tests.cpp:14:BOOST_FIXTURE_TEST_SUITE(wallet_crypto_tests, BasicTestingSetup)
src/wallet/test/wallet_tests.cpp:44:BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup)
src/wallet/test/wallet_transaction_tests.cpp:12:BOOST_FIXTURE_TEST_SUITE(wallet_transaction_tests, WalletTestingSetup)
src/wallet/test/walletdb_tests.cpp:13:BOOST_FIXTURE_TEST_SUITE(walletdb_tests, BasicTestingSetup)

If I can't figure out why, I guess I'll re-open as an issue.

willcl-ark closed this on Dec 19, 2022

willcl-ark commented at 12:38 PM on December 19, 2022: contributor

Certainly seems to be something wrong with my local regex searching via python, although I'm not really sure how that can happen.

For example modifying the regex line to use a different regex (with look behind capture group) will have it pass locally for me, as it seems to be doing on CI:

diff --git a/test/lint/lint-tests.py b/test/lint/lint-tests.py
index 849ddcb96..cafba9f45 100755
--- a/test/lint/lint-tests.py
+++ b/test/lint/lint-tests.py
@@ -30,7 +30,7 @@ def check_matching_test_names(test_suite_list):
     not_matching = [
         x
         for x in test_suite_list
-        if re.search(r"/(.*?)\.cpp:BOOST_FIXTURE_TEST_SUITE\(\1, .*\)", x) is None
+        if re.search(r"/((?<=\/)[^\/]*)\.cpp:\d*:BOOST_FIXTURE_TEST_SUITE\(\1, .*\)", x) is None
     ]
     if len(not_matching) > 0:
         not_matching = "\n".join(not_matching)

maflcko commented at 12:40 PM on December 19, 2022: member

To debug this, we'd need steps to reproduce. Ideally, starting from a fresh install of your OS

willcl-ark commented at 2:43 PM on December 19, 2022: contributor

Ok I've found the issue in case anyone else ever ends up here. my git grep was returning linenumbers by default and throwing off the regex.

bitcoin locked this on Dec 19, 2023