Unit testing tightly coupled structures

Hi folks,

I try to write some unit tests for my program. I have a Decoder struct I want to test, which reads a socket and implements a protocol. One of its fields is MainExecutor, which Decoder uses to do stuff according to what Decoder::decode() reads (i.e. when a command 'turn on the light' comes, executor.turn_on_light() is invoked). How can I mock MainExecutor, if it is a field in Decoder? Or shall I use mpsc to pass the commands (perhaps enum variants) from Decoder to the Executor, in order to decouple them? Or can perhaps a crate like mockall be useful in this situation?

A common myth is that “unit testing” needs to test the unit in isolation from absolutely everything else, even all of its dependencies. This is not a wise approach, because trying to mock 100% of dependencies leads to tests which are not only effortful to write, but which may expose the unit to behaviors of the mocked dependencies which do not match the behavior of the real dependencies, so your unit tests may fail to provide genuine test coverage for the real versions of the situations you intended to test.

So, the question is, is there some other reason to replace MainExecutor with a mock? Does MainExecutor have side effects that are inappropriate in a test? If so, then that is a good reason to replace it. On the other hand, if you can create a MainExecutor which is configured so that turn_on_light() only affects things within the test, then do that instead of using a mock.

This is potentially a very good strategy, if a channel is also suitable for how the code should work outside of testing.

Another option you should consider, especially for protocol implementations, is the “sans-io” architecture: don’t make the decoder permanently contain/own the place where it sends decoded commands. Instead, each time incoming data is available, call decoder.decode(data, callback), where the callback receives commands (in the same format you would put the commands on a channel). This makes the decoder maximally flexible in how it interacts with the outside world — the caller gets to control every aspect. This is particularly useful in async or no_std applications. However, it often forces you to write your decoder as an explicit state machine, which may be less convenient, and if using a channel is an adequate solution for you, you can just do that.

6 Likes

This seems like a wrong question to ask ...

If I understand correctly, there are two possible cases to consider:

  1. The purpose of Decoder is to read, decode and execute commands from input.
  2. The purpose of Decoder is to read and decode commands from input and send then to the caller (by returning from function, by callback, by mpsc, ... right now it is not important how exactly)

It seems to me you implemented #1 and want to test #2. Quite a mismatch. So as the first step, you should make conscious decision and stick to it.

  • If you choose #1, there should be no need to mock the receiver of those commands (MainExecutor) since it is internal part of Decoder invisible to tests. (Despite formally being a dependency -- would you want to mock the sqrt function (provided by external math library) when testing the quadratic formula?)
  • If you choose #2, there should be no need to mock the receiver of those commands since you are the receiver.
  • If you intended #2, but somehow ended up with #1 (happens to the best of us ...) you shouldn't ask "How do I mock ...?" but "How do I do #2 properly?"

On the last one, @kpreid already gave you some ideas (mpsc, callback). Another one may be to describe MainExecutor as trait, make Decoder (or its decode method only) generic over it and make sure it is supplied by caller. That's also essentially a kind of callback.

This may look like mocking the MainExecutor, but it is not. The driving question here was "How/whether do I send decoded commands back to caller?", not "How do I, for testing purposes only, substitute some internal part that happens to be a dependency (and I happen to know about it)?"

1 Like

I'm currently working on the channel approach, as it seems to fit quite well. I just discovered that in fact I shall get some data back from the executor in some cases, and send that data back via the Decoder, but I think that won't pose additional problems.

I'm generally new to unit testing, but I don't think I'm falling into this trap. Executor is working with I/O (like serial port), so my idea was to make sure Decoder sends it the right commands.

To me, serial port io isn't appropriate. I might test it with a serial loopback and a separate test program maybe, but I'll leave it for now.

When I started this project 2 weeks ago I was thinking about something similar, but I didn't knew how to do it (faster than the way I did the solution I have now, which works). Is sansio - Rust a good place to start?

The execution is done by the Executor, which is a separate struct, so no.

Yes, in the decoder() I used to call executor.turn_on_light() , now I do channel_to_executor.send(ExecutorCommand::Initialize).await

I did that, but then I lost the direction where shall I go next. I need to retry it.

Many thanks for the quick, detailed and informative answers!

Never heard of it. Sans-io is a design pattern and I only meant to recommend the pattern.

1 Like

This focuses on Python network libraries, but it's where I first heard about the concept and it has a good explanation in depth in the inner page links:

1 Like

I am puzzled how to do the Response enum, because different operations give different response types. (For example, executor.turn_on_light() gives Result<()> (more precisely, anyhow::Result<()>) , executor.read_something()-> Result<String> etc.) If I have two structures, one for Rx, and one for Tx,

enum ExecutorRx {
   TurnOnLight,
   SendValueX{device_num: usize},
   SendValueY{device_num: usize},
}

enum ExecutorTx {
   SendValueXResponse{Result<String>}
   SendValueYResponse{Result<i32>}
}

it will take a lot of boilerplate to wire all the variants, and it will be error-prone because there won't be mapping between the Rx command variant and the corresponding Tx response. The other possibility is to have one single enum, where every field represents the executor request, and the response fields will be an enum variant value, like this:

pub enum ExecutorCommandResponse {
    Initialize,
    SendValueX(SendValueX),
}

pub struct SendValueX {
    device_num: usize, // written before sending to the Executor
    result: Option<Result<String>>, // executor result, sent back to the caller
}

This doesn't look good either.

Here something I've learnt today while searching how to know which Executor response is a result from which request.

use tokio::sync::{mpsc, oneshot};

struct Request {
    data: String,
    respond_to: oneshot::Sender<Response>,
}

struct Response {
    result: String,
}

struct Worker {
    receiver: mpsc::Receiver<Request>,
}

impl Worker { 
    async fn run(mut self) {
        while let Some(req) = self.receiver.recv().await {
            let result = format!("Processed: {}", req.data);

            let _ = req.respond_to.send(Response { result });
        }
    }
}

struct Client {
    sender: mpsc::Sender<Request>,
}

impl Client {
    async fn send_request(&self, data: String) -> String {
        let (tx, rx) = oneshot::channel();

        let request = Request {
            data,
            respond_to: tx,
        };

        self.sender.send(request).await.unwrap();

        let response = rx.await.unwrap();
        response.result
    }
}

#[tokio::main]
async fn main() {
    let (tx, rx) = mpsc::channel(100);

    let worker = Worker { receiver: rx };
    tokio::spawn(worker.run());

    let client = Client { sender: tx };

    let result = client.send_request("hello".into()).await;
    println!("{}", result);
}

I guess my question now is "how to do function calls via channel messages with less boilerplate".

What part of the system cares about the response? The Decoder that we have been discussing should not; it should report what it has decoded and not know what happens as a result of that. Something else can receive the messages from the Decoder and execute the actions, whether that involves oneshot channels or just awaiting the completion.

There was small refactoring, here are the actual situation with naming which should make more sense (better naming suggestions are welcome).
I have a Server struct (previously called Decoder, it's what I want to test) containing the TCP socket address, serve() method with an infinite loop inside waiting for connection, and a second nested loop, parsing the stream.
There is a Datagram structure, which represents the datagram (packet) sent over TCP. It is instantiated twice in serve(), for both tx and rx. In serve() I read in rx (using Datagrams clear_and_receive(&mut stream) function, which does the TCP stream read), read the command and invoke the corresponding executor method (MainExecutor), read the executor response (for example, Ok(), Ok(message: String) or Ok(meters: u32)) and put that data in tx I prepare (Datagram has convenient methods for pushing consistently responses). (Each datagram has one or more commands I shall iterate trough, execute and give response.)