[tests] Remove 'account' API from wallet functional tests #13075

jnewbery · 2018-04-25T20:06:08Z

Next step in #12952. Removes all usage of the 'account' API from the wallet functional tests, except:

rpc_deprecated.py (which specifically tests the -deprecatedrpc=accounts command line argument is working properly).
wallet_labels.py (which tests that both the 'label' and 'account' APIs work in V0.17).

'account' API usage for both of those tests can be removed once V0.17 has been branched.

Also excluded is:

wallet_importprunedfunds.py (which fails due to a bitcoind OOM error)

laanwj · 2018-04-26T08:26:02Z

Nice!
Concept ACK.

jonasschnelli · 2018-04-26T08:29:24Z

Concept ACK.
I think combining some commits (especially the flake8 fixes) would help in review.

laanwj · 2018-05-01T14:01:43Z

This currently fails Travis due to an OOM issue exposed by wallet_importprunedfunds.py

Ref: this issue is tracked in #13078, and so this PR depends on that being fixed

jnewbery · 2018-05-01T15:13:02Z

I've squashed all the flake8 fixes into a single commit as requested by @jonasschnelli , and opened a PR for just those fixes at #13136.

…l tests a533834 [tests] Fix flake8 warnings in several wallet functional tests (John Newbery) Pull request description: Fixes flake8 warnings in several wallet functional tests. Several wallet functional tests need rewrite to remove the accounts API (#13075). To prepare for that, I fixed all the flake8 warnings in those tests. #13075 is blocked on a bitcoind bug. This PR is just the flake8 fixes so we're not completely blocked. Tree-SHA512: 2dc1d589b2f8f4318083a681e487532d0f8f3d57e8bc8f37b660b728ffc33329b88e9251eb223104aea89f293c3f4074ca700fe690e645617326b859da3e93c3

jnewbery · 2018-05-01T19:24:26Z

Rebased on master (to pick up the flake8 fixes)
Moved the wallet_importprunedfunds.py change to a separate PR (since it exposes a bitcoind OOM error)
Squashed all remaining commits.

Should be a fairly straightforward review.

maflcko · 2018-05-01T20:34:29Z

tests fail on travis

jnewbery · 2018-05-01T21:01:03Z

Thanks @MarcoFalke . Should now be fixed.

sipa · 2018-05-01T22:07:07Z

Concept ACK

maflcko · 2018-05-01T23:25:35Z

test/functional/wallet_keypool_topup.py

@@ -25,7 +25,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
    def set_test_params(self):
        self.setup_clean_chain = True
        self.num_nodes = 2
-        self.extra_args = [['-deprecatedrpc=accounts'], ['-deprecatedrpc=accounts', '-keypool=100', '-keypoolmin=20']]
+        self.extra_args = [[], ['-keypool=100', '-keypoolmin=20']]


What does keypoolmin do?

Looks like this was added in d34957e and it is fine to just remove.

maflcko · 2018-05-01T23:28:13Z

test/functional/wallet_listtransactions.py

-                            {"category": "receive", "amount": Decimal("0.1")},
-                            {"txid": txid, "account": "watchonly"})
+        assert not [tx for tx in self.nodes[0].listtransactions("*", 100, 0, False) if "label" in tx and tx["label"] == "watchonly"]
+        txs = [tx for tx in self.nodes[0].listtransactions("*", 100, 0, True) if "label" in tx and tx['label'] == 'watchonly']


nit: Could use named args for those True and False?

You can't mix positional and named args, so I've named them all.

maflcko · 2018-05-01T23:32:42Z

utACK 6b7a4ea7e9929d0ad65c7caf822f44ab6c01a475. Some style nits/questions.

meshcollider · 2018-05-02T09:14:31Z

Concept ACK

Removes usage of account API from the following functional tests: - wallet_listreceivedby.py - wallet_basic.py - wallet_keypool_topup.py - wallet_txn_clone.py - wallet_listsinceblock.py - wallet_import_rescan.py - wallet_listtransactions.py - wallet_txn_doublespend.py

jnewbery · 2018-05-02T21:55:07Z

Thanks for the reviews. I've addressed @MarcoFalke 's comments.

→ git diff 6b7a4ea7e9929d0ad65c7caf822f44ab6c01a475 5d536619abda745c298fd827a856df775f223241
diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py
index 30a0c9a76..d657dc30c 100755
--- a/test/functional/wallet_keypool_topup.py
+++ b/test/functional/wallet_keypool_topup.py
@@ -25,7 +25,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
-        self.extra_args = [[], ['-keypool=100', '-keypoolmin=20']]
+        self.extra_args = [[], ['-keypool=100']]
 
     def run_test(self):
         wallet_path = os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")
diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py
index 3de65b9b5..7cf2e456c 100755
--- a/test/functional/wallet_listtransactions.py
+++ b/test/functional/wallet_listtransactions.py
@@ -94,8 +94,8 @@ class ListTransactionsTest(BitcoinTestFramework):
         txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1)
         self.nodes[1].generate(1)
         self.sync_all()
-        assert not [tx for tx in self.nodes[0].listtransactions("*", 100, 0, False) if "label" in tx and tx["label"] == "watchonly"]
-        txs = [tx for tx in self.nodes[0].listtransactions("*", 100, 0, True) if "label" in tx and tx['label'] == 'watchonly']
+        assert not [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=False) if "label" in tx and tx["label"] == "watchonly"]
+        txs = [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=True) if "label" in tx and tx['label'] == 'watchonly']
         assert_array_result(txs, {"category": "receive", "amount": Decimal("0.1")}, {"txid": txid})
 
         self.run_rbf_opt_in_test()

5d53661 [tests] Remove 'account' API from wallet functional tests (John Newbery) Pull request description: Next step in #12952. Removes all usage of the 'account' API from the wallet functional tests, except: - rpc_deprecated.py (which specifically tests the `-deprecatedrpc=accounts` command line argument is working properly). - `wallet_labels.py` (which tests that both the 'label' and 'account' APIs work in V0.17). 'account' API usage for both of those tests can be removed once V0.17 has been branched. Also excluded is: - `wallet_importprunedfunds.py` (which fails due to a bitcoind OOM error) Tree-SHA512: 6701b32f83d2d47597ba093ded665d7aa630f7a9c759ff15e3e33a3e3bc7600e8d29cf4e72aed5f8f9f6769cc9b614c681951720eab1ed2473f5f8dec57e7a6f

This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382

…by label da427db Rename ListTransactions filter variable (Russell Yanofsky) 65b740f [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: This change partially reverts #13075 and #14023. Fixes #14382 Tree-SHA512: 8c4e56104b3a45784cdc06bae8e5facdfff04fe3545b63a35e0ec2e440a41b79d84833ca4c4e728d8af7ebb8a519303a9eda7bee4bbfb92bd50c58587a33eb30

This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382

…funds.py 38040c3 [tests] Remove accounts from wallet_importprunedfunds.py (John Newbery) Pull request description: This was split from bitcoin#13075 to not block review/merge of that PR. Tree-SHA512: 631d7139ed2bda5222ec395cc75720261e2e1f741dba04723d09fe04ef6cf92222a3679d886026ec33e2db2d1e2fa1a0f36c2451581d0f733a9939a98c7118ab

…nal tests 5d53661 [tests] Remove 'account' API from wallet functional tests (John Newbery) Pull request description: Next step in bitcoin#12952. Removes all usage of the 'account' API from the wallet functional tests, except: - rpc_deprecated.py (which specifically tests the `-deprecatedrpc=accounts` command line argument is working properly). - `wallet_labels.py` (which tests that both the 'label' and 'account' APIs work in V0.17). 'account' API usage for both of those tests can be removed once V0.17 has been branched. Also excluded is: - `wallet_importprunedfunds.py` (which fails due to a bitcoind OOM error) Tree-SHA512: 6701b32f83d2d47597ba093ded665d7aa630f7a9c759ff15e3e33a3e3bc7600e8d29cf4e72aed5f8f9f6769cc9b614c681951720eab1ed2473f5f8dec57e7a6f

… incoming transactions by label 89306ab [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: Backport of PR bitcoin#14411 to v0.17. This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382 Tree-SHA512: 1f8300e1a79e826cd706561265b8788deef505fa510be1a76ed9a62e5fca37cf6a741423ac0e5de2a36d6e8b9f25f141885455aacacbbf6474814e6eae406a27

…nal tests 5d53661 [tests] Remove 'account' API from wallet functional tests (John Newbery) Pull request description: Next step in bitcoin#12952. Removes all usage of the 'account' API from the wallet functional tests, except: - rpc_deprecated.py (which specifically tests the `-deprecatedrpc=accounts` command line argument is working properly). - `wallet_labels.py` (which tests that both the 'label' and 'account' APIs work in V0.17). 'account' API usage for both of those tests can be removed once V0.17 has been branched. Also excluded is: - `wallet_importprunedfunds.py` (which fails due to a bitcoind OOM error) Tree-SHA512: 6701b32f83d2d47597ba093ded665d7aa630f7a9c759ff15e3e33a3e3bc7600e8d29cf4e72aed5f8f9f6769cc9b614c681951720eab1ed2473f5f8dec57e7a6f

… incoming transactions by label 89306ab [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: Backport of PR bitcoin#14411 to v0.17. This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382 Tree-SHA512: 1f8300e1a79e826cd706561265b8788deef505fa510be1a76ed9a62e5fca37cf6a741423ac0e5de2a36d6e8b9f25f141885455aacacbbf6474814e6eae406a27

…nctional tests a533834 [tests] Fix flake8 warnings in several wallet functional tests (John Newbery) Pull request description: Fixes flake8 warnings in several wallet functional tests. Several wallet functional tests need rewrite to remove the accounts API (bitcoin#13075). To prepare for that, I fixed all the flake8 warnings in those tests. bitcoin#13075 is blocked on a bitcoind bug. This PR is just the flake8 fixes so we're not completely blocked. Tree-SHA512: 2dc1d589b2f8f4318083a681e487532d0f8f3d57e8bc8f37b660b728ffc33329b88e9251eb223104aea89f293c3f4074ca700fe690e645617326b859da3e93c3

…funds.py 38040c3 [tests] Remove accounts from wallet_importprunedfunds.py (John Newbery) Pull request description: This was split from bitcoin#13075 to not block review/merge of that PR. Tree-SHA512: 631d7139ed2bda5222ec395cc75720261e2e1f741dba04723d09fe04ef6cf92222a3679d886026ec33e2db2d1e2fa1a0f36c2451581d0f733a9939a98c7118ab

…ctions by label da427db Rename ListTransactions filter variable (Russell Yanofsky) 65b740f [wallet] Restore ability to list incoming transactions by label (Russell Yanofsky) Pull request description: This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382 Tree-SHA512: 8c4e56104b3a45784cdc06bae8e5facdfff04fe3545b63a35e0ec2e440a41b79d84833ca4c4e728d8af7ebb8a519303a9eda7bee4bbfb92bd50c58587a33eb30

jnewbery force-pushed the remove_functional_tests_accounts branch from 5489efd to e55ab86 Compare April 25, 2018 20:28

jnewbery mentioned this pull request Apr 25, 2018

Remove wallet 'account' API #12952

Closed

6 tasks

laanwj added the Tests label May 1, 2018

jnewbery changed the title ~~Remove 'account' API from wallet functional tests~~ [Don't merge yet] Remove 'account' API from wallet functional tests May 1, 2018

jnewbery force-pushed the remove_functional_tests_accounts branch from e55ab86 to 09de916 Compare May 1, 2018 15:06

jnewbery mentioned this pull request May 1, 2018

[tests] Fix flake8 warnings in several wallet functional tests #13136

Merged

jnewbery force-pushed the remove_functional_tests_accounts branch from 09de916 to 659e92b Compare May 1, 2018 15:32

jnewbery force-pushed the remove_functional_tests_accounts branch 2 times, most recently from 5b0ec78 to 5c1fcb9 Compare May 1, 2018 19:21

jnewbery changed the title ~~[Don't merge yet] Remove 'account' API from wallet functional tests~~ [tests] Remove 'account' API from wallet functional tests May 1, 2018

jnewbery mentioned this pull request May 1, 2018

[tests] Remove accounts from wallet_importprunedfunds.py #13138

Merged

jnewbery force-pushed the remove_functional_tests_accounts branch from 5c1fcb9 to 6b7a4ea Compare May 1, 2018 20:57

maflcko reviewed May 1, 2018

View reviewed changes

jnewbery force-pushed the remove_functional_tests_accounts branch from 6b7a4ea to 5d53661 Compare May 2, 2018 21:54

maflcko merged commit 5d53661 into bitcoin:master May 10, 2018

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Nov 13, 2018

[wallet] Restore ability to list incoming transactions by label

65b740f

This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Nov 26, 2018

[wallet] Restore ability to list incoming transactions by label

2d9f104

This change partially reverts bitcoin#13075 and bitcoin#14023. Fixes bitcoin#14382

Fuzzbawls mentioned this pull request Jun 25, 2020

[Tracking] Remove wallet 'account' API PIVX-Project/PIVX#1706

Closed

11 tasks

xdustinface mentioned this pull request Dec 18, 2020

backport: Deprecate accounts dashpay/dash#3886

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

[tests] Remove 'account' API from wallet functional tests #13075

[tests] Remove 'account' API from wallet functional tests #13075

Uh oh!

Conversation

jnewbery commented Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Apr 26, 2018

Uh oh!

jonasschnelli commented Apr 26, 2018

Uh oh!

laanwj commented May 1, 2018

Uh oh!

jnewbery commented May 1, 2018

Uh oh!

jnewbery commented May 1, 2018

Uh oh!

maflcko commented May 1, 2018

Uh oh!

jnewbery commented May 1, 2018

Uh oh!

sipa commented May 1, 2018

Uh oh!

maflcko May 1, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko May 2, 2018

Choose a reason for hiding this comment

Uh oh!

jnewbery May 2, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko May 1, 2018

Choose a reason for hiding this comment

Uh oh!

jnewbery May 2, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 1, 2018

Uh oh!

meshcollider commented May 2, 2018

Uh oh!

jnewbery commented May 2, 2018

Uh oh!

Uh oh!

jnewbery commented Apr 25, 2018 •

edited

Loading