Rusoto: Provide mocking implementation of DispatchSignedRequest

[jonhoo]

For the purposes of testing, it'd be really handy to easily be able to set up a DispatchSignedRequest that you could pass to, say Ec2Client::new, and where you could dictate what responses should be returned. Currently, users would need to implement DispatchSignedRequest on their own mock type, and deal with manually constructed HttpResponses, but at least in theory it should be able to provide an API that gives higher-level response types.

[srijs]

Hi @jonhoo!

Have you had a chance to check whether the MockRequestDispatcher from the rusoto_mock crate does what you need?

[jonhoo]

Oh, I had not seen that! That's a start, but it still requires me to operate on raw requests/responses, which is not really what I want for testing something that uses rusoto. Instead, I'd want something that lets me do, say:

use rusoto_ec2::*;
let mut mock = MockDispatcher::new();
let ec2 = Ec2Client::new(mock, provider);
// auth stuff

let req = DescribeInstancesRequest { /* .. */ };
let mock_res = DescribeInstancesResult { /* .. */ };
mock.respond(&mock_res);
let res = ec2.describe_instances(&req).unwrap();
assert_eq!(res, mock_res);

In reality, I probably wouldn't assert at all on res— all I really want is a way to say "make it look like the AWS API returned the following response". That way I can write tests that exercise my code in response to things that the AWS API responds, without having to understand the on-the-wire protocol for the API. I can just operate with the requests I normally generate, and the responses I expect to see. This would allow writing tests for my own library like so:

fn test() {
    let mut mock = MockDispatcher::new();
    let mut x = mylib::Foo::with_dispatcher(mock);

    // set up sequence of responses for the requests I know the library should make
    // there may have to be something like an enum of all possible requests, and one
    // of all possible responses, so that this will type-check. those enums will both
    // implement `Into<Vec<u8>>` or something.
    mock.expect(RequestSpotInstancesResult { /* ... */ });
    mock.respond(RequestSpotInstancesResult { /* ... */ });
    // checking requests should be optional
    mock.respond(DescribeSpotInstanceRequestsResult { /* ... */ });
    // perhaps expect takes a closure so that I can check just some request fields
    mock.expect(|req| {
        match req {
            Requests::DescribeInstancesResult { ref instance_ids, .. } => {
                assert_eq!(instance_ids, vec![/* .. */]);
            }
            _ => panic!("unexpected request: {:?}", req),
        }
    });
    mock.respond(DescribeInstancesResult { /* ... */ });
    // etc.

    // now that we've set up the expected interactions with AWS,
    // test that the library indeed does the right thing
    x.run().unwrap();
}

[srijs]

Yeah, that makes sense!

I have to ask though, do you feel like that would be mocking the dependency at the right level? To me, it feels like if you wanted to stub out a return value like RequestSpotInstancesResult, maybe that would be more convenient to do at the service level rather than dispatcher level?

As you may know, rusoto exposes the EC2 functionality in two components: An Ec2 trait, and an Ec2Client struct which implements this trait. Mocking out the EC2 interactions by providing a different implementation of Ec2 seems like it might be more appropriate. Maybe rusoto_mock could include some way to easily create these "mock" structs.

Or am I missing something and this wouldn't work in your case for some reason?

[jonhoo]

Hmm, no, you might be right that it's better to mock this at the Ec2 trait level. I always found that extra trait with only one implementor somewhat awkward, but I suppose it helps in this case.

The one argument I can see for why you might want mocking at the dispatcher level is that a decent number of crates are likely to use multiple clients (e.g., both STS and EC2). It makes sense for a library to allow its users to choose the dispatch (default_tls for example) and the provider by providing them to a constructor. It makes somewhat less sense to have the library take an impl Ec2 and an impl Sts and whatever else. Sharing the dispatcher also makes it easier to mock such higher-level libraries that use many rusoto clients, since you can interact with just one mocker, rather than mock the APIs individually.

Not sure what the right thing to do is though.

[matthewkmayer]

Thanks for the clear use case for wanting the mocking ability!

The service traits were introduced specifically for mocking, but that doesn't cover the use case in your most recent comment without a bunch of boilerplate.

I'm also not sure what the right path forwards is. Feedback like this certainly helps us determine how we can make things better, though! 😄

[mjkillough]

Maybe rusoto_mock could include some way to easily create these "mock" structs.

That would be great!

A lot of the traits provide a lot of methods, and it's typically only necessary to mock a few of them in a single test. A great example of this is the S3Mock type defined in the doc comment of rusoto_core::Future.

I have an application which is doing a few very simple S3 operations (list_objects_v2/get_object) in order to load some parameters. I've implemented this behind a custom trait, so that I can provide a implementation that doesn't use S3 for most tests and local development. However, it would be nice if there was a way to easily unit test the implementation that does use rusoto_s3::S3. Currently this would require me to implement the entirety of the rusoto_s3::S3 trait, just to provide the two methods I actually want to test.

I think it would be possible to generate an S3Mock struct automatically which allows usage like:

let mock = S3Mock {
    get_object: Box::new(|req| {
        let resp = match req.key.as_ref() {
            "object-1" => Ok(GetObjectOutput {
                body: Some("object-1 body".as_bytes().to_vec().into()),
                ..Default::default()
            }),
            _ => Err(GetObjectError::NoSuchKey(req.key.clone())),
        };
        RusotoFuture::from(resp)
    }),

    ..Default::default()
};

I haven't tried modifying the codegen to create one of these S3Client structs, but I have played around with creating one manually:

// Subset of the actual rusoto_s3::S3 trait, to make it easier to experiment with.
trait S3 {
    fn get_object(&self, input: GetObjectRequest) -> RusotoFuture<GetObjectOutput, GetObjectError>;

    fn list_objects(
        &self,
        input: ListObjectsRequest,
    ) -> RusotoFuture<ListObjectsOutput, ListObjectsError>;

    fn list_objects_v2(
        &self,
        input: ListObjectsV2Request,
    ) -> RusotoFuture<ListObjectsV2Output, ListObjectsV2Error>;

    // ...
}

// Generated automatically
pub struct S3Mock {
    pub get_object: Box<dyn Fn(GetObjectRequest) -> RusotoFuture<GetObjectOutput, GetObjectError>>,
    pub list_objects:
        Box<dyn Fn(ListObjectsRequest) -> RusotoFuture<ListObjectsOutput, ListObjectsError>>,
    pub list_objects_v2:
        Box<dyn Fn(ListObjectsV2Request) -> RusotoFuture<ListObjectsV2Output, ListObjectsV2Error>>,
    // ...
}

// Generated automatically
impl Default for S3Mock {
    fn default() -> Self {
        Self {
            get_object: Box::new(|_| unimplemented!("S3Mock::get_object")),
            list_objects: Box::new(|_| unimplemented!("S3Mock::list_objects")),
            list_objects_v2: Box::new(|_| unimplemented!("S3Mock::list_objects_v2")),
            // ...
        }
    }
}

// Generated automatically
impl S3 for S3Mock {
    fn get_object(&self, input: GetObjectRequest) -> RusotoFuture<GetObjectOutput, GetObjectError> {
        (self.get_object)(input)
    }

    fn list_objects(
        &self,
        input: ListObjectsRequest,
    ) -> RusotoFuture<ListObjectsOutput, ListObjectsError> {
        (self.list_objects)(input)
    }

    fn list_objects_v2(
        &self,
        input: ListObjectsV2Request,
    ) -> RusotoFuture<ListObjectsV2Output, ListObjectsV2Error> {
        (self.list_objects_v2)(input)
    }

    // ...
}

Was something like this what you had in mind? I'd be happy to try prototyping a working version if it was something you'd be interested in exploring.

As each mock would need to be generated at the same time as the trait, I think it'd need to be part of the crate implementing the service, rather than the rusoto_mock crate, but it could easily be placed behind a feature flag.