Merge bitcoin#14365: tests: Add Python dead code linter (vulture) to …

…Travis c82190c tests: Add Python dead code linter (vulture) (practicalswift) 590a57f 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 bitcoin#14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
pravblockc · Aug 12, 2021 · 12047d7 · 12047d7
1 parent 1a1378b
commit 12047d7
Show file tree

Hide file tree

Showing 18 changed files with 46 additions and 78 deletions.
diff --git 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
@@ -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
@@ -33,7 +33,6 @@ def setup_network(self):
         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
@@ -151,7 +151,7 @@ def run_test(self):
         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 @@ def run_test(self):
         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
@@ -40,14 +40,6 @@ def start_controller_node(self):
             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 @@ def prepare_mn(self, node, idx, alias):
         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
@@ -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
@@ -35,7 +35,6 @@ def setup_network(self):
         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
@@ -30,7 +30,6 @@ def setup_network(self):
         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
@@ -33,7 +33,6 @@ def setup_network(self):
         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
@@ -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
@@ -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
@@ -86,9 +86,6 @@ def _check_result(val, func, args):
 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 @@ def verify(self, hash, sig):
         """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
@@ -36,7 +36,6 @@
 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 @@ def __repr__(self):
 
 
 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("<I", f.read(4))[0]

diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
@@ -400,8 +400,7 @@ def on_qgetdata(self, message): pass
     def on_qdata(self, message): pass
     def on_qwatch(self, message): pass
 
-    def on_verack(self, message):
-        self.verack_received = True
+    def on_verack(self, message): pass
 
     def on_version(self, message):
         assert message.nVersion >= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED)
@@ -474,18 +473,19 @@ def test_function():
 
         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
@@ -54,10 +54,9 @@ def __repr__(self):
         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 @@ def __init__(self, conf):
 
     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
@@ -575,7 +575,6 @@ def set_dash_test_params(self, num_nodes, masterodes_count, extra_args=None, fas
         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
@@ -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
@@ -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")