tests: Remove unused testing code #14312

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

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:

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 9a589240a8..238b50387d 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -161,8 +161,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         success = TestStatus.FAILED
 
         try:
-            if self.options.usecli and not self.supports_cli:
-                raise SkipTest("--usecli specified but test does not support using CLI")
+            if self.options.usecli:
+                if not self.supports_cli:
+                    raise SkipTest("--usecli specified but test does not support using CLI")
+                self.skip_if_no_cli()
             self.skip_test_if_missing_module()
             self.setup_chain()
             self.setup_network()

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

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

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

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.

DrahtBot commented at 8:38 PM on September 24, 2018: member

No more conflicts as of last run.

practicalswift force-pushed on Sep 24, 2018

practicalswift commented at 8:50 PM on September 24, 2018: contributor

@MarcoFalke Thanks for the quick review. Updated. Please re-review :-)

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!

practicalswift force-pushed on Sep 24, 2018

fanquake added the label Tests on Sep 25, 2018

DrahtBot added the label Needs rebase on Sep 27, 2018

practicalswift force-pushed on Sep 27, 2018

DrahtBot removed the label Needs rebase on Sep 27, 2018

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 %

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) :-)

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.

promag commented at 10:11 AM on October 2, 2018: member

Concept ACK, agree with rationale.

Mind sharing how you find these unused code?

tests: Remove unused testing code e17da14e83

practicalswift force-pushed on Oct 2, 2018

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 :-)

practicalswift commented at 1:44 PM on October 2, 2018: contributor

Closing this PR in favour of #14365 as suggested by @promag :-)

practicalswift closed this on Oct 2, 2018

MarcoFalke referenced this in commit d26d15c6c8 on Nov 7, 2018

practicalswift deleted the branch on Apr 10, 2021

knst referenced this in commit edd2d3b002 on Aug 10, 2021

pravblockc referenced this in commit 81b358bae3 on Aug 11, 2021

pravblockc referenced this in commit 3e7efd5f32 on Aug 11, 2021

pravblockc referenced this in commit 1ee408473a on Aug 11, 2021

pravblockc referenced this in commit 19e8d9664e on Aug 12, 2021

pravblockc referenced this in commit 12047d77d0 on Aug 12, 2021

gades referenced this in commit 199e30639d on May 11, 2022

DrahtBot locked this on Aug 16, 2022