Add tests for legacy Backend impl

[pkhry]

Description

As in the title. This PR adds tests for LegacyBackend implementation in subxt/backend.
Pr uses a mocked RpcClient implementation for providing expected responses.

Part of the work on #1677

[niklasad1]

I don't follow what you fixed with this but I think the previous code would try to fetch all storage keys in one retry block i.e, all storage key calls needs to be successful or retry it.

Looks more efficient to try each storage entry individually as you do now but maybe some other issue with it that I don't follow?

[pkhry]

Let's say we tried to fetch 2 entries:

• "A" and "B".
• "A" returns an OK result
• "B" returns an error
  The function would return Ok(streamOf(OK,Err)

So it just would not retry before this change

[jsdw]

Yup ok, so previously we wrapped code in retry, but that code didn't actually do any async stuff and just returned a StreamOf. Now, we are retrying each individual async request to fetch a storage value. Good catch!

[niklasad1]

Cool, you could also create a separate PR for this if you want get it in ASAP :)

[niklasad1]

Nice work, looks promising.

I would like some more wrapper functions to make the code a little easier to read such as subscription instead of using mpsc::channel etc. It took my a while to understand why the channel was not used for pure method calls etc.

[jsdw]

I wonder whether you can generalize this a little to then reuse in the UNstableBackend tests and generally for testing any RPC stuff, eg maybe we could have an interface like:

struct MockRpcClient { .. }

let mock_client = MockRpcClient::builder()
    // Mock methods
    .method(|name, params| {
        match name {
            // See methods and send back mock responses.
            "state_getStorage" => {
                Some(something_that_serializes_to_json)
            },
            // Return an error or None if we haven't mocked the method.
            // This will be turned into some error
            _ => {
                None
            }
        }
    })
    // Mock subscriptions
    .subscription(|name, params| {
        match name {
            "..." => {
                // Return some vec of serializable responses or something like that
                // which are then each sent as individual stream items. 
            }
        }
    })
    .build();

// `MockRpcClient` impls RpcClientT so can be used with Subxt backends etc.
subxt::rpc::RpcClient::new(mock_client);

// We can pull stuff into the closure to mock responses in order or whatever we like
fn mock_storage_responses() -> MockRpcClient {
    let mut mock_storage_responses = VecDeque::from_iter([
        mock_repsonse1, 
        mock_response2, 
        ..
    ]);

    MockRpcClient::builder()
        .method(move |name, params| {
            match name {
                "state_getStorage" => mock_storage_responses.pop_front(),
                _ => None
            }
        })
        .build()
} 

This might provide a nice sort of foundation to write a bunch of these sorts of tests

[pkhry]

i'm currently trying to see if it would work with unstable backend testing, although i'd prefer to make this change in the next PR

[jsdw]

This is a good start! I think it would be good to try and make it super clear/simple if possible, eg something along the lines of:

#[tokio::test]
async fn storage_fetch_values() {
    // Setup
    let mock_data = [
        ("ID1", Ok("Data1".as_bytes())),
        ("ID2", Err(RpcError::DisconnectedWillReconnect("Reconnecting".to_string()))),
        ("ID2", Ok("Data2".as_bytes())),
        ("ID3", Err(RpcError::DisconnectedWillReconnect("Reconnecting".to_string()))),
        ("ID3", Ok("Data3".as_bytes())),
    ];
    let mock_client = mock_storage_client(mock_data.clone());

    // Test
    let backend: LegacyBackend<Conf> = LegacyBackend::builder().build(rpc_client);
    let response = backend
        .storage_fetch_values(
            mock_data.iter().map(|x| x.0.clone().into()).collect(),
            crate::utils::H256::random(),
        )
        .await
        .unwrap();

    let response = response
        .map(|x| x.unwrap())
        .collect::<Vec<StorageResponse>>()
        .await;
    let expected: Vec<_> = mock_data
        .iter()
        .map(|(key, value)| storage_response(key, value))
        .collect();
    assert_eq!(expected, response)
}

There mock_storage_client is a small function that wraps `MockRpcClient and takes some IntoIterator (in my example at least) to return appropriate storage entries

[pkhry]

factored some of the common code in the last commit, let me know if it looks good to you

[niklasad1]

LGTM

[jsdw]

Prob worth a comment for why this is necessary instead of just importing SubstrateConfig or whatever :)

[jsdw]

The tests look much cleaner now; nice one!

I still would like to see a more generic MockRpcClient thing that I think might help to simplify some of the code around the tests, but can be tried out as part of a separate PR!