7

This is a problem I have been trying to wrap my head around a couple months now. It has shown up again in a learning project I was working on last night, so I will use that as an example.

I am building an entity-component system, and have a Movement system that controls the movement of each entity on each turn.

class MovementSystem { private: vector<PositionComp> positions; vector<MoveAIComp> moveAIs; vector<vector<bool> > generateMap() { // Cycle through all positions and generates a 2D array // detailing whether each map space is occupied or not } void updateComps(vector<vector<bool> > map) { // For each entity generates a 3x3 array from map, centered // at that entity's position, passes it to // moveAI.getMove(surroundings), then updates the position } public: void update(Turn turn) { map = this->generateMap(); this->updateComps(map); } };

Now unit testing some of this functionality is fairly easy. I add a position and moveAI component to the system. The moveAI component is a mock object that returns a specific move that I specify in the test, I then call update on the sytem, and make sure that the position changed in the way is was supposed to.

The problem with this is I have skipped over the entire call to generateMap() and the first part of updateComps that creates the 3x3 array. Sure, I have tested that the position is properly updated based on what decision the moveAI makes, but I have not tested that the moveAI is getting the correct information to make that decision.

So far I have come up with a few ways to get around this:

  1. Change my mock MoveAIComponent to cache the inputs to getMove so I can check them in the test as well.
  2. My MovementSystem may be doing too much, so I should split it into two, a MapSystem that generates the map and updates a new SurroundingsComponent, then change MovementSystem to use the surroundings, position, and moveAI, allowing tests on MapSystem to cover that functionality.
  3. Leave testing of that functionality for integration tests, when I will be using real MoveAIComponent objects that do care about surroundings.

None of these options I particularly like. 1. greatly increases the complexity of both my tests and my mock class(es) since I need to add asserts to ensure the input is correct and cache every call to the mock objects. 2. feels like interface bloat. Since surroundings is only used inside MovementComponent, why should I make extra long-lived components to track what is actually temporary, transient, one use data? And finally 3. I dislike due to much the same reasons as 1, except worse. Tests could now fail for two reason: the surroundings is incorrect when passed to moveAI or the moveAI is making an unexpected decision.

So more generally, when you have a class that generates data that is only passed deeper into the stack and is never returned upward, what best way to test that functionality. Of course I am open to suggestions/redesigns that I have not thought of.

Improve this question
4
  • 1
    Could it be that you need to write your tests first? Commented Mar 4, 2016 at 16:06
  • 3
    So more generally, when you have a class that generates data that is only passed deeper into the stack and is never returned upward well, something is consuming that data - test whether that is consumed right (or processed correctly). Commented Mar 4, 2016 at 16:09
  • @enderland So that would lean towards my first option? Ensure that the object which is consuming the data is receiving the correct data? I don't want tests of MovementSystem to depend on the logic in MoveAI so storing the inputs in a mock object then analyzing would probably be the simplest way to handle that. Commented Mar 4, 2016 at 16:12
  • @RobertHarvey So I have been trying to use TDD but maybe I am rearranging steps. For this project I designed what my Systems would be (there are half a dozen or so) and in what order their update methods would be called. Then I stubbed in tests for MovementSystem, naming them based on the functionality it would have. Then I implemented the first test, then implemented the logic in MovementSystem for it to pass, then moved on to the next test. Perhaps that is where I messed up. When I made the first test pass, I implemented all of the generateMap logic which wasn't truly needed Commented Mar 4, 2016 at 16:17

1 Answer 1

Reset to default
7

First, your option 2 is not so bad is it might look to you. Having two separate, decoupled components A and B with a data flow from A -> B through a public interface allows you to test them each individually, or to "plug" some kind of monitor component M between them "A->M->B" for testing or logging purposes.

However, if you really dislike that approach, you could instead add such a (optional) monitor object to your class, only for testing purposes, which can be injected from outside. The monitor provides a method which gets the input of getMove and returns this immediately, and you use it like this:

 moveAI.getMove(monitor.log(surroundings))

Make the default implementation of the monitor a trivial one which just passes the input through. For testing purposes, you add a "real" monitor which caches the input of getMove. That way, you can keep that functionality out of your MoveAIComponent mock.

For complex classes, it is a well known, clean technique to provide "maintenance hatches" for testing purposes. The monitor object is an example for this. Don't be afraid of using this technique because it will mean to make few changes to your class to make it more testable.

Improve this answer
3
  • I like this idea. Additionally, the monitor injection in the MoveSystem constructor could have a default parameter so it would be entirely invisible when constructing them in the production environment. And hopefully the compiler would completely get rid of the do-nothing pass through in that case as well (although the virtual call may not allow that). Commented Mar 4, 2016 at 16:32
  • @GodricSeer: of course, that is indeed the kind of design I had in mind with my suggestion. Another design alternative is to give the monitor object a run-time switch to allow the activation / deactivation of "logging" also in production (if that is a requirement). Commented Mar 4, 2016 at 19:32
  • I have been thinking about this, and in a unit testing situation it isn't much different than idea 1 from my question (other than it simplifies all of my mock classes), but it has the added advantage of being equally useful for integration tests while also allowing debugging in the production environment if bugs appear if I use a runtime switch rather than polymorphism to remove it. I won't accept this answer for a day or so in case it chases another answer off, but this is likely the way I will go. Commented Mar 4, 2016 at 19:39

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.