Add listunspent() test for spendable/unspendable UTXO #7822

pull joaopaulofonseca wants to merge 2 commits into bitcoin:master from uphold:support/add-test-listunspent changing 7 files +98 −145
  1. joaopaulofonseca commented at 10:30 am on April 6, 2016: contributor

    This PR adds a test on wallet.py covering the behavior of spendable/unspendable value for UTXO, when calling RPC listunspent.

    This value should be different for address or private key import.

  2. laanwj added the label Tests on Apr 6, 2016
  3. in qa/rpc-tests/listunspent.py: in 0d8581d4a3 outdated
    63+        # retrieve that transaction and check if the transaction inputs are not among the unspents
    64+        rawtx = self.nodes[0].getrawtransaction(txid, 1)
    65+        for txin in rawtx["vin"]:
    66+            for utxo in list_unspent_after:
    67+                if (txin["txid"] == utxo["txid"] and txin["vout"] == utxo["vout"]):
    68+                    raise AssertionError("Fount used output on UTXOs list")
    


    MarcoFalke commented at 3:57 pm on April 6, 2016:
    Nit: typo
  4. in qa/rpc-tests/listunspent.py: in 0d8581d4a3 outdated
    87+        assert(not self.nodes[1].listunspent()[0]["spendable"])
    88+
    89+        try:
    90+            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1)
    91+        except JSONRPCException:
    92+            print("Impossible to send coins. Available output is unspendable.")
    


    MarcoFalke commented at 3:59 pm on April 6, 2016:
    You may want to try assert_raises instead.
  5. in qa/rpc-tests/listunspent.py: in 0d8581d4a3 outdated
    105+        assert_equal(len(self.nodes[1].listunspent()), 1)
    106+        assert(self.nodes[1].listunspent()[0]["spendable"])
    107+
    108+        try:
    109+            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
    110+        except JSONRPCException, e:
    


    MarcoFalke commented at 4:00 pm on April 6, 2016:
    Please use py3 syntax here: as e:
  6. in qa/rpc-tests/listunspent.py: in 0d8581d4a3 outdated
    106+        assert(self.nodes[1].listunspent()[0]["spendable"])
    107+
    108+        try:
    109+            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
    110+        except JSONRPCException, e:
    111+            raise AssertionError("Not possible to send coins. Possible unsufficient funds.")
    


    MarcoFalke commented at 4:01 pm on April 6, 2016:
    Nit: No need to shadow the original exception. You can just remove the try-except block altogether and only leave the sendtoaddress

    joaopaulofonseca commented at 5:31 pm on April 6, 2016:
    Thanks for the tips. Fixed and rebased.
  7. MarcoFalke commented at 4:01 pm on April 6, 2016: member
    Thanks, Concept ACK.
  8. joaopaulofonseca force-pushed on Apr 6, 2016
  9. joaopaulofonseca renamed this:
    [qa] Add test to RPC listunspent
    Add test to RPC listunspent
    on Apr 7, 2016
  10. laanwj commented at 2:24 pm on April 14, 2016: member
    I’m a bit divided whether I should propose to add this into the wallet test, which already uses listunspent. This currently adds some startup/teardown overhead (test takes ~9 seconds) just to test one call. I don’t feel strongly about that though. What do you think @MarcoFalke?
  11. MarcoFalke commented at 8:34 am on April 15, 2016: member

    startup/teardown overhead

    I think such overhead is rather small. (Note that disablewallet.py runs in less than a second)

    The overhead probably is caused by setting up the bidirectional connections and sync_all() calls.

    I have not yet looked closely at the test (I try to avoid large diffs right now in the /qa subfolder as each patch may or may not silently conflict with the python3 refactoring work.) @jpdffonseca Would you mind to take a look if this can be neatly squashed into the wallet.py test, as suggested by @laanwj ?

  12. joaopaulofonseca commented at 8:48 am on April 15, 2016: contributor
    @MarcoFalke and @laanwj yes that makes sense. I will try to squash this verifications into wallet.py.
  13. joaopaulofonseca force-pushed on Apr 18, 2016
  14. joaopaulofonseca force-pushed on Apr 18, 2016
  15. joaopaulofonseca renamed this:
    Add test to RPC listunspent
    Add wallet test to check spendable/unspendable UTXO on RPC listunspent
    on Apr 18, 2016
  16. joaopaulofonseca commented at 2:18 pm on April 18, 2016: contributor
    @MarcoFalke, after checking that wallet.py already covers some of the tests for listuspent(), I discarded the separate test file and merged the tests regarding the import of bitcoin addresses and private keys into wallet.py. The test execution time should remain the same.
  17. joaopaulofonseca renamed this:
    Add wallet test to check spendable/unspendable UTXO on RPC listunspent
    Add listunspent test for spendable/unspendable UTXO
    on Apr 18, 2016
  18. joaopaulofonseca renamed this:
    Add listunspent test for spendable/unspendable UTXO
    Add listunspent() test for spendable/unspendable UTXO
    on Apr 18, 2016
  19. MarcoFalke commented at 7:03 pm on April 18, 2016: member

    Thanks. Looks better now.

    utACK 3b2eedc

  20. Move method to check matches within arrays on util.py fa942c755a
  21. Add test to check spendable and unspendable UTXO on RPC listunspent 5d217decc1
  22. in qa/rpc-tests/wallet.py: in 3b2eedcba3 outdated
    5@@ -6,6 +6,27 @@
    6 from test_framework.test_framework import BitcoinTestFramework
    7 from test_framework.util import *
    8 
    9+def check_array_result(object_array, to_match, expected):
    


    MarcoFalke commented at 7:04 pm on April 18, 2016:
    If you copied this function from elsewhere, wouldn’t it make sense to move it instead to test_framework/util.py, so the utility function can be used in other tests as well?

    joaopaulofonseca commented at 9:26 pm on April 18, 2016:
    Indeed.. It is used by multiple tests. I think it comes in handy for current and future test cases. If you guys agree I guess this could be moved to test_framework/util.py to be reused whenever needed..

    MarcoFalke commented at 10:21 pm on April 18, 2016:
    Sure, go ahead.

    laanwj commented at 9:21 am on April 19, 2016:
    Do you intend to do so in this pull? If so I’ll wait with merging.

    joaopaulofonseca commented at 10:14 am on April 19, 2016:
    Yes, since we are on this topic.
  23. joaopaulofonseca force-pushed on Apr 19, 2016
  24. joaopaulofonseca commented at 11:37 am on April 19, 2016: contributor

    Moved assert_array_result() to test_framework/util.py.

    I noticed that the previous method on receivedby.py had a bug when using the flag should_not_find. If the flag is set to True, the validation wouldn’t work correctly when the values passed as to_match are indeed found inside object_array, and no assertion is raised.

  25. laanwj commented at 12:57 pm on April 19, 2016: member
    utACK 5d217de
  26. laanwj merged this on Apr 19, 2016
  27. laanwj closed this on Apr 19, 2016

  28. laanwj referenced this in commit 0e6fd5e4af on Apr 19, 2016
  29. fixe deleted the branch on Apr 19, 2016
  30. MarcoFalke referenced this in commit 6862627ce6 on Apr 19, 2016
  31. nomnombtc referenced this in commit 5c020a5bcb on Nov 11, 2016
  32. nomnombtc referenced this in commit 47fd3b6eeb on Nov 12, 2016
  33. nomnombtc referenced this in commit 9811199a3e on Nov 13, 2016
  34. sickpig referenced this in commit f5f418a5d9 on Nov 14, 2016
  35. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-04 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me