From 444421db69539b74077306b6d0cb23e82afeb891 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 27 Aug 2024 10:27:40 +0200 Subject: [PATCH 1/6] test: [refactor] Fix E714 pycodestyle --- test/functional/test_framework/blocktools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 5c2fa28a31a..705b8e8fe5e 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2015-2022 The Bitcoin Core developers +# Copyright (c) 2015-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Utilities for manipulating blocks and transactions.""" @@ -74,7 +74,7 @@ def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl block.nVersion = version or tmpl.get('version') or VERSIONBITS_LAST_OLD_BLOCK_VERSION block.nTime = ntime or tmpl.get('curtime') or int(time.time() + 600) block.hashPrevBlock = hashprev or int(tmpl['previousblockhash'], 0x10) - if tmpl and not tmpl.get('bits') is None: + if tmpl and tmpl.get('bits') is not None: block.nBits = struct.unpack('>I', bytes.fromhex(tmpl['bits']))[0] else: block.nBits = 0x207fffff # difficulty retargeting is disabled in REGTEST chainparams From faaf3e53f09c73278e36674db0af14a262f0bd94 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 27 Aug 2024 14:14:45 +0200 Subject: [PATCH 2/6] test: [refactor] Fix F841 flake8 --- contrib/devtools/clang-format-diff.py | 2 +- .../interface_usdt_coinselection.py | 10 ++++----- test/functional/mempool_package_rbf.py | 22 +++++++++---------- test/functional/mempool_sigoplimit.py | 2 +- test/functional/p2p_1p1c_network.py | 2 +- test/functional/p2p_tx_download.py | 4 ++-- test/functional/rpc_createmultisig.py | 4 ++-- test/functional/wallet_upgradewallet.py | 10 ++++----- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/contrib/devtools/clang-format-diff.py b/contrib/devtools/clang-format-diff.py index e2b661d65d2..30e804dbe27 100755 --- a/contrib/devtools/clang-format-diff.py +++ b/contrib/devtools/clang-format-diff.py @@ -164,7 +164,7 @@ def main(): 'Failed to run "%s" - %s"' % (" ".join(command), e.strerror) ) - stdout, stderr = p.communicate() + stdout, _stderr = p.communicate() if p.returncode != 0: sys.exit(p.returncode) diff --git a/test/functional/interface_usdt_coinselection.py b/test/functional/interface_usdt_coinselection.py index dc40986a75d..f684848aedf 100755 --- a/test/functional/interface_usdt_coinselection.py +++ b/test/functional/interface_usdt_coinselection.py @@ -181,7 +181,7 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): # 5. aps_create_tx_internal (type 4) wallet.sendtoaddress(wallet.getnewaddress(), 10) events = self.get_tracepoints([1, 2, 3, 1, 4]) - success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events) assert_equal(success, True) assert_greater_than(change_pos, -1) @@ -190,7 +190,7 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): # 1. normal_create_tx_internal (type 2) assert_raises_rpc_error(-6, "Insufficient funds", wallet.sendtoaddress, wallet.getnewaddress(), 102 * 50) events = self.get_tracepoints([2]) - success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events) assert_equal(success, False) self.log.info("Explicitly enabling APS results in 2 tracepoints") @@ -200,7 +200,7 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): wallet.setwalletflag("avoid_reuse") wallet.sendtoaddress(address=wallet.getnewaddress(), amount=10, avoid_reuse=True) events = self.get_tracepoints([1, 2]) - success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events) assert_equal(success, True) assert_equal(use_aps, None) @@ -213,7 +213,7 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): # 5. aps_create_tx_internal (type 4) wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True, avoid_reuse=False) events = self.get_tracepoints([1, 2, 3, 1, 4]) - success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events) assert_equal(success, True) assert_equal(change_pos, -1) @@ -223,7 +223,7 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): # 2. normal_create_tx_internal (type 2) wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True) events = self.get_tracepoints([1, 2]) - success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + success, use_aps, _algo, _waste, change_pos = self.determine_selection_from_usdt(events) assert_equal(success, True) assert_equal(change_pos, -1) diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py index 9b4269f0a0e..f4d57262f20 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -189,7 +189,7 @@ class PackageRBFTest(BitcoinTestFramework): package_hex4, package_txns4 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE) node.submitpackage(package_hex4) self.assert_mempool_contents(expected=package_txns4) - package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE) + package_hex5, _package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE) pkg_results5 = node.submitpackage(package_hex5) assert 'package RBF failed: package feerate is less than or equal to parent feerate' in pkg_results5["package_msg"] self.assert_mempool_contents(expected=package_txns4) @@ -336,16 +336,16 @@ class PackageRBFTest(BitcoinTestFramework): self.assert_mempool_contents(expected=expected_txns) # Now make conflicting packages for each coin - package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex1) assert_equal(f"package RBF failed: {parent_result['tx'].rehash()} has 2 descendants, max 1 allowed", package_result["package_msg"]) - package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex2) assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has both ancestor and descendant, exceeding cluster limit of 2", package_result["package_msg"]) - package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex3) assert_equal(f"package RBF failed: {grandchild_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"]) @@ -389,15 +389,15 @@ class PackageRBFTest(BitcoinTestFramework): self.assert_mempool_contents(expected=expected_txns) # Now make conflicting packages for each coin - package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex1) assert_equal(f"package RBF failed: {parent1_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"]) - package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex2) assert_equal(f"package RBF failed: {parent2_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"]) - package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex3) assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"]) @@ -443,15 +443,15 @@ class PackageRBFTest(BitcoinTestFramework): self.assert_mempool_contents(expected=expected_txns) # Now make conflicting packages for each coin - package_hex1, package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex1) assert_equal(f"package RBF failed: {parent_result['tx'].rehash()} has 2 descendants, max 1 allowed", package_result["package_msg"]) - package_hex2, package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex2) assert_equal(f"package RBF failed: {child1_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"]) - package_hex3, package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) + package_hex3, _package_txns3 = self.create_simple_package(coin3, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex3) assert_equal(f"package RBF failed: {child2_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"]) @@ -519,7 +519,7 @@ class PackageRBFTest(BitcoinTestFramework): # Package 2 feerate is below the feerate of directly conflicted parent, so it fails even though # total fees are higher than the original package - package_hex2, package_txns2 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE - Decimal("0.00000001"), child_fee=DEFAULT_CHILD_FEE) + package_hex2, _package_txns2 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE - Decimal("0.00000001"), child_fee=DEFAULT_CHILD_FEE) pkg_results2 = node.submitpackage(package_hex2) assert_equal(pkg_results2["package_msg"], 'package RBF failed: insufficient feerate: does not improve feerate diagram') self.assert_mempool_contents(expected=package_txns1) diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 4656176a759..47df0c614ae 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -154,7 +154,7 @@ class BytesPerSigOpTest(BitcoinTestFramework): return (tx_utxo, tx) tx_parent_utxo, tx_parent = create_bare_multisig_tx() - tx_child_utxo, tx_child = create_bare_multisig_tx(tx_parent_utxo) + _tx_child_utxo, tx_child = create_bare_multisig_tx(tx_parent_utxo) # Separately, the parent tx is ok parent_individual_testres = self.nodes[0].testmempoolaccept([tx_parent.serialize().hex()])[0] diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py index c3cdb3e0b31..f9e782f5248 100755 --- a/test/functional/p2p_1p1c_network.py +++ b/test/functional/p2p_1p1c_network.py @@ -107,7 +107,7 @@ class PackageRelayTest(BitcoinTestFramework): # 3: 2-parent-1-child package. Both parents are above mempool min feerate. No package submission happens. # We require packages to be child-with-unconfirmed-parents and only allow 1-parent-1-child packages. - package_hex_3, parent_31, parent_32, child_3 = self.create_package_2p1c(self.wallet) + package_hex_3, parent_31, _parent_32, child_3 = self.create_package_2p1c(self.wallet) # 4: parent + child package where the child spends 2 different outputs from the parent. package_hex_4, parent_4, child_4 = self.create_package_2outs(self.wallet) diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index 11b4d9cc3b8..efad4e7c0fb 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -156,9 +156,9 @@ class TxDownloadTest(BitcoinTestFramework): # One of the peers is asked for the tx peer2.wait_until(lambda: sum(p.tx_getdata_count for p in [peer1, peer2]) == 1) with p2p_lock: - peer_expiry, peer_fallback = (peer1, peer2) if peer1.tx_getdata_count == 1 else (peer2, peer1) + _peer_expiry, peer_fallback = (peer1, peer2) if peer1.tx_getdata_count == 1 else (peer2, peer1) assert_equal(peer_fallback.tx_getdata_count, 0) - self.nodes[0].setmocktime(int(time.time()) + GETDATA_TX_INTERVAL + 1) # Wait for request to peer_expiry to expire + self.nodes[0].setmocktime(int(time.time()) + GETDATA_TX_INTERVAL + 1) # Wait for request to _peer_expiry to expire peer_fallback.wait_until(lambda: peer_fallback.tx_getdata_count >= 1, timeout=1) self.restart_node(0) # reset mocktime diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 9f4e17a3288..d95820bbf87 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -47,7 +47,7 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): return node.get_wallet_rpc(wallet_name) def run_test(self): - node0, node1, node2 = self.nodes + node0, node1, _node2 = self.nodes self.wallet = MiniWallet(test_node=node0) if self.is_wallet_compiled(): @@ -122,7 +122,7 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Unsupported multisig script size for legacy wallet. Upgrade to descriptors to overcome this limitation for p2sh-segwit or bech32 scripts", wallet_multi.addmultisigaddress, 16, pubkeys, '', 'bech32') def do_multisig(self, nkeys, nsigs, output_type, wallet_multi): - node0, node1, node2 = self.nodes + node0, _node1, node2 = self.nodes pub_keys = self.pub[0: nkeys] priv_keys = self.priv[0: nkeys] diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 7d1d244dffd..ef3f925ee82 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -231,7 +231,7 @@ class UpgradeWalletTest(BitcoinTestFramework): assert b'\x07hdchain' in new_kvs hd_chain = new_kvs[b'\x07hdchain'] assert_equal(28, len(hd_chain)) - hd_chain_version, external_counter, seed_id = struct.unpack(' Date: Tue, 27 Aug 2024 12:42:34 +0200 Subject: [PATCH 3/6] lint: Remove python lint rules that are SyntaxError Any kind of syntax error is already reported, so there is no need to enumerate all possible types of syntax errors of ancient versions of Python 2 or 3. --- test/lint/lint-python.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/test/lint/lint-python.py b/test/lint/lint-python.py index eabd13322e1..3d0d0203bd1 100755 --- a/test/lint/lint-python.py +++ b/test/lint/lint-python.py @@ -61,8 +61,6 @@ ENABLED = ( 'E722,' # do not use bare 'except' 'E742,' # do not define classes named "l", "O", or "I" 'E743,' # do not define functions named "l", "O", or "I" - 'E901,' # SyntaxError: invalid syntax - 'E902,' # TokenError: EOF in multi-line string 'F401,' # module imported but unused 'F402,' # import module from line N shadowed by loop variable 'F403,' # 'from foo_module import *' used; unable to detect undefined names @@ -73,33 +71,19 @@ ENABLED = ( 'F601,' # dictionary key name repeated with different values 'F602,' # dictionary key variable name repeated with different values 'F621,' # too many expressions in an assignment with star-unpacking - 'F622,' # two or more starred expressions in an assignment (a, *b, *c = d) 'F631,' # assertion test is a tuple, which are always True 'F632,' # use ==/!= to compare str, bytes, and int literals - 'F701,' # a break statement outside of a while or for loop - 'F702,' # a continue statement outside of a while or for loop - 'F703,' # a continue statement in a finally block in a loop - 'F704,' # a yield or yield from statement outside of a function - 'F705,' # a return statement with arguments inside a generator - 'F706,' # a return statement outside of a function/method - 'F707,' # an except: block as not the last exception handler 'F811,' # redefinition of unused name from line N 'F812,' # list comprehension redefines 'foo' from line N 'F821,' # undefined name 'Foo' 'F822,' # undefined name name in __all__ 'F823,' # local variable name … referenced before assignment - 'F831,' # duplicate argument name in function definition 'F841,' # local variable 'foo' is assigned to but never used 'W191,' # indentation contains tabs 'W291,' # trailing whitespace 'W292,' # no newline at end of file 'W293,' # blank line contains whitespace - 'W601,' # .has_key() is deprecated, use "in" - 'W602,' # deprecated form of raising exception - 'W603,' # "<>" is deprecated, use "!=" - 'W604,' # backticks are deprecated, use "repr()" 'W605,' # invalid escape sequence "x" - 'W606,' # 'async' and 'await' are reserved keywords starting with Python 3.7 ) From faebeb828f5f0ec68d90e7f76add66bc562f6fa3 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 27 Aug 2024 13:17:04 +0200 Subject: [PATCH 4/6] lint: Remove python whitespace and shadowing lint rules The rules have many issues: * Most are redundant, because Python already has a built-in IndentationError, a subclass of SyntaxError, to enforce whitespace. * They are not enforced consistently anyway, see for examples [1][2] below. * They are stylistic rules where the author intentionally formatted the code to be easier to read. Starting to enforce them now would make the code harder to read and create frustration in the future. Fix all issues by removing them. [1]: test/functional/feature_cltv.py:63:35: E272 [*] Multiple spaces before keyword | 61 | # | Script to prepend to scriptSig | nSequence | nLockTime | 62 | # +-------------------------------------------------+------------+--------------+ 63 | [[OP_CHECKLOCKTIMEVERIFY], None, None], | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E272 [2]: contrib/asmap/asmap.py:395:13: E306 [*] Expected 1 blank line before a nested definition, found 0 | 393 | prefix.pop() 394 | hole = not fill and (lhole or rhole) 395 | def candidate(ctx: Optional[int], res0: Optional[list[ASNEntry]], | ^^^ E306 --- test/lint/lint-python.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/lint/lint-python.py b/test/lint/lint-python.py index 3d0d0203bd1..439d1f1a09c 100755 --- a/test/lint/lint-python.py +++ b/test/lint/lint-python.py @@ -30,28 +30,8 @@ MYPY_FILES_ARGS = ['git', 'ls-files', 'test/functional/*.py', 'contrib/devtools/ ENABLED = ( 'E101,' # indentation contains mixed spaces and tabs - 'E112,' # expected an indented block - 'E113,' # unexpected indentation - 'E115,' # expected an indented block (comment) - 'E116,' # unexpected indentation (comment) - 'E125,' # continuation line with same indent as next logical line - 'E129,' # visually indented line with same indent as next logical line - 'E131,' # continuation line unaligned for hanging indent - 'E133,' # closing bracket is missing indentation - 'E223,' # tab before operator - 'E224,' # tab after operator - 'E242,' # tab after ',' - 'E266,' # too many leading '#' for block comment - 'E271,' # multiple spaces after keyword - 'E272,' # multiple spaces before keyword - 'E273,' # tab after keyword - 'E274,' # tab before keyword - 'E275,' # missing whitespace after keyword - 'E304,' # blank lines found after function decorator - 'E306,' # expected 1 blank line before a nested definition 'E401,' # multiple imports on one line 'E402,' # module level import not at top of file - 'E502,' # the backslash is redundant between brackets 'E701,' # multiple statements on one line (colon) 'E702,' # multiple statements on one line (semicolon) 'E703,' # statement ends with a semicolon @@ -74,7 +54,6 @@ ENABLED = ( 'F631,' # assertion test is a tuple, which are always True 'F632,' # use ==/!= to compare str, bytes, and int literals 'F811,' # redefinition of unused name from line N - 'F812,' # list comprehension redefines 'foo' from line N 'F821,' # undefined name 'Foo' 'F822,' # undefined name name in __all__ 'F823,' # local variable name … referenced before assignment From faf17df7fb88590d936d10c471a9ea6a2ce4454d Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 27 Aug 2024 13:32:24 +0200 Subject: [PATCH 5/6] lint: Document missing py_lint dependency Also, change the linter name, needed for the next commit. --- test/lint/README.md | 1 + test/lint/test_runner/src/main.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/lint/README.md b/test/lint/README.md index 04a836c4d20..48344288eca 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -52,6 +52,7 @@ or `--help`: | [`lint-python-dead-code.py`](/test/lint/lint-python-dead-code.py) | [vulture](https://github.com/jendrikseipp/vulture) | [`lint-shell.py`](/test/lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck) | [`lint-spelling.py`](/test/lint/lint-spelling.py) | [codespell](https://github.com/codespell-project/codespell) +| `py_lint` | [ruff](https://github.com/astral-sh/ruff) | markdown link check | [mlc](https://github.com/becheran/mlc) In use versions and install instructions are available in the [CI setup](../../ci/lint/04_install.sh). diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index 1a8c11dd428..8a1d3291855 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -36,9 +36,9 @@ fn get_linter_list() -> Vec<&'static Linter> { lint_fn: lint_markdown }, &Linter { - description: "Check the default arguments in python", - name: "py_mut_arg_default", - lint_fn: lint_py_mut_arg_default, + description: "Lint Python code", + name: "py_lint", + lint_fn: lint_py_lint, }, &Linter { description: "Check that std::filesystem is not used directly", @@ -185,7 +185,7 @@ fn lint_subtree() -> LintResult { } } -fn lint_py_mut_arg_default() -> LintResult { +fn lint_py_lint() -> LintResult { let bin_name = "ruff"; let checks = ["B006", "B008"] .iter() From fafdb7df34507eee735893aa871da6ae529e6372 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 27 Aug 2024 13:27:59 +0200 Subject: [PATCH 6/6] lint: Speed up flake8 checks Previously they may have taken more than 10 seconds. Now they should finish in less than one second. This also allows to drop one dependency to be installed. --- ci/lint/04_install.sh | 1 - test/lint/README.md | 1 - test/lint/lint-python.py | 59 ++----------------------------- test/lint/test_runner/src/main.rs | 48 ++++++++++++++++++++++--- 4 files changed, 45 insertions(+), 64 deletions(-) diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index acec2f32e9f..cfdb4ea9368 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -49,7 +49,6 @@ fi ${CI_RETRY_EXE} pip3 install \ codespell==2.2.6 \ - flake8==6.1.0 \ lief==0.13.2 \ mypy==1.4.1 \ pyzmq==25.1.0 \ diff --git a/test/lint/README.md b/test/lint/README.md index 48344288eca..8c1f0fedf07 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -45,7 +45,6 @@ or `--help`: | Lint test | Dependency | |-----------|:----------:| -| [`lint-python.py`](/test/lint/lint-python.py) | [flake8](https://github.com/PyCQA/flake8) | [`lint-python.py`](/test/lint/lint-python.py) | [lief](https://github.com/lief-project/LIEF) | [`lint-python.py`](/test/lint/lint-python.py) | [mypy](https://github.com/python/mypy) | [`lint-python.py`](/test/lint/lint-python.py) | [pyzmq](https://github.com/zeromq/pyzmq) diff --git a/test/lint/lint-python.py b/test/lint/lint-python.py index 439d1f1a09c..e2dbe25b884 100755 --- a/test/lint/lint-python.py +++ b/test/lint/lint-python.py @@ -5,13 +5,12 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """ -Check for specified flake8 and mypy warnings in python files. +Check for specified mypy warnings in python files. """ import os from pathlib import Path import subprocess -import sys from importlib.metadata import metadata, PackageNotFoundError @@ -19,52 +18,12 @@ from importlib.metadata import metadata, PackageNotFoundError cache_dir = Path(__file__).parent.parent / ".mypy_cache" os.environ["MYPY_CACHE_DIR"] = str(cache_dir) -DEPS = ['flake8', 'lief', 'mypy', 'pyzmq'] - -# All .py files, except those in src/ (to exclude subtrees there) -FLAKE_FILES_ARGS = ['git', 'ls-files', '*.py', ':!:src/*.py'] +DEPS = ['lief', 'mypy', 'pyzmq'] # Only .py files in test/functional and contrib/devtools have type annotations # enforced. MYPY_FILES_ARGS = ['git', 'ls-files', 'test/functional/*.py', 'contrib/devtools/*.py'] -ENABLED = ( - 'E101,' # indentation contains mixed spaces and tabs - 'E401,' # multiple imports on one line - 'E402,' # module level import not at top of file - 'E701,' # multiple statements on one line (colon) - 'E702,' # multiple statements on one line (semicolon) - 'E703,' # statement ends with a semicolon - 'E711,' # comparison to None should be 'if cond is None:' - 'E714,' # test for object identity should be "is not" - 'E721,' # do not compare types, use "isinstance()" - 'E722,' # do not use bare 'except' - 'E742,' # do not define classes named "l", "O", or "I" - 'E743,' # do not define functions named "l", "O", or "I" - 'F401,' # module imported but unused - 'F402,' # import module from line N shadowed by loop variable - 'F403,' # 'from foo_module import *' used; unable to detect undefined names - 'F404,' # future import(s) name after other statements - 'F405,' # foo_function may be undefined, or defined from star imports: bar_module - 'F406,' # "from module import *" only allowed at module level - 'F407,' # an undefined __future__ feature name was imported - 'F601,' # dictionary key name repeated with different values - 'F602,' # dictionary key variable name repeated with different values - 'F621,' # too many expressions in an assignment with star-unpacking - 'F631,' # assertion test is a tuple, which are always True - 'F632,' # use ==/!= to compare str, bytes, and int literals - 'F811,' # redefinition of unused name from line N - 'F821,' # undefined name 'Foo' - 'F822,' # undefined name name in __all__ - 'F823,' # local variable name … referenced before assignment - 'F841,' # local variable 'foo' is assigned to but never used - 'W191,' # indentation contains tabs - 'W291,' # trailing whitespace - 'W292,' # no newline at end of file - 'W293,' # blank line contains whitespace - 'W605,' # invalid escape sequence "x" -) - def check_dependencies(): for dep in DEPS: @@ -78,20 +37,6 @@ def check_dependencies(): def main(): check_dependencies() - if len(sys.argv) > 1: - flake8_files = sys.argv[1:] - else: - flake8_files = subprocess.check_output(FLAKE_FILES_ARGS).decode("utf-8").splitlines() - - flake8_args = ['flake8', '--ignore=B,C,E,F,I,N,W', f'--select={ENABLED}'] + flake8_files - flake8_env = os.environ.copy() - flake8_env["PYTHONWARNINGS"] = "ignore" - - try: - subprocess.check_call(flake8_args, env=flake8_env) - except subprocess.CalledProcessError: - exit(1) - mypy_files = subprocess.check_output(MYPY_FILES_ARGS).decode("utf-8").splitlines() mypy_args = ['mypy', '--show-error-codes'] + mypy_files diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index 8a1d3291855..1cd3d9287b6 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -187,10 +187,48 @@ fn lint_subtree() -> LintResult { fn lint_py_lint() -> LintResult { let bin_name = "ruff"; - let checks = ["B006", "B008"] - .iter() - .map(|c| format!("--select={}", c)) - .collect::>(); + let checks = format!( + "--select={}", + [ + "B006", // mutable-argument-default + "B008", // function-call-in-default-argument + "E101", // indentation contains mixed spaces and tabs + "E401", // multiple imports on one line + "E402", // module level import not at top of file + "E701", // multiple statements on one line (colon) + "E702", // multiple statements on one line (semicolon) + "E703", // statement ends with a semicolon + "E711", // comparison to None should be 'if cond is None:' + "E714", // test for object identity should be "is not" + "E721", // do not compare types, use "isinstance()" + "E722", // do not use bare 'except' + "E742", // do not define classes named "l", "O", or "I" + "E743", // do not define functions named "l", "O", or "I" + "F401", // module imported but unused + "F402", // import module from line N shadowed by loop variable + "F403", // 'from foo_module import *' used; unable to detect undefined names + "F404", // future import(s) name after other statements + "F405", // foo_function may be undefined, or defined from star imports: bar_module + "F406", // "from module import *" only allowed at module level + "F407", // an undefined __future__ feature name was imported + "F601", // dictionary key name repeated with different values + "F602", // dictionary key variable name repeated with different values + "F621", // too many expressions in an assignment with star-unpacking + "F631", // assertion test is a tuple, which are always True + "F632", // use ==/!= to compare str, bytes, and int literals + "F811", // redefinition of unused name from line N + "F821", // undefined name 'Foo' + "F822", // undefined name name in __all__ + "F823", // local variable name … referenced before assignment + "F841", // local variable 'foo' is assigned to but never used + "W191", // indentation contains tabs + "W291", // trailing whitespace + "W292", // no newline at end of file + "W293", // blank line contains whitespace + "W605", // invalid escape sequence "x" + ] + .join(",") + ); let files = check_output( git() .args(["ls-files", "--", "*.py"]) @@ -198,7 +236,7 @@ fn lint_py_lint() -> LintResult { )?; let mut cmd = Command::new(bin_name); - cmd.arg("check").args(checks).args(files.lines()); + cmd.args(["check", &checks]).args(files.lines()); match cmd.status() { Ok(status) if status.success() => Ok(()),