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

Create a mock Client for testing #429

Closed
clux opened this issue Feb 21, 2021 · 10 comments
Closed

Create a mock Client for testing #429

clux opened this issue Feb 21, 2021 · 10 comments
Labels
automation ci and testing related client kube Client related help wanted Not immediately prioritised, please help!

Comments

@clux
Copy link
Member

clux commented Feb 21, 2021

Create a mockable Client that allows us to perform fake Api calls, intercept the output, and send fake responses to it.

Why?:

  • Allows us (maintainers) to unit test kube::Api and kube_runtime
  • Allows us (maintainers) to unit test Api -> Service interactions (like the proposed default_namespaced in Api::namespaced should work with default namespace #209)
  • Allows users to unit test their abstractions on top of kube::Api and kube_runtime by faking a few responses

Asked for in #426.

This is something that is theoretically possible through something like tower_test, but it brings with it complications:

  • Client would need to take two types of Service's (one of which intercepts calls and allows shoehorning in responses)
  • Multiple Client types => generic parameter required in Api (since it houses the Client) - discussed in sans-io Client #406
  • The latter is a huge breaking change (though not necessarily against it unless it can be avoided)
  • Actually shoehorning in responses to a MockService seems hairy based on tower_test::Mock

This is a more important (and concrete) want than what is nebulously outlined in #406 - so we probably close that in favour of this testing issue.


UPDATE: Client takes Service<http::Request<hyper::Body>> (#532) so this is easier now.

@clux clux added client kube Client related automation ci and testing related labels Feb 21, 2021
@clux
Copy link
Member Author

clux commented Feb 21, 2021

One point on the potential breaking change, if we are to do it, we should try our best to ensure the ergonomics of it stay as close to "specify the service generic once, have it be inferred everywhere else" when modifying Api.

@clux clux mentioned this issue Feb 21, 2021
@kazk
Copy link
Member

kazk commented Feb 21, 2021

UPDATE: kube::Service was removed (#532) and now Client can use Service<http::Request<hyper::Body>>

kube::Service was added to avoid adding a type parameter to Client. It should be possible to create a mock service (its new takes any tower Service as long as request and response type matches) and create a Client with it.

Maybe I missed something? I’m on my phone right now, so I’ll look into it later.

@clux
Copy link
Member Author

clux commented Feb 21, 2021

If it's possible to use kube::Service directly, then that would be great!
I only did some small experiments that led me to believe that we would have to add a lot to kube::Service to be able to support both a mockable mode and a normal mode, but maybe I am wrong.

@kazk
Copy link
Member

kazk commented Feb 22, 2021

UPDATE: kube::Service was removed and Client takes Service<http::Request<hyper::Body>> (#532) so this is easier now. The example below was updated.

The following works (tested 6ae744d):

#[cfg(test)]
mod tests {
    use kube::{Api, Client};

    use futures::pin_mut;
    use http::{Request, Response};
    use hyper::Body;
    use k8s_openapi::api::core::v1::Pod;
    use tower_test::mock;

    #[tokio::test]
    async fn test_mock() {
        let (mock_service, handle) = mock::pair::<Request<Body>, Response<Body>>();
        let spawned = tokio::spawn(async move {
            // Receive a request for pod and respond with some data
            pin_mut!(handle);
            let (request, send) = handle.next_request().await.expect("service not called");
            assert_eq!(request.method(), http::Method::GET);
            assert_eq!(request.uri().to_string(), "/api/v1/namespaces/default/pods/test");
            let pod: Pod = serde_json::from_value(serde_json::json!({
                "apiVersion": "v1",
                "kind": "Pod",
                "metadata": {
                    "name": "test",
                    "annotations": { "kube-rs": "test" },
                },
                "spec": {
                    "containers": [{ "name": "test", "image": "test-image" }],
                }
            }))
            .unwrap();
            send.send_response(
                Response::builder()
                    .body(Body::from(serde_json::to_vec(&pod).unwrap()))
                    .unwrap(),
            );
        });

        let pods: Api<Pod> = Api::namespaced(Client::new(mock_service), "default");
        let pod = pods.get("test").await.unwrap();
        assert_eq!(pod.metadata.annotations.unwrap().get("kube-rs").unwrap(), "test");
        spawned.await.unwrap();
    }
}

Another example provided by user (remove kube::Service wrapper to use with kube >= 0.55.0): #426 (reply in thread)
See also #429 (comment)

I'd like to provide something more convenient, though.

We can also decorate the mock service as needed similar to the default service.

kazk added a commit to kazk/kube-rs that referenced this issue Feb 22, 2021
kazk added a commit to kazk/kube-rs that referenced this issue Feb 22, 2021
@clux
Copy link
Member Author

clux commented Feb 22, 2021

Ah, that is not bad for out of the box!

I assumed we had to build something on top of our Service, but I had approached it the annoying way of trying to get all our service layers into the mock, which probably isn't that useful for end-users, but somewhat useful for us when unit testing. Not sure if that's worth pursuing. We can test auth at a unit test level within service at the very least - and that's the layer that probably is most test-worthy.

@kazk
Copy link
Member

kazk commented Feb 22, 2021

Yeah, I'd test layers separately like I did for AuthLayer.

I had approached it the annoying way of trying to get all our service layers into the mock, which probably isn't that useful for end-users, but somewhat useful for us when unit testing.

If you want, you should be able to define a function that returns the default stack, then apply that layer, something like mock_service.layer(default_stack()) (might need to use ServiceBuilder).

Similar to how we have common stack:

https://github.com/clux/kube-rs/blob/72ae086e9fdb7f56f30a2229ca1cc5ff12e3b897/kube/src/service/mod.rs#L95-L98

that's applied with layer
https://github.com/clux/kube-rs/blob/72ae086e9fdb7f56f30a2229ca1cc5ff12e3b897/kube/src/service/mod.rs#L119-L123

@clux clux added the help wanted Not immediately prioritised, please help! label Feb 28, 2021
@kazk
Copy link
Member

kazk commented May 21, 2021

Using tower_test::mock::Handle (like an example provided by a user):

async fn mock_get_pod(handle: &mut Handle<Request<Body>, Response<Body>>) {
    let (request, send) = handle.next_request().await.expect("Service not called");

    let body = match (request.method().as_str(), request.uri().to_string().as_str()) {
        ("GET", "/api/v1/namespaces/default/pods/test") => {
            // Can also use recorded resources by reading from a file.
            // Or create entire mock from some file mapping routes to resources.
            let pod: Pod = serde_json::from_value(serde_json::json!({
                "apiVersion": "v1",
                "kind": "Pod",
                "metadata": {
                    "name": "test",
                    "annotations": { "kube-rs": "test" },
                },
                "spec": {
                    "containers": [{ "name": "test", "image": "test-image" }],
                }
            }))
            .unwrap();
            serde_json::to_vec(&pod).unwrap()
        }
        _ => panic!("Unexpected API request {:?}", request),
    };

    send.send_response(Response::builder().body(Body::from(body)).unwrap());
}

Used like:

#[tokio::test]
async fn test_mock_handle() {
    let (mock_service, mut handle) = mock::pair::<Request<Body>, Response<Body>>();
    let spawned = tokio::spawn(async move {
        mock_get_pod(&mut handle).await;
    });
    let pods: Api<Pod> = Api::default_namespaced(Client::new(mock_service));
    let pod = pods.get("test").await.unwrap();
    assert_eq!(pod.annotations().get("kube-rs").unwrap(), "test");
    spawned.await.unwrap();
}

Now with kube::Service removed, it's pretty straightforward. We can add some helpers and come up with conventions to make this more convenient, but it's usable already.

@kazk
Copy link
Member

kazk commented May 21, 2021

@clux I think we don't need anything else on Client now that it can use mock Service directly. I'll continue playing with tower_test and see if I find anything more convenient.
Maybe this can be closed, and new issues can be opened for adding test helpers (not sure what yet).

@clux
Copy link
Member Author

clux commented May 21, 2021

Yeah, I agree. The situation is pretty good now, and improvement is likely to come from helper functions.

Side note; It feels really hard to present a nice interface for testing controllers well. So many potential flows to keep track of, even the basic test case would be something like: 1. watchevent input -> 2. reconcile start -> 3. reconcile asks something of the api 4. api mock returns canned response, 5. reconcile decides on action, 6. we verify final request.

But yeah, let's close. The core issue (and many other things this issue was originally concerned with) are dealt with 👍

@clux clux closed this as completed May 21, 2021
@kazk
Copy link
Member

kazk commented May 22, 2021

Yeah, this will be useful for small unit tests, but I don't think it's realistic to test controller's complex behavior like this. It should be possible to make stateful mocks (like POST and then GET, or change response after some condition), but it'll be painful to manage for complex scenarios.

For that, I think we should add a test helper to start a cluster for each test (#382), and some test macros for convenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ci and testing related client kube Client related help wanted Not immediately prioritised, please help!
Projects
None yet
Development

No branches or pull requests

2 participants