What is the ideomatic way to abstract dependencies for unit testing (Dependency injection)?

code review
1

In my library, I make HTTP requests via reqwest. To test these functions, I currently use mockall to create mock objects that abstract the HTTP call.

#[cfg_attr(test, automock)]
pub(super) trait HttpFetcher {
    /// Send a HTTP POST request with JSON as payload.
    fn http_post_request(
        &self,
        url: &str,
        payload: serde_json::Value,
        options: &FetchOptions,
    ) -> Result<String, NetworkError>;
}

impl HttpFetcher for Client { 
// Implementation for http_post_request
}

In my tests I then create a mock object to abstract that call

let mut mock = MockHttpFetcher::new();
mock.expect_http_post_request().return_const({
    Ok("Sample response data".into())
});

Now in order to use the mock object, I have two functions: A public function that the consumer of the library is calling, which sets up the dependency and then calls a impl function that contains the actual logic.

pub fn query_data(options: &FetchOptions) -> Result<Project, QueryError> {
    let http_client = Client::new();
    query_data_impl(options, &http_client)
}

fn query_data_impl(
    options: &FetchOptions,
    http_client: &impl HttpFetcher,
) -> Result<Project, QueryError> {
    // Setup here
    let response = http_client.http_post_request(/* params */)
   // Processing response here
}

Now I'm wondering if this is indeed idiomatic Rust code or if there is a better way?

2

Is there any particular reason query_data isn't a method on, say, HttpFetcher? The "dependency" then becomes the struct itself.

Edit: "the struct itself" here referring to concrete implementers of HttpFetcher, ie Client or MockClient

3

In my opinion, it is valuable that your testing strategy

  • Is realistic: Tests present the code under test with conditions that accurately reflect those it will encounter in non-test environments.
  • Does not add complexity to the code under test that does not serve its non-test needs (when possible).

Therefore, you should not add a trait between your client code and reqwest, because it is not needed outside of tests and it is easy to implement it unrealistically. Instead, your tests should create HTTP servers which pretend to be the normal API server. This way, your dependency-injection for testing consists of giving a different root URL to the client — not substituting any code.

3 Likes
4

query_data also contains setup for the HTTP request (i.e. preparing the JSON for the POST) and processing the result (handling pagination of the API, deserializing response). I put the entire reqwest functionality within http_post_request() for abstraction and modularity reasons.

5

This is something I would consider at the integration test level, but IMO for the unit test level it is a little too much. While I also think that you shouldn't have to bend your code too much for your tests, a bit of change for mocking a external dependency is alright in my books.

6

Separate from the setup you do in query_data_impl? To be clearer, I'm wondering why it is:

pub fn query_data(options: &FetchOptions) -> Result<Project, QueryError> {
    let http_client = Client::new();
    query_data_impl(options, &http_client)
}

fn query_data_impl(
    options: &FetchOptions,
    http_client: &impl HttpFetcher,
) -> Result<Project, QueryError> {
    // Setup here
    let response = http_client.http_post_request(/* params */)
   // Processing response here
}

Instead of, perhaps:

#[cfg_attr(test, automock)]
pub(super) trait HttpFetcher {
    /// Send a HTTP POST request with JSON as payload.
    fn http_post_request(
        &self,
        url: &str,
        payload: serde_json::Value,
        options: &FetchOptions,
    ) -> Result<String, NetworkError>;

    // Default implementation, less boilerplate.
    fn query_data(&self, options: &FetchOptions) -> Result<Project, QueryError> {
        query_data_impl(options, &self)
    }
}

fn query_data_impl(
    options: &FetchOptions,
    http_client: &impl HttpFetcher,
) -> Result<Project, QueryError> {
    // Setup here
    let response = http_client.http_post_request(/* params */)
   // Processing response here
}

I'm not personally a fan of having the two separate functions to begin with, but if I were going to do something like this, I'd think the second approach would be more pleasant.

7

If you only need to have one implementation in unit tests and another in production and integration tests, don't make things more complicated than they need to be: the simplest mechanism in Rust to swap out code is #[cfg(test)], for example:

#[cfg(not(test))]
mod api;

#[cfg(test)]
#[path = "api_mock.rs"]
mod api;

use api::ApiClient;

// ...

And have api.rs and api_mock.rs implement a common interface (can be just type and method names, no traits necessary) for your code under test, and any extra mock configuration can be directly exposed without issue.

Or you can use it directly in the implementation itself, but it gets a bit more messy there.

1 Like