Initial unit tests for Trustee interaction

[Jakob-Naucke]

Respinning #34. Attempts to supersede #39 + #20.

e: contains some minimally-invasive refactorings for better testability in separate commits

[alicefr]

nit: can you create a macro for the cases we don't use any input, it will be a bit easier to read the code:

macro_rules! return_config_map {
    (ok) => {
        |_: &_| Ok(ConfigMap::default())
    };
    (err, $err:expr) => {
        |_: &_| Err::<ConfigMap, _>($err)
    };
}

let clos_ok = return_config_map!(ok);
let clos_err = return_config_map!(err, StatusCode::CONFLICT);

[Jakob-Naucke]

had one but dropped it again. checking URIs & methods now, and with that there would be one single use for the Ok case and zero for the Err case.

[alicefr]

@Jakob-Naucke one question, you are implementing a closure which return a single object for all the type of requests. I'm wondering if we should distinguish the type of request. IOW, I might want to use the mocking client for Get, Create and Patch. Maybe for now we can leave it as it is since the patches are already quite complex.

Another functionalities we might require in the future is the ability to specify how many times you want to return that object. For example, in a retry loop we might just want to return it only once and then changing behavior. It can be useful for example to test the recovery from an error, you return first and error and then the correct object.

[alicefr]

it already looks very promising, thanks! I will do a second pass later

[Jakob-Naucke]

one question, you are implementing a closure which return a single object for all the type of requests. I'm wondering if we should distinguish the type of request. IOW, I might want to use the mocking client for Get, Create and Patch. Maybe for now we can leave it as it is since the patches are already quite complex.

hmm, already with this patch you could match on the method in the same way that e.g. test_update_rvs matches on the URI. is that not what you mean?

Another functionalities we might require in the future is the ability to specify how many times you want to return that object. For example, in a retry loop we might just want to return it only once and then changing behavior. It can be useful for example to test the recovery from an error, you return first and error and then the correct object.

ayy, state. I suppose the response_closure could also take an attempt_number: u8 (maybe in a future PR) or there's something more elegant here.

[alicefr]

one question, you are implementing a closure which return a single object for all the type of requests. I'm wondering if we should distinguish the type of request. IOW, I might want to use the mocking client for Get, Create and Patch. Maybe for now we can leave it as it is since the patches are already quite complex.

hmm, already with this patch you could match on the method in the same way that e.g. test_update_rvs matches on the URI. is that not what you mean?

yes, but you aren't checking that. It could be any method called and you return that with the mock.
What I meant was more close as the go mock that they always have a mock call per methods in the interface.
What if for example I change the update_reference_values with a different set of client calls e.g create, get and patch (even if the example doesn't make sense)

Another functionalities we might require in the future is the ability to specify how many times you want to return that object. For example, in a retry loop we might just want to return it only once and then changing behavior. It can be useful for example to test the recovery from an error, you return first and error and then the correct object.

ayy, state. I suppose the response_closure could also take an attempt_number: u8 (maybe in a future PR) or there's something more elegant here.

Yes, the comment wasn't for now

[Jakob-Naucke]

one question, you are implementing a closure which return a single object for all the type of requests. I'm wondering if we should distinguish the type of request. IOW, I might want to use the mocking client for Get, Create and Patch. Maybe for now we can leave it as it is since the patches are already quite complex.

hmm, already with this patch you could match on the method in the same way that e.g. test_update_rvs matches on the URI. is that not what you mean?

yes, but you aren't checking that. It could be any method called and you return that with the mock. What I meant was more close as the go mock that they always have a mock call per methods in the interface. What if for example I change the update_reference_values with a different set of client calls e.g create, get and patch (even if the example doesn't make sense)

Sorry, I still don't understand what you're going for. You could do e.g.

let clos = move |req: &Option<Request<_>>| match req {
    Some(r) if r.method() == Method::PATCH => Ok(some_config_map.clone()),
…

what is it instead that you want to be able to do?

[alicefr]

one question, you are implementing a closure which return a single object for all the type of requests. I'm wondering if we should distinguish the type of request. IOW, I might want to use the mocking client for Get, Create and Patch. Maybe for now we can leave it as it is since the patches are already quite complex.

hmm, already with this patch you could match on the method in the same way that e.g. test_update_rvs matches on the URI. is that not what you mean?

yes, but you aren't checking that. It could be any method called and you return that with the mock. What I meant was more close as the go mock that they always have a mock call per methods in the interface. What if for example I change the update_reference_values with a different set of client calls e.g create, get and patch (even if the example doesn't make sense)

Sorry, I still don't understand what you're going for. You could do e.g.

let clos = move |req: &Option<Request<_>>| match req {
    Some(r) if r.method() == Method::PATCH => Ok(some_config_map.clone()),
…

what is it instead that you want to be able to do?

As discussed offline, this is exactly what I want. The issue was that the test didn't include the specification of the method we were testing against

[alicefr]

Very nice, thanks Jakob for the work! Looks great!
/lgtm