RPC Tests

[SachinMeier]

Currently, RPC Tests are commented out because bitcoind must be running to test them. I am thinking this should change to check if bitcoind is running. I am unsure of the best implementation, but this is what I am thinking. I was hoping to get feedback before I open a PR. test_rpc.py:

def is_active():
    try:
        p = Proxy()
        return True
    except ValueError: 
        return False

class Test_RPC(unittest.TestCase):
    _IS_ACTIVE = is_active()
    ...

We can then have each test begin by checking _IS_ACTIVE and passing if not.

[ccdle12]

Python unittest has as a decorator that can be applied to the entire test class. Also I think the Proxy instance needs to attempt to make call in order to know whether there is a connection or not, specifically it would raise a ConnectionRefusedError or something similar.

So you could do something like:

import unittest

from bitcoin.rpc import Proxy

def is_active():
    try:
        Proxy().getnewaddress()
        return True
    except ConnectionRefusedError: 
        return False

@unittest.skipUnless(is_active(), "no active connection to bitcoind found")
class Test_RPC(unittest.TestCase):
 ...

[dgpv]

getnewaddress() alters the state of the wallet in the bitcoind that is connected to. Since bitcoinlib tries to connect using parameters from ~/.bitcoin/bitcoin.conf, it can access the user's "non-test" bitcoind instance. Altering the state of the wallet for non-test instance will certainly not be expected by the user.

I think that better command to test that the daemon responds is echo: Proxy().echo("test") or something like this

[SachinMeier]

For me, simply instantiating a Proxy with an attempted connection yields the desired result. Please let me know if you get different results. Perhaps we could add a bitcoin-cli help call in to make sure it returns the correct value.

For reference, I implemented this in #242.

[ccdle12]

@dgpv has good points,

@SachinMeier I fetched the PR branch and tried to run the tests using: python3 -m unittest discover and received raised connection errors (since I'm not running an instance of bitcoind).

======================================================================
ERROR: test_verifymessage (test_rpc.Test_RPC)
----------------------------------------------------------------------
...
ConnectionRefusedError: [Errno 111] Connection refused

Let me know if I'm missing an assumed configuration for running the tests.

[SachinMeier]

I get a different result. All my tests pass. I can add a single call to theis_active function, say help. Can you add p.help() to the is_active function and let me know if you see different results?

[ccdle12]

hey @SachinMeier,

Calling p.help() works as long as the ConnectionRefusedError is handled in is_active().

So I think the reason we are seeing different errors is that if a user has the bitcoin folder initialized at the default path, the Proxy will run the constructor without raising an error but will have a runtime error when making the rpc calls in the tests (because bitcoind is not running) https://github.com/petertodd/python-bitcoinlib/blob/master/bitcoin/rpc.py#L136.

If the bitcoin folder is not initialized in the default path, the constructor will raise a ValueError like you mentioned above, which is currently caught by is_active() - https://github.com/petertodd/python-bitcoinlib/blob/master/bitcoin/rpc.py#L172