From 12047d77d0ced9e78da82eeba0f144f786651bc1 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 7 Nov 2018 12:20:01 -0500 Subject: [PATCH] Merge #14365: tests: Add Python dead code linter (vulture) to Travis c82190cdb6 tests: Add Python dead code linter (vulture) (practicalswift) 590a57fdec tests: Remove unused testing code (practicalswift) Pull request description: Add Python dead code linter (`vulture`) to Travis. Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`): * Less is more :-) * Unused code is by definition "untested" * Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-) * Unused code is hard to spot for humans and is thus often missed during manual review * [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) Based on #14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6 --- .travis/lint_04_install.sh | 1 + ci/Dockerfile.builder | 1 + test/functional/feature_addressindex.py | 1 - .../feature_block_reward_reallocation.py | 4 ++-- .../feature_dip3_deterministicmns.py | 9 ------- test/functional/feature_multikeysporks.py | 1 - test/functional/feature_spentindex.py | 1 - test/functional/feature_timestampindex.py | 1 - test/functional/feature_txindex.py | 1 - test/functional/mempool_expiry.py | 5 ++-- test/functional/p2p_leak_tx.py | 5 ++-- test/functional/test_framework/key.py | 10 -------- test/functional/test_framework/messages.py | 20 +--------------- test/functional/test_framework/mininode.py | 24 +++++++++---------- test/functional/test_framework/socks5.py | 7 +++--- .../test_framework/test_framework.py | 1 - test/functional/wallet_listtransactions.py | 13 +--------- test/lint/lint-python-dead-code.sh | 19 +++++++++++++++ 18 files changed, 46 insertions(+), 78 deletions(-) create mode 100755 test/lint/lint-python-dead-code.sh diff --git a/.travis/lint_04_install.sh b/.travis/lint_04_install.sh index 8c3a9124b9..723e7c56f1 100755 --- a/.travis/lint_04_install.sh +++ b/.travis/lint_04_install.sh @@ -8,3 +8,4 @@ export LC_ALL=C travis_retry pip install codespell==1.13.0 travis_retry pip install flake8==3.5.0 +travis_retry pip install vulture==0.29 diff --git a/ci/Dockerfile.builder b/ci/Dockerfile.builder index f77d0d70bc..bdcb083153 100644 --- a/ci/Dockerfile.builder +++ b/ci/Dockerfile.builder @@ -17,6 +17,7 @@ RUN pip3 install pyzmq # really needed? RUN pip3 install jinja2 RUN pip3 install flake8==3.5.0 RUN pip3 install codespell==1.13.0 +RUN pip3 install vulture==0.29 # dash_hash RUN git clone https://github.com/dashpay/dash_hash diff --git a/test/functional/feature_addressindex.py b/test/functional/feature_addressindex.py index 8b2a37dfc0..c145d0cd0a 100755 --- a/test/functional/feature_addressindex.py +++ b/test/functional/feature_addressindex.py @@ -33,7 +33,6 @@ class AddressIndexTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 2) connect_nodes(self.nodes[0], 3) - self.is_network_split = False self.sync_all() def run_test(self): diff --git a/test/functional/feature_block_reward_reallocation.py b/test/functional/feature_block_reward_reallocation.py index ca4f853780..ad69a46e61 100755 --- a/test/functional/feature_block_reward_reallocation.py +++ b/test/functional/feature_block_reward_reallocation.py @@ -151,7 +151,7 @@ class BlockRewardReallocationTest(DashTestFramework): assert_equal(bt['masternode'][0]['amount'], 6874285801) # 0.4999999998 self.log.info("Reallocation should kick-in with the superblock mined at height = 2010") - for period in range(19): # there will be 19 adjustments, 3 superblocks long each + for _ in range(19): # there will be 19 adjustments, 3 superblocks long each for i in range(3): self.bump_mocktime(10) self.nodes[0].generate(10) @@ -163,7 +163,7 @@ class BlockRewardReallocationTest(DashTestFramework): assert_equal(bt['masternode'][0]['amount'], 6132959502) # 0.6 self.log.info("Reward split should stay ~60/40 after reallocation is done") - for period in range(10): # check 10 next superblocks + for _ in range(10): # check 10 next superblocks self.bump_mocktime(10) self.nodes[0].generate(10) bt = self.nodes[0].getblocktemplate() diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index ae739314f3..0773e7e9b7 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -40,14 +40,6 @@ class DIP3Test(BitcoinTestFramework): if i < len(self.nodes) and self.nodes[i] is not None and self.nodes[i].process is not None: connect_nodes(self.nodes[i], 0) - def stop_controller_node(self): - self.log.info("stopping controller node") - self.stop_node(0) - - def restart_controller_node(self): - self.stop_controller_node() - self.start_controller_node() - def run_test(self): self.log.info("funding controller node") while self.nodes[0].getbalance() < (self.num_initial_mn + 3) * 1000: @@ -216,7 +208,6 @@ class DIP3Test(BitcoinTestFramework): mn = Masternode() mn.idx = idx mn.alias = alias - mn.is_protx = True mn.p2p_port = p2p_port(mn.idx) blsKey = node.bls('generate') diff --git a/test/functional/feature_multikeysporks.py b/test/functional/feature_multikeysporks.py index 35b00f77ae..5419a6b92f 100755 --- a/test/functional/feature_multikeysporks.py +++ b/test/functional/feature_multikeysporks.py @@ -23,7 +23,6 @@ class MultiKeySporkTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 5 self.setup_clean_chain = True - self.is_network_split = False def setup_network(self): # secret(base58): 931wyuRNVYvhg18Uu9bky5Qg1z4QbxaJ7fefNBzjBPiLRqcd33F diff --git a/test/functional/feature_spentindex.py b/test/functional/feature_spentindex.py index cd60676fb9..913f872312 100755 --- a/test/functional/feature_spentindex.py +++ b/test/functional/feature_spentindex.py @@ -35,7 +35,6 @@ class SpentIndexTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 2) connect_nodes(self.nodes[0], 3) - self.is_network_split = False self.sync_all() def run_test(self): diff --git a/test/functional/feature_timestampindex.py b/test/functional/feature_timestampindex.py index 5c43a5640e..217c1e5630 100755 --- a/test/functional/feature_timestampindex.py +++ b/test/functional/feature_timestampindex.py @@ -30,7 +30,6 @@ class TimestampIndexTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 2) connect_nodes(self.nodes[0], 3) - self.is_network_split = False self.sync_all() def run_test(self): diff --git a/test/functional/feature_txindex.py b/test/functional/feature_txindex.py index 2602f7f947..25eca0251f 100755 --- a/test/functional/feature_txindex.py +++ b/test/functional/feature_txindex.py @@ -33,7 +33,6 @@ class TxIndexTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 2) connect_nodes(self.nodes[0], 3) - self.is_network_split = False self.sync_all() def run_test(self): diff --git a/test/functional/mempool_expiry.py b/test/functional/mempool_expiry.py index 8b9b7b155a..9869453fd3 100755 --- a/test/functional/mempool_expiry.py +++ b/test/functional/mempool_expiry.py @@ -27,8 +27,9 @@ class MempoolExpiryTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() + # TODO: enable when skip_if_no_wallet is backported + # def skip_test_if_missing_module(self): + # self.skip_if_no_wallet() def test_transaction_expiry(self, timeout): """Tests that a transaction expires after the expiry timeout and its diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py index dc4d475b2d..9cccb0fbfc 100755 --- a/test/functional/p2p_leak_tx.py +++ b/test/functional/p2p_leak_tx.py @@ -21,8 +21,9 @@ class P2PLeakTxTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() + # TODO: enable when skip_if_no_wallet is backported + # def skip_test_if_missing_module(self): + # self.skip_if_no_wallet() def run_test(self): gen_node = self.nodes[0] # The block and tx generating node diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index 1b3e510dc4..ff66f74bb6 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -86,9 +86,6 @@ ssl.EC_KEY_new_by_curve_name.errcheck = _check_result class CECKey(): """Wrapper around OpenSSL's EC_KEY""" - POINT_CONVERSION_COMPRESSED = 2 - POINT_CONVERSION_UNCOMPRESSED = 4 - def __init__(self): self.k = ssl.EC_KEY_new_by_curve_name(NID_secp256k1) @@ -181,13 +178,6 @@ class CECKey(): """Verify a DER signature""" return ssl.ECDSA_verify(0, hash, len(hash), sig, len(sig), self.k) == 1 - def set_compressed(self, compressed): - if compressed: - form = self.POINT_CONVERSION_COMPRESSED - else: - form = self.POINT_CONVERSION_UNCOMPRESSED - ssl.EC_KEY_set_conv_form(self.k, form) - class CPubKey(bytes): """An encapsulated public key diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 1b49309446..148c52d449 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -36,7 +36,6 @@ MY_VERSION = 70219 # LLMQ_DATA_MESSAGES_VERSION MY_SUBVERSION = b"/python-mininode-tester:0.0.3%s/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) -MAX_INV_SZ = 50000 MAX_LOCATOR_SZ = 101 MAX_BLOCK_SIZE = 1000000 @@ -166,22 +165,6 @@ def ser_uint256_vector(l): return r -def deser_string_vector(f): - nit = deser_compact_size(f) - r = [] - for i in range(nit): - t = deser_string(f) - r.append(t) - return r - - -def ser_string_vector(l): - r = ser_compact_size(len(l)) - for sv in l: - r += ser_string(sv) - return r - - def deser_dyn_bitset(f, bytes_based): if bytes_based: nb = deser_compact_size(f) @@ -831,13 +814,12 @@ class BlockTransactions: class CPartialMerkleTree: - __slots__ = ("fBad", "nTransactions", "vBits", "vHash") + __slots__ = ("nTransactions", "vBits", "vHash") def __init__(self): self.nTransactions = 0 self.vBits = [] self.vHash = [] - self.fBad = False def deserialize(self, f): self.nTransactions = struct.unpack("= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED) @@ -474,18 +473,19 @@ class P2PInterface(P2PConnection): wait_until(test_function, timeout=timeout, lock=mininode_lock) - def wait_for_inv(self, expected_inv, timeout=60): - """Waits for an INV message and checks that the first inv object in the message was as expected.""" - if len(expected_inv) > 1: - raise NotImplementedError("wait_for_inv() will only verify the first inv object") + # TODO: enable when p2p_filter.py is backported + # def wait_for_inv(self, expected_inv, timeout=60): + # """Waits for an INV message and checks that the first inv object in the message was as expected.""" + # if len(expected_inv) > 1: + # raise NotImplementedError("wait_for_inv() will only verify the first inv object") - def test_function(): - assert self.is_connected - return self.last_message.get("inv") and \ - self.last_message["inv"].inv[0].type == expected_inv[0].type and \ - self.last_message["inv"].inv[0].hash == expected_inv[0].hash + # def test_function(): + # assert self.is_connected + # return self.last_message.get("inv") and \ + # self.last_message["inv"].inv[0].type == expected_inv[0].type and \ + # self.last_message["inv"].inv[0].hash == expected_inv[0].hash - wait_until(test_function, timeout=timeout, lock=mininode_lock) + # wait_until(test_function, timeout=timeout, lock=mininode_lock) def wait_for_verack(self, timeout=60): def test_function(): diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py index 4cd8d2734d..39c75ca495 100644 --- a/test/functional/test_framework/socks5.py +++ b/test/functional/test_framework/socks5.py @@ -54,10 +54,9 @@ class Socks5Command(): return 'Socks5Command(%s,%s,%s,%s,%s,%s)' % (self.cmd, self.atyp, self.addr, self.port, self.username, self.password) class Socks5Connection(): - def __init__(self, serv, conn, peer): + def __init__(self, serv, conn): self.serv = serv self.conn = conn - self.peer = peer def handle(self): """Handle socks5 request according to RFC192.""" @@ -137,9 +136,9 @@ class Socks5Server(): def run(self): while self.running: - (sockconn, peer) = self.s.accept() + (sockconn, _) = self.s.accept() if self.running: - conn = Socks5Connection(self, sockconn, peer) + conn = Socks5Connection(self, sockconn) thread = threading.Thread(None, conn.handle) thread.daemon = True thread.start() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 7e0f73e9fb..1473ea6546 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -575,7 +575,6 @@ class DashTestFramework(BitcoinTestFramework): self.num_nodes = num_nodes self.mninfo = [] self.setup_clean_chain = True - self.is_network_split = False # additional args if extra_args is None: extra_args = [[]] * num_nodes diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index b05f773e02..82fdd09418 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -4,20 +4,9 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the listtransactions API.""" from decimal import Decimal -from io import BytesIO -from test_framework.messages import CTransaction from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_array_result, - hex_str_to_bytes, -) - -def tx_from_hex(hexstring): - tx = CTransaction() - f = BytesIO(hex_str_to_bytes(hexstring)) - tx.deserialize(f) - return tx +from test_framework.util import assert_array_result class ListTransactionsTest(BitcoinTestFramework): def set_test_params(self): diff --git a/test/lint/lint-python-dead-code.sh b/test/lint/lint-python-dead-code.sh new file mode 100755 index 0000000000..7e415ff785 --- /dev/null +++ b/test/lint/lint-python-dead-code.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# +# Find dead Python code. + +export LC_ALL=C + +if ! command -v vulture > /dev/null; then + echo "Skipping Python dead code linting since vulture is not installed. Install by running \"pip3 install vulture\"" + exit 0 +fi + +vulture \ + --min-confidence 60 \ + --ignore-names "argtypes,connection_lost,connection_made,converter,data_received,daemon,errcheck,get_ecdh_key,get_privkey,is_compressed,is_fullyvalid,msg_generic,on_*,optionxform,restype,set_privkey,*serialize_v2" \ + $(git ls-files -- "*.py" ":(exclude)contrib/" ":(exclude)src/crc32c/" ":(exclude)test/functional/test_framework/address.py")