qa: generate --> generatetoaddress change to allow tests run without wallet #14236

pull sanket1729 wants to merge 1 commits into bitcoin:master from sanket1729:tests_final changing 22 files +58 −107

sanket1729 commented at 5:24 AM on September 17, 2018: contributor

Addresses #14216 . Changed Changed get_deterministic_priv_key() to return named tuple(address, key) I have tried to be exhaustive as possible in maximum coverage for non-wallet mode without affecting any coverage for wallet mode.

However, I could not check the tests in wallet mode because of timeout issues. Hopefully, travis job checks those.

Tests feature_block.py, feature_logging.py and feature_reindex.py were skipping despite having no direct dependency on any wallet functions. So, I have also disabled the skip_test_no_wallet() for those files too.
fanquake added the label Tests on Sep 17, 2018
sanket1729 force-pushed on Sep 17, 2018

Changed functional tests which do not require wallets to run without

skipping  .Addreses #14216. Changed get_deterministic_priv_key() to a

named tuple

0ca4c8b3c6

in test/functional/test_framework/test_node.py:103 in b3b8f04c49 outdated
```
  99 | @@ -99,17 +100,18 @@ def __init__(self, i, datadir, *, rpchost, timewait, bitcoind, bitcoin_cli, mock
 100 |  
 101 |      def get_deterministic_priv_key(self):
 102 |          """Return a deterministic priv key in base58, that only depends on the node's index"""
 103 | +        address_key_pair = collections.namedtuple('address_key_pair', ['address', 'key'])
```
laanwj commented at 12:56 PM on September 17, 2018:

namedtuple defines a type: please use CamelCase for the name (e.g. AddressKeyPair) and define it outside of the function (see other uses of namedtuple for consistency)

sanket1729 commented at 2:44 PM on September 17, 2018:

Fixed. Thanks for pointing out, I will make sure to read existing conventions for future PR's.
sanket1729 force-pushed on Sep 17, 2018
MarcoFalke renamed this:
~~generate --> generatetoaddress trivial change~~
qa: generate --> generatetoaddress change to allow tests run without wallet
on Sep 17, 2018
MarcoFalke merged this on Sep 17, 2018
MarcoFalke closed this on Sep 17, 2018
MarcoFalke referenced this in commit 4901c00792 on Sep 17, 2018
MarcoFalke commented at 6:08 PM on September 17, 2018: member

utACK 0ca4c8b3c6

in test/functional/test_framework/test_node.py:20 in 0ca4c8b3c6

  16 | @@ -17,6 +17,7 @@
  17 |  import tempfile
  18 |  import time
  19 |  import urllib.parse
  20 | +import collections

promag commented at 9:23 PM on September 17, 2018:

nit, sort.

in test/functional/rpc_preciousblock.py:46 in 0ca4c8b3c6
```
  45 |          self.setup_nodes()
  46 |  
  47 |      def run_test(self):
  48 |          self.log.info("Ensure submitblock can in principle reorg to a competing chain")
  49 | -        self.nodes[0].generate(1)
  50 | +        gen_address = lambda i: self.nodes[i].get_deterministic_priv_key().address  # A non-wallet address to mine to
```
promag commented at 9:27 PM on September 17, 2018:

nit, in remaining tests this auxiliary function is not used, maybe it could be removed and inline where necessary.

MarcoFalke commented at 10:03 PM on September 17, 2018:

There are 5 instances of calling gen_address in this test. I think in-lining would make this test overly verbose.

promag commented at 10:37 PM on September 17, 2018:

IMO it should be consistent, which helps in moving code around, copy&paste and readability. So either expose the auxiliary function or inline everywhere.

sanket1729 commented at 11:14 PM on September 17, 2018:

My rationale was make a clean looking code. Wherever there were many occurrences for the call to get_deterministic_priv_key(), I used a variable (in 1 test case where there was only 1 node) and auxiliary function in this test(where there are multiple nodes).
promag commented at 9:33 PM on September 17, 2018: member

utACK 0ca4c8b, left 2 comments but aren't worth the effort of a pull.

Agree with @laanwj in #14216 (comment). IIUC this change improves coverage when wallet is not enabled since the tests are not skipped.
deadalnix referenced this in commit d2436be68d on Feb 11, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#14216 Rewrite functional tests to not use the wallet