Add call args assertions bitcoin client test (#1541)

* Add assertions for call args for some bitcoin client mocks

* Simplify bitcoin core client

* Add assertions on call args for getblockcount

* Add test for make_request method of bitcoin core client

* Use make_request method for bitcoin connection error tests

* Remove old comments

* Use call arg assertions for all methods of bitcoin core client

squeaknode/bitcoin/bitcoin_core_client.py

@@ -73,9 +73,7 @@ class BitcoinCoreClient(BitcoinClient):
         }
         json_response = self.make_request(payload)
         result = json_response["result"]
-        block_count = int(result)
-        logger.debug("Got block_count: {}".format(block_count))
-        return block_count
+        return int(result)
 
     def get_block_hash(self, block_height: int) -> bytes:
         payload = {
@@ -86,9 +84,7 @@ class BitcoinCoreClient(BitcoinClient):
         }
         json_response = self.make_request(payload)
         result = json_response["result"]
-        block_hash = result
-        logger.debug("Got block_hash: {}".format(block_hash))
-        return bytes.fromhex(block_hash)
+        return bytes.fromhex(result)
 
     def get_block_header(self, block_hash: bytes, verbose: bool = False) -> CBlockHeader:
         payload = {
@@ -99,7 +95,6 @@ class BitcoinCoreClient(BitcoinClient):
         }
         json_response = self.make_request(payload)
         result = json_response["result"]
-        logger.debug("Got block_header: {}".format(result))
         header_bytes = bytes.fromhex(result)
         return CBlockHeader.deserialize(header_bytes)
 

tests/bitcoin/test_bitcoin_core_client.py

@@ -19,6 +19,8 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
+import json
+
 import mock
 import pytest
 from bitcoin.core import CBlockHeader
@@ -33,12 +35,32 @@ from squeaknode.bitcoin.exception import BitcoinRequestError
 
 
 @pytest.fixture
-def bitcoin_core_client():
+def bitcoin_host():
+    yield "fake_bitcoin_host"
+
+
+@pytest.fixture
+def bitcoin_port():
+    yield 5678
+
+
+@pytest.fixture
+def bitcoin_user():
+    yield "fake_user"
+
+
+@pytest.fixture
+def bitcoin_pass():
+    yield "fake_pass"
+
+
+@pytest.fixture
+def bitcoin_core_client(bitcoin_host, bitcoin_port, bitcoin_user, bitcoin_pass):
     yield BitcoinCoreClient(
-        host="fake_bitcoin_host",
-        port=5678,
-        rpc_user="fake_user",
-        rpc_password="fake_pass",
+        host=bitcoin_host,
+        port=bitcoin_port,
+        rpc_user=bitcoin_user,
+        rpc_password=bitcoin_pass,
         use_ssl=False,
         ssl_cert="fake_ssl_cert",
     )
@@ -161,68 +183,87 @@ def mock_invalid_status_response():
     yield MockInvalidStatusResponse()
 
 
-def test_get_block_count(bitcoin_core_client, mock_get_count_response, block_count):
+def test_make_request(bitcoin_host, bitcoin_port, bitcoin_user, bitcoin_pass, bitcoin_core_client, mock_empty_response):
     with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
-        mock_post.return_value = mock_get_count_response
-        retrieved_block_count = bitcoin_core_client.get_block_count()
+        mock_post.return_value = mock_empty_response
+        retrieved_json_response = bitcoin_core_client.make_request({})
+        (bitcoin_address,) = mock_post.call_args.args
+        request_json = mock_post.call_args.kwargs['data']
 
-        assert retrieved_block_count == block_count
+        assert bitcoin_host in bitcoin_address
+        assert str(bitcoin_port) in bitcoin_address
+        assert bitcoin_user in bitcoin_address
+        assert bitcoin_pass in bitcoin_address
+        assert json.loads(request_json) == {}
+        assert retrieved_json_response == {}
 
 
-def test_get_block_count_invalid_status(bitcoin_core_client, mock_invalid_status_response):
+def test_make_request_invalid_status(bitcoin_core_client, mock_invalid_status_response):
     with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
         mock_post.return_value = mock_invalid_status_response
 
         with pytest.raises(BitcoinRequestError):
-            bitcoin_core_client.get_block_count()
+            bitcoin_core_client.make_request({})
 
 
-def test_get_block_count_connection_error(bitcoin_core_client, mock_invalid_status_response):
+def test_make_request_connection_error(bitcoin_core_client, mock_invalid_status_response):
     with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
         mock_post.side_effect = ConnectionError()
 
         with pytest.raises(BitcoinRequestError):
-            bitcoin_core_client.get_block_count()
+            bitcoin_core_client.make_request({})
 
 
-def test_get_block_count_timeout_error(bitcoin_core_client, mock_invalid_status_response):
+def test_make_request_timeout_error(bitcoin_core_client, mock_invalid_status_response):
     with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
         mock_post.side_effect = Timeout()
 
         with pytest.raises(BitcoinRequestError):
-            bitcoin_core_client.get_block_count()
+            bitcoin_core_client.make_request({})
 
 
-def test_get_block_count_request_exception(bitcoin_core_client, mock_invalid_status_response):
-    # TODO: assert mocks called with correct args.
+def test_make_request_request_exception(bitcoin_core_client, mock_invalid_status_response):
     with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
         mock_post.side_effect = RequestException()
 
         with pytest.raises(BitcoinRequestError):
-            bitcoin_core_client.get_block_count()
+            bitcoin_core_client.make_request({})
+
+
+def test_get_block_count(bitcoin_host, bitcoin_port, bitcoin_core_client, mock_get_count_response, block_count):
+    with mock.patch.object(bitcoin_core_client, 'make_request', autospec=True) as mock_make_request:
+        mock_make_request.return_value = mock_get_count_response.json()
+        retrieved_block_count = bitcoin_core_client.get_block_count()
+        (payload,) = mock_make_request.call_args.args
+
+        assert payload['method'] == 'getblockcount'
+        assert retrieved_block_count == block_count
 
 
 def test_get_block_hash(bitcoin_core_client, mock_get_block_hash_response, block_count, block_hash):
-    # TODO: assert mocks called with correct args.
-    with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
-        mock_post.return_value = mock_get_block_hash_response
+    with mock.patch.object(bitcoin_core_client, 'make_request', autospec=True) as mock_make_request:
+        mock_make_request.return_value = mock_get_block_hash_response.json()
         retrieved_block_hash = bitcoin_core_client.get_block_hash(block_count)
+        (payload,) = mock_make_request.call_args.args
 
+        assert payload['method'] == 'getblockhash'
+        assert payload['params'] == [block_count]
         assert retrieved_block_hash == block_hash
 
 
 def test_get_block_header(bitcoin_core_client, mock_get_block_header_response, block_hash, block_header):
-    # TODO: assert mocks called with correct args.
-    with mock.patch('squeaknode.bitcoin.bitcoin_core_client.requests.post', autospec=True) as mock_post:
-        mock_post.return_value = mock_get_block_header_response
+    with mock.patch.object(bitcoin_core_client, 'make_request', autospec=True) as mock_make_request:
+        mock_make_request.return_value = mock_get_block_header_response.json()
         retrieved_block_header = bitcoin_core_client.get_block_header(
             block_hash)
+        (payload,) = mock_make_request.call_args.args
 
+        assert payload['method'] == 'getblockheader'
+        assert payload['params'] == [block_hash.hex(), False]
         assert retrieved_block_header == block_header
 
 
 def test_get_block_info_by_height(bitcoin_core_client, block_count, block_hash, block_header):
-    # TODO: assert mocks called with correct args.
     with mock.patch.object(bitcoin_core_client, 'get_block_hash', autospec=True) as mock_get_block_hash, \
             mock.patch.object(bitcoin_core_client, 'get_block_header', autospec=True) as mock_get_block_header:
         mock_get_block_hash.return_value = block_hash
@@ -230,6 +271,10 @@ def test_get_block_info_by_height(bitcoin_core_client, block_count, block_hash,
         retrieved_block_info = bitcoin_core_client.get_block_info_by_height(
             block_count)
 
+        assert mock_get_block_hash.call_args == mock.call(
+            block_count)
+        assert mock_get_block_header.call_args == mock.call(
+            block_hash)
         assert retrieved_block_info == BlockInfo(
             block_height=block_count,
             block_hash=block_hash,
@@ -238,11 +283,13 @@ def test_get_block_info_by_height(bitcoin_core_client, block_count, block_hash,
 
 
 def test_get_best_block_info(bitcoin_core_client, block_count, block_info):
-    # TODO: assert mocks called with correct args.
     with mock.patch.object(bitcoin_core_client, 'get_block_count', autospec=True) as mock_get_block_count, \
             mock.patch.object(bitcoin_core_client, 'get_block_info_by_height', autospec=True) as mock_get_block_info_by_height:
         mock_get_block_count.return_value = block_count
         mock_get_block_info_by_height.return_value = block_info
         retrieved_block_info = bitcoin_core_client.get_best_block_info()
 
+        assert mock_get_block_count.call_args == ()
+        assert mock_get_block_info_by_height.call_args == mock.call(
+            block_count)
         assert retrieved_block_info == block_info