[tests] Test listwallets RPC #11310

pull mess110 wants to merge 1 commits into bitcoin:master from mess110:master changing 1 files +2 −0

mess110 commented at 6:58 PM on September 12, 2017: contributor

No description provided.
in test/functional/wallet-accounts.py:84 in 1824188caf outdated
```
  80 | @@ -78,54 +81,54 @@ def run_test(self):
  81 |          for account in accounts:
  82 |              address = node.getaccountaddress(account)
  83 |              account_addresses[account] = address
  84 | -            
  85 | +
```
promag commented at 7:08 PM on September 12, 2017:

Don't commit this and the following changes.

mess110 commented at 7:21 PM on September 12, 2017:

Was thinking of the boy scout rule, but yes, it is unrelated to the patch

in test/functional/wallet-accounts.py:31 in 1824188caf outdated

  27 | @@ -27,6 +28,8 @@ def run_test(self):
  28 |          # Check that there's no UTXO on any of the nodes
  29 |          assert_equal(len(node.listunspent()), 0)
  30 |  
  31 | +        assert_equal(len(node.listwallets()), 1)

promag commented at 7:09 PM on September 12, 2017:

Assert the content instead?

promag changes_requested
mess110 force-pushed on Sep 12, 2017
jnewbery commented at 8:33 PM on September 12, 2017: member

I have a test for listwallets in #10740 in multiwallet.py, which I think makes more sense than adding to wallet-accounts.py. The commit is here: https://github.com/bitcoin/bitcoin/pull/10740/commits/915c65a3bd6b7ace49e7046d78574390d2302e6f

I suggest you either close this PR or change it to add the listwallets test to multiwallet.py.
mess110 force-pushed on Sep 12, 2017
mess110 commented at 8:53 PM on September 12, 2017: contributor

I moved the code to multiwallet.py.

I would close this PR if we know #10740 will be merged. The feature is clearly helpful, and will probably be merged.

Taking this into account, if you think this patch is still useful, please continue the review process, otherwise please close it.
promag commented at 9:07 PM on September 12, 2017: member

Easy utACK d510ec5.
jnewbery commented at 9:33 PM on September 12, 2017: member

#10740 requires a lot of review and test, so I think it's fine to merge this first.

My only comment is that listwallets doesn't necessarily guarantee an order for the returned wallets, so perhaps converting to a set and doing a set comparison is more correct.

However, I'm probably just being overly picky. utACK d510ec5dff4de1df03463dda4238da3ceb60f8f3
Add listwallets RPC test to multiwallet.py fb1f325822
mess110 force-pushed on Sep 12, 2017
MarcoFalke merged this on Sep 12, 2017
MarcoFalke closed this on Sep 12, 2017
MarcoFalke referenced this in commit 96ac26e566 on Sep 12, 2017
MarcoFalke referenced this in commit b88ea16efa on Oct 3, 2017
MarcoFalke referenced this in commit b6468d3c42 on Oct 3, 2017
PastaPastaPasta referenced this in commit 5b461e922c on Sep 24, 2019
codablock referenced this in commit bc5918aa1b on Sep 24, 2019
barrystyle referenced this in commit b5f5fdc690 on Jan 22, 2020
random-zebra referenced this in commit 2d50b6e3b9 on Jun 9, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

mess110

promag

jnewbery