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.

  • 1
    Could it be that you need to write your tests first? – Robert Harvey Mar 4 '16 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). – enderland Mar 4 '16 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. – Godric Seer Mar 4 '16 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 – Godric Seer Mar 4 '16 at 16:17
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.

  • 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). – Godric Seer Mar 4 '16 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). – Doc Brown Mar 4 '16 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. – Godric Seer Mar 4 '16 at 19:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.