This repository has been archived by the owner on Aug 30, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 28
PB-570: add the first coordinator::core::Service test #352
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 27, 2020
0ae47be
to
8474d94
Compare
8474d94
to
31ddc22
Compare
The main tasks, _ie_ that tasks that implement most of the business logic are `aggregator::Service` for the aggregator and `coordinator::Service` for the coordinator. This commit implements a first tests for `coordinator::core::Service`. In order to test the service, we need to control its inputs and outputs There are three source: - an RPC server that receives requests from the network and sends back responses - an RPC client that sends requests to the network and receives responses back - an REST API that receives requests from the network, and sends back responses However, two of these are not _direct_ sources of the `Service`. The API and RPC server layers are already isolated and run in their own task. They communicate with the `Service` via a `Handle` as illustrated below: ``` +-------------+ | API | | +------+ | +---------------------------+ +----->Handle| <---+ | Service | | | +------+ | | | +----------+ +--------+ | | +-------------+ | +--->RPC client| |Receiver<--------+ +-------------+ | | | +----------+ +--------+ | | | RPC server | | | +---------------------------+ | | +------+ | | | +----->Handle| <---+ | | +------+ | | | +-------------+ | | | | +---------+ | | | | | +-------------------> Network <----------------------------+ | | +---------+ ``` We already control these handles and we can use them to send requests to the service. Thus, there is no need to mock the API and RPC server and our system under test can already be simplified: ``` +------+ +---------------------------+ +----->Handle| | Service | | +------+ | +----------+ +--------+ | | +--->RPC client| |Receiver<--------+ | | +----------+ +--------+ | | | +---------------------------+ | +------+ | +----->Handle| | +------+ | | | +---------+ | | | +-------------------> Network | | | +---------+ ``` The RPC client is the only remaining piece, and the idea of the PR is to mock it away so that our system under test is finally fully under our control: ``` +------+ +---------------------------+ +----->Handle| | Service | | +------+ | +----------+ +--------+ | | | | Mock | |Receiver<--------+ | +----------+ +--------+ | | +---------------------------+ | +------+ +----->Handle| +------+ ``` For the mocking itself, we use the `mockall` crate. The idea is to leverage rust's conditional compilation to replace the `rpc::Client` by our mock when running `cargo test`. For this, we use the `#[cfg(test)]` and `#[cfg(not(test))]` attributes. The mock itself doesn't expose the exact same API than the `rpc::Client`: the types in some of the function signatures are slightly simpler. That's works well in Rust because most APIs are designed around traits. For instance the real `rpc::Client::select` method looks like this: ``` pub fn select( &mut self, ctx: Context, credentials: Credentials ) -> impl Future<Output = Result<Result<(), ()>>> + '_ ``` Thus, the return type is _some type that implements `Future<Output=io::Result<Result<(), ()>>>`, and an anonymous lifetime (the `'_` part). But in our mock, the return type is an actual concrete type: ``` fn select(&mut self, ctx: Context, credentials: Credentials) -> future::Ready<io::Result<Result<(), ()>>>; ``` It works, because that concrete type satisfies the `Future<Output = Result<Result<(), ()>>> + '_` trait bound. In the coordinator binary, we use a random selector. In order to keep full control of the selection, we defined a very simple selector `MaxSelector` that selects all the waiting clients. For more sophisticated tests, we could define other `Selector` implementors. These tests are require quite a lot of boilerplate code, so it would be nice to keep them out of `src/` in their own `tests/` directory. Unfortunately, this doesn't work well with the RPC client mock, which must be compiled with the rest of the crate. Therefore we created a `tests/` directory inside `src/`. `tests/lib/` contains code that can easily be re-used as well as mocks. When compiling the tests, some module may `use` symbols from `tests::lib`. For instance in `aggregator/rpc.rs` we have: ```rust use crate::tests::lib::rpc::aggregator::Client; ```
31ddc22
to
5441e47
Compare
Robert-Steiner
approved these changes
Mar 30, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main tasks, ie that tasks that implement most of the business
logic are
aggregator::Service
for the aggregator andcoordinator::Service
for the coordinator. This commit implements afirst tests for
coordinator::core::Service
.In order to test the service, we need to control its inputs and
outputs
There are three source:
However, two of these are not direct sources of the
Service
. TheAPI and RPC server layers are already isolated and run in their own
task. They communicate with the
Service
via aHandle
asillustrated below:
We already control these handles and we can use them to send requests
to the service. Thus, there is no need to mock the API and RPC server
and our system under test can already be simplified:
The RPC client is the only remaining piece, and the idea of the PR is
to mock it away so that our system under test is finally fully under
our control:
For the mocking itself, we use the
mockall
crate. The idea is toleverage rust's conditional compilation to replace the
rpc::Client
by our mock when running
cargo test
. For this, we use the#[cfg(test)]
and#[cfg(not(test))]
attributes.The mock itself doesn't expose the exact same API than the
rpc::Client
: the types in some of the function signatures areslightly simpler. That's works well in Rust because most APIs are
designed around traits.
For instance the real
rpc::Client::select
method looks like this:Thus, the return type is _some type that implements
Future<Output=io::Result<Result<(), ()>>>
, and an anonymouslifetime (the
'_
part). But in our mock, the return type is anactual concrete type:
It works, because that concrete type satisfies the
Future<Output = Result<Result<(), ()>>> + '_
trait bound.
In the coordinator binary, we use a random selector. In order to keep
full control of the selection, we defined a very simple selector
MaxSelector
that selects all the waiting clients. For moresophisticated tests, we could define other
Selector
implementors.These tests are require quite a lot of boilerplate code, so it would
be nice to keep them out of
src/
in their owntests/
directory. Unfortunately, this doesn't work well with the RPC client
mock, which must be compiled with the rest of the crate.
Therefore we created a
tests/
directory insidesrc/
.tests/lib/
contains code that can easily be re-used as well as mocks. When
compiling the tests, some module may
use
symbols fromtests::lib
. For instance inaggregator/rpc.rs
we have: