tests: Remove unused testing code #14312

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:remove-dead-testing-code changing 8 files +9 −51
  1. practicalswift commented at 7:44 pm on September 24, 2018: contributor

    Remove unused testing code.

    Rationale:

    • Less is more :-)
    • Unused code is by definition “untested”
    • YAGNI
  2. in test/functional/test_framework/test_framework.py:518 in 80032fb59f outdated
    514@@ -515,18 +515,6 @@ def skip_if_no_wallet(self):
    515         if not self.is_wallet_compiled():
    516             raise SkipTest("wallet has not been compiled.")
    517 
    518-    def skip_if_no_cli(self):
    


    MarcoFalke commented at 8:28 pm on September 24, 2018:

    Should probably call this:

     0diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
     1index 9a589240a8..238b50387d 100755
     2--- a/test/functional/test_framework/test_framework.py
     3+++ b/test/functional/test_framework/test_framework.py
     4@@ -161,8 +161,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
     5         success = TestStatus.FAILED
     6 
     7         try:
     8-            if self.options.usecli and not self.supports_cli:
     9-                raise SkipTest("--usecli specified but test does not support using CLI")
    10+            if self.options.usecli:
    11+                if not self.supports_cli:
    12+                    raise SkipTest("--usecli specified but test does not support using CLI")
    13+                self.skip_if_no_cli()
    14             self.skip_test_if_missing_module()
    15             self.setup_chain()
    16             self.setup_network()
    
  3. in test/functional/test_framework/mininode.py:316 in 80032fb59f outdated
    312@@ -313,7 +313,7 @@ def on_ping(self, message):
    313         self.send_message(msg_pong(message.nonce))
    314 
    315     def on_verack(self, message):
    316-        self.verack_received = True
    317+        pass
    


    MarcoFalke commented at 8:28 pm on September 24, 2018:
    ack
  4. in test/functional/test_framework/messages.py:1153 in 80032fb59f outdated
    1079-
    1080-    def serialize(self):
    1081-        return self.data
    1082-
    1083-    def __repr__(self):
    1084-        return "msg_generic()"
    


    MarcoFalke commented at 8:28 pm on September 24, 2018:
    ack
  5. in test/functional/test_framework/messages.py:62 in 80032fb59f outdated
    53@@ -54,9 +54,6 @@
    54 def sha256(s):
    55     return hashlib.new('sha256', s).digest()
    56 
    57-def ripemd160(s):
    58-    return hashlib.new('ripemd160', s).digest()
    


    MarcoFalke commented at 8:29 pm on September 24, 2018:
    When has this last been used? Not sure if we want to remove library functions that are still used in other branches. Same for the key.py changes.

    practicalswift commented at 8:42 pm on September 24, 2018:
    Last use was removed in b9f34e84befa7db6ff8c9b92a09d0dfa40388fb7

    MarcoFalke commented at 8:48 pm on September 24, 2018:
    ack
  6. in test/functional/feature_cltv.py:28 in 80032fb59f outdated
    24@@ -25,7 +25,7 @@
    25 
    26 # Reject codes that we might receive in this test
    27 REJECT_INVALID = 16
    28-REJECT_OBSOLETE = 17
    29+# REJECT_OBSOLETE = 17
    


    MarcoFalke commented at 8:31 pm on September 24, 2018:
    When was the first and last use? Might want to remove altogether after investigating that.

    practicalswift commented at 8:41 pm on September 24, 2018:
    Last use was removed in your commit fac3e22b18cd29053bc17065fd75db7b84ba6f40 :-)

    MarcoFalke commented at 8:48 pm on September 24, 2018:
    So please remove the constants here as well.
  7. DrahtBot commented at 8:38 pm on September 24, 2018: member
  8. practicalswift force-pushed on Sep 24, 2018
  9. practicalswift force-pushed on Sep 24, 2018
  10. practicalswift force-pushed on Sep 24, 2018
  11. practicalswift commented at 8:50 pm on September 24, 2018: contributor
    @MarcoFalke Thanks for the quick review. Updated. Please re-review :-)
  12. in test/functional/test_framework/messages.py:34 in 065bedcea3 outdated
    30@@ -31,7 +31,7 @@
    31 MY_SUBVERSION = b"/python-mininode-tester:0.0.3/"
    32 MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37)
    33 
    34-MAX_INV_SZ = 50000
    35+# MAX_INV_SZ = 50000
    


    MarcoFalke commented at 8:57 pm on September 24, 2018:
    remove? (This is no longer used after the removal of the comptool)

    practicalswift commented at 8:59 pm on September 24, 2018:
    Done!
  13. practicalswift force-pushed on Sep 24, 2018
  14. fanquake added the label Tests on Sep 25, 2018
  15. DrahtBot added the label Needs rebase on Sep 27, 2018
  16. practicalswift force-pushed on Sep 27, 2018
  17. DrahtBot removed the label Needs rebase on Sep 27, 2018
  18. DrahtBot commented at 4:43 am on September 28, 2018: member
    Coverage Change (pull 14312) Reference (master)
    Lines -0.0154 % 87.0361 %
    Functions +0.0926 % 84.1130 %
    Branches -0.0199 % 51.5451 %
  19. in test/functional/test_framework/test_framework.py:167 in 908c8883de outdated
    160@@ -161,8 +161,10 @@ def main(self):
    161         success = TestStatus.FAILED
    162 
    163         try:
    164-            if self.options.usecli and not self.supports_cli:
    165-                raise SkipTest("--usecli specified but test does not support using CLI")
    166+            if self.options.usecli:
    167+                if not self.supports_cli:
    168+                    raise SkipTest("--usecli specified but test does not support using CLI")
    169+                self.skip_if_no_cli()
    


    promag commented at 10:08 am on October 2, 2018:
    Does this change belong here?

    practicalswift commented at 10:26 am on October 2, 2018:
    That was suggested by @MarcoFalke in #14312 (review) :-)
  20. in test/functional/test_framework/messages.py:1139 in 908c8883de outdated
    1130@@ -1136,7 +1131,6 @@ def serialize(self):
    1131     def __repr__(self):
    1132         return "msg_block(block=%s)" % (repr(self.block))
    1133 
    1134-
    


    promag commented at 10:10 am on October 2, 2018:
    nit, unrelated.
  21. promag commented at 10:11 am on October 2, 2018: member

    Concept ACK, agree with rationale.

    Mind sharing how you find these unused code?

  22. tests: Remove unused testing code e17da14e83
  23. practicalswift force-pushed on Oct 2, 2018
  24. practicalswift commented at 11:01 am on October 2, 2018: contributor
    @promag vulture is a great tool for finding dead Python code. See #14365 for vulture integration in Travis :-)
  25. practicalswift commented at 1:44 pm on October 2, 2018: contributor
    Closing this PR in favour of #14365 as suggested by @promag :-)
  26. practicalswift closed this on Oct 2, 2018

  27. MarcoFalke referenced this in commit d26d15c6c8 on Nov 7, 2018
  28. practicalswift deleted the branch on Apr 10, 2021
  29. knst referenced this in commit edd2d3b002 on Aug 10, 2021
  30. pravblockc referenced this in commit 81b358bae3 on Aug 11, 2021
  31. pravblockc referenced this in commit 3e7efd5f32 on Aug 11, 2021
  32. pravblockc referenced this in commit 1ee408473a on Aug 11, 2021
  33. pravblockc referenced this in commit 19e8d9664e on Aug 12, 2021
  34. pravblockc referenced this in commit 12047d77d0 on Aug 12, 2021
  35. gades referenced this in commit 199e30639d on May 11, 2022
  36. DrahtBot locked this on Aug 16, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-18 00:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me