Skip to content

fix: correct pre-existing test mocking issues#13

Closed
b-l-u-e wants to merge 1 commit into2140-dev:masterfrom
b-l-u-e:fix/pre-existing-test-mocking
Closed

fix: correct pre-existing test mocking issues#13
b-l-u-e wants to merge 1 commit into2140-dev:masterfrom
b-l-u-e:fix/pre-existing-test-mocking

Conversation

@b-l-u-e
Copy link
Copy Markdown
Collaborator

@b-l-u-e b-l-u-e commented Mar 7, 2026

This PR fixes two unrelated test issues so the suite runs reliably without false failures.

1. Backend: test_clamps_target_in_rpc_call (RPC service)

problem: The test mocks _rpc_call with a single return_value. estimate_smart_fee() performs several RPC calls internally (estimatesmartfee, getblockchaininfo, getblockcount, getmempoolfeeratediagram, getblockstats). mock.call_args refers to the last call, not the estimatesmartfee call, so the assertion on the clamped target can raise or assert the wrong thing.

Error observed:

self = <test_rpc_service.TestRpcService testMethod=test_clamps_target_in_rpc_call>

    def test_clamps_target_in_rpc_call(self):
        with patch.object(self.rpc, '_rpc_call', return_value={"feerate": 0.0001}) as mock:
            self.rpc.estimate_smart_fee(1, "unset", 2)
>           self.assertEqual(mock.call_args[0][1][0], 2)  # params[0] should be 2
                             ^^^^^^^^^^^^^^^^^^^^^^^
E           IndexError: list index out of range

tests/test_rpc_service.py:88: IndexError
------------------------------------------------- Captured log call --------------------------------------------------
ERROR    rpc_service:rpc_service.py:204 Failed to include health stats: -1
============================================== short test summary info ===============================================
FAILED tests/test_rpc_service.py::TestRpcService::test_clamps_target_in_rpc_call - IndexError: list index out of range

2. Frontend: API constructor

problem: BitcoinCoreAPI did not accept a baseUrl argument. Tests pass new BitcoinCoreAPI('http://test-api:5001') so that fetch is mocked against that URL, but the argument was ignored and the client used /api, so assertions on the request URL fail and tests hit the real proxy instead of the mock.

Error observed:

 FAIL  src/services/api.test.ts
  BitcoinCoreAPI
    ✕ should fetch fee estimate (5 ms)
    ✕ should fetch block count with network info (1 ms)
    ✓ should handle fetch errors (2 ms)

  ● BitcoinCoreAPI › should fetch fee estimate

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    Expected: "http://test-api:5001/fees/2/economical/2", undefined
    Received: "/api/fees/2/economical/2", undefined

    Number of calls: 1

      22 |
      23 |     const result = await api.getFeeEstimate(2, 'economical', 2);
    > 24 |     expect(fetchMock).toHaveBeenCalledWith('http://test-api:5001/fees/2/economical/2', undefined);
         |                       ^
      25 |     expect(result.feerate).toBe(0.0001);
      26 |   });
      27 |

      at Object.<anonymous> (src/services/api.test.ts:24:23)

  ● BitcoinCoreAPI › should fetch block count with network info

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    Expected: "http://test-api:5001/blockcount", undefined
    Received: "/api/blockcount", undefined

    Number of calls: 1

      33 |
      34 |     const result = await api.getBlockCount();
    > 35 |     expect(fetchMock).toHaveBeenCalledWith('http://test-api:5001/blockcount', undefined);
         |                       ^
      36 |     expect(result.blockcount).toBe(800000);
      37 |     expect(result.chain_display).toBe('MAINNET');
      38 |   });

      at Object.<anonymous> (src/services/api.test.ts:35:23)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 1 passed, 3 total

Backend: estimate_smart_fee issues multiple RPC calls internally;
use side_effect and filter call_args_list for the right call.
Frontend: accept optional baseUrl in API constructor so test
suite uses the injected mock URL.
@ismaelsadeeq
Copy link
Copy Markdown
Collaborator

Thanks for the fix.
I have taken the frontend fix in #15, the backend issue is from the code, it should not have query the mempool diagram
for estimatesmartfee.

This is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants