Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Sep 17, 2018

Conversation

@sanket1729
Copy link
Contributor

@sanket1729 sanket1729 commented Sep 17, 2018

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.

@@ -99,17 +100,18 @@ def __init__(self, i, datadir, *, rpchost, timewait, bitcoind, bitcoin_cli, mock

def get_deterministic_priv_key(self):
"""Return a deterministic priv key in base58, that only depends on the node's index"""
address_key_pair = collections.namedtuple('address_key_pair', ['address', 'key'])
Copy link
Member

@laanwj laanwj Sep 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)

Copy link
Contributor Author

@sanket1729 sanket1729 Sep 17, 2018

Fixed. Thanks for pointing out, I will make sure to read existing conventions for future PR's.

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

named tuple
@MarcoFalke MarcoFalke changed the title generate --> generatetoaddress trivial change qa: generate --> generatetoaddress change to allow tests run without wallet Sep 17, 2018
@MarcoFalke MarcoFalke merged commit 0ca4c8b into bitcoin:master Sep 17, 2018
2 checks passed
MarcoFalke added a commit that referenced this issue Sep 17, 2018
…s run without wallet

0ca4c8b Changed functional tests which do not require wallets to run without (sanket1729)

Pull request description:

  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.

Tree-SHA512: 8f84bd8400a732d4266c7518d5cbcf1eb761f623a64a74849e0470142c8ef22cb75364474ddae75d9213c3d16659a52917b5ed979a313695da6abd16c4fd7445
@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Sep 17, 2018

utACK 0ca4c8b

Copy link
Member

@promag promag left a comment

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.

@@ -17,6 +17,7 @@
import tempfile
import time
import urllib.parse
import collections
Copy link
Member

@promag promag Sep 17, 2018

nit, sort.

test/functional/rpc_preciousblock.py Show resolved Hide resolved
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Feb 11, 2020
…s run without wallet

Summary:
0ca4c8b3c6 Changed functional tests which do not require wallets to run without (sanket1729)

Pull request description:

  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.

Tree-SHA512: 8f84bd8400a732d4266c7518d5cbcf1eb761f623a64a74849e0470142c8ef22cb75364474ddae75d9213c3d16659a52917b5ed979a313695da6abd16c4fd7445

Backport of Core [[https://github.com/bitcoin/bitcoin/pull/14236/files | PR14236]]
bitcoin/bitcoin#14236

Test Plan:
  cmake -GNinja .. -DBUILD_BITCOIN_WALLET=OFF
  ninja check-functional-extended

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5267
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants