Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: mock transport for tests #507

Closed
wants to merge 1 commit into from
Closed

Conversation

shramee
Copy link

@shramee shramee commented Nov 26, 2023

Testing functionality based on RPC response

Tests require setting up an RPC server with appropriate state to test different cases.
This makes tests slow and setup tedious.

This PR adds a mock transport mode to mock certain requests.

Features

Flow

JsonRpcTransport::send_request is implemented to,

  1. Tries to find mocked response for request_json in self.mocked_requests.
  2. Tries to find mocked response for request method mock in self.mocked_methods.
  3. Otherwise, Falls back to http_transport if available.
  4. Else throws an error (all requests must be resolved).

Optional HttpTransport fallback

Adding an http_transport allows falling back to HttpTransport for requests not mocked.

Mocking

Responses can be mocked for

  1. specific request json string (high priority) or
  2. all requests if w specific JsonRpcMethod.

Dev Experience

Helpful prints are added to stdout to help build mocks for the requests received and responses from fallback HTTP.

Use this code to mock this request

mock_transport.mock_request(
    "{\"id\":1,\"jsonrpc\":\"2.0\",\"method\":\"starknet_getBlo...}".into(),
    "{\"jsonrpc\":\"2.0\",\"result\":{\"block_hash\":\"0x5cc6b9...}".into()
);

image

@shramee shramee force-pushed the master branch 3 times, most recently from 010f65c to 8d4a802 Compare December 2, 2023 06:23
mock: use raw strings
@shramee
Copy link
Author

shramee commented Dec 2, 2023

Raw strings for cleaner JSON...

Use this code to mock this request

mock_transport.mock_request(
    r#"{"id":1,"jsonrpc":"2.0","method":"starknet_getBlockW...}"#.into(),
    r#"{"jsonrpc":"2.0","result":{"block_hash":"0x5b586405c...}"#.into()
);
image

@xJonathanLEI
Copy link
Owner

It's been... a while. While I agree that a mock transport would be helpful for testing stuff in downstream applications, I don't quite agree with the approach taken here.

In the context of starknet-rs, a "transport" is strictly a way of passing requests and responses back and forth. For example, an HTTP transport uses HTTP connections. I would imagine a mock transport to always use mock data for responses, instead of wrapping an HTTP transport as fallback in this PR.

I can see how being able to fallback to another transport is useful - sometimes you might want to just mock a few requests but use real data for the rest. However, I think the best way to implement this would be something like a DualProvider, where you can configure routing rules for requests.

On top of that, the PR seems to be incomplete. There are spots that have empty docs sections; there are println statements in non-test code; there are commented out lines. I don't think it's ready for review yet.

So I'm closing this one here. I do believe there's a place for something like this (probably not string-based though), but this one submitted likely isn't the one.

@shramee
Copy link
Author

shramee commented Jul 30, 2024

Why strings

It's easier to have it string based coz the response usually is string.
And exactly the same serde steps can be taken to parse the mocked string responses.

Ease of strings

The requests can be trivially mocked by just copying over the expected response from, say, a 'prepared' devnet.
Making it real easy to write tests that run in a fraction of what setting up a devnet/sequencer in CI would entail.

Using typed responses

We could indeed pass the request we want to mock through serde first and then export the rust variables. Or even write response type instantiations from scratch for tests.
This would be more type safe but would force the peeps to to learn about the types used in the the library for every request.

Next?

Yeah, this is quite out of date, was a proof of concept to begin with. DualProvider sounds interesting, would be nice if the learning overhead for writing tests is as low as possible.
Looking forward to see what we want to do about this. We might even abandon this issue if not many people need fast tests with starknet-rs.

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