4

I'm currently developing some low level drivers for an embedded platform in plain C. I use unity+cmock as a unit testing framework.

However while writing the low level stuff I often come across the following pattern:

Test:

void test_mcp2515_read_register(void)
{
    spi_frame_t expected_frame = {{0}};
    expected_frame.tx_length = 2;
    expected_frame.rx_length = 3;
    expected_frame.tx_data[0] = MCP2515_READ_CMD;
    expected_frame.tx_data[1] = TEST_ADDR;
    expected_frame.callback = callback_test;

    spi_transmit_ExpectAndReturn(expected_frame, true);

    mcp2515_read_register(TEST_ADDR, callback_test);
}

Implementation:

void mcp2515_read_register(uint8_t addr, spi_callback callback)
{
    spi_frame_t frame = {{0}};
    frame.tx_length = 2;
    frame.rx_length = 3;
    frame.tx_data[0] = MCP2515_READ_CMD;
    frame.tx_data[1] = addr;
    frame.callback = callback;

    spi_transmit(frame);
}

As you can see there is a lot of duplication between the code between the test and the implementation.

Is this a problem? Am I writing my tests wrong? Or should I not bother writing tests at all for this low level stuff?

Lundin
  • 195,001
  • 40
  • 254
  • 396
Willem Melching
  • 41
  • 4

1 Answers1

2

The efficiency of a test code doesn't usually matter. It depends on what you are trying to test, but duplicate code can indicate a design flaw.

In your case, you could perhaps have divided the mcp2515_read_register function in two parts: one which creates a struct and another which handles the SPI transmission.

The optimal OO program design would probably involve the following modules:

  • SPI driver only concerned with the actual communication.
  • A CAN controller driver only concerned with the specifics of the controller.
  • A caller ("main" or whatever).
  • Test code for the CAN controller driver: replaces main.

The SPI driver declares spi_frame_t as an opaque type, a struct which is only concerned with SPI data and communication. None outside the SPI driver knows or needs to know the contents of this struct. I don't know what the callback function does, but it doesn't look like something related to the SPI driver. It rather looks like something related to the code that calls the SPI driver.

The CAN controller driver includes the SPI driver. It calls a "constructor" from the SPI driver to create a frame, then passes the frame on to the SPI communication routines. The CAN controller driver then has no tight coupling to SPI functionality. For example, it doesn't make sense to rewrite your CAN controller code because you found a bug in the SPI. Nor should you need a CAN controller to be able to use SPI, should you want to re-use the SPI driver in another project.

The test case either replaces the caller ("main") or the CAN controller code, depending on what part of the program you are trying to test. It uses the very same functions as the production code.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • That is almost exactly how the application is build up. The callback function is necessary for when the (async) SPI transfer is done. The spi_frame_t is thus independent of the SPI implementation and is only used to indicate what data has to be transferred. My original question is still not answered. How do I keep myself from duplicating code between test and implementation. If my implementation is almost a 90% copy/paste from my test, what use does the test have. – Willem Melching Nov 20 '14 at 13:57
  • @WillemMelching If the SPI frame has nothing to do with SPI, why did you name it with the SPI prefix? Either you gave the type a confusing name, or your CAN controller code is doing things it shouldn't concern itself with. – Lundin Nov 20 '14 at 14:39
  • @WillemMelching Anyway, it doesn't really matter where the SPI frame belongs, because you can still do the implementation with opaque type and constructor. That way the frame creation is separate from the transmission, and your test code can call the frame constructor just as the production code function. Your application does not do this, you are using plain "procedural programming structs", where your application code is tightly coupled to the struct data type. And that is why you have to repeat the code in the test case. – Lundin Nov 20 '14 at 14:39
  • If you're copying and pasting from your test to your code, you're doing it wrong. Obviously the test will pass in that situation. I suggest reading up on some unit testing methods, see http://xunitpatterns.com/Behavior Verification.html and possibly http://martinfowler.com/articles/mocksArentStubs.html – Ross Nov 20 '14 at 22:32
  • 1
    @Ross Again, it depends on what he wants to test. You can't just say copy/paste is wrong, with no context. – Lundin Nov 21 '14 at 07:13