TDD Platformer Part 1: First Tests

Welcome to the first part of my TDD Platformer series, if you’ve just jumped here, it’d be good to read the intro first.

For those of you following along at home, the code for all this can be found on GitHub

Unity and Unit Testing

I mentioned in the introduction that making a TDD game in Unity has some difficulties, the main one being that the standard way to add behaviours to objects in Unity is to use classes that inherit from MonoBehvaiour, and such classes cannot be unit tested. This can lead to a situation where you have to use the integration test framework for a large number of your tests, and whilst the framework is good, it suffers from all the usual issues of higher-level tests (slower, more flakey, more difficult to diagnose what’s wrong).

To get around this, the Unity Test team wrote a couple of blog posts detailing an approach that uses interfaces and mock-objects.

Following this pattern, you end up with a ‘normal’ C# class that has properties for the interfaces it uses, allowing you to mock out these interfaces for unit tests. The MonoBehaviour also implements these interfaces, but with the ‘real’ implementation. The Unity Test Framework even comes with NSubstitute to support this mock-based approach.

But I don’t like mocks, so I’ll be doing something else.

The Case Against Mocks

Mocks can be very powerful and incredibly useful, they are definitely the right tool to use in certain situations (legacy code being a good one) and if you’re new to testing you should basically use them with reckless abandon (because learning the alternatives takes time and it’s valuable to just start testing now, you can always refactor them away later). But they come with a lot of issues.

They make your tests more fragile, because all the tests are tightly bound to the mock (and thus to the interface) any change to the interface can cause many tests to break (ideally, only a change to behaviour would cause a test to break, and then only a single test)
It’s easy to mock your way out of a bad design. If you need to create 3/4 mocks per test, that should tell you that the code design is bad, but since it’s so easy to create mocks (especially with a mocking framework) it can be easy to miss this feedback.
You can end up just testing the mock, especially when you have mocks returning mocks. (I have done this in the past).
It makes the tests messy, the idea unit test is really short and the most interesting part of the code is the actual line that does the behaviour you’re testing. Setting up a lot of mocks makes the tests longer and makes it harder at a glance to see what is actually being tested.

Essentially, I consider mocks to be a ‘code smell’ and look for ways to design my code so that it doesn’t use them. In practice this means I use a lot of callbacks, and make heavy use of events, both of which are easy to test (but can make the interactions between classes more difficult to follow).

That said sometimes I still do use mocks, as a general rule I’ll give myself a short time to figure out a better design and if I can’t I’ll use a mock (and hand-write it, rather than using a framework)

The Alternative

For the main character I’m going to attempt a different approach, all of the interesting code will be on a normal C# class, which will have properties for all the parts of the world-state (are we standing on the ground?) and the input state that it is interested in. There will then be some form of Update method that will use those inputs and return a desired movement vector. The MonoBehaviour will be responsible for setting the world-state and input state, and applying the result.

This seems like a fairly nice design, we’ll only need a few integration tests to check that the MonoBehaviour sets the world and input states correctly, and one to check that it applies the result correctly, all the other tests can be unit tests (which is good because character control logic can get complex really fast)

The First Tests

Falling

The first test we’ll do will show that if we don’t press any input, and the player is not on the ground, they will start to fall.

This is the test:

[Test]
public void ShouldFallIfNotOnGround()
{
    var movement = new PlayerMovement {IsOnGround = false};

    var result = movement.Update();

    result.y.Should().BeLessThan(0.0f);
}

And it fails with the following:

ShouldFallIfNotOnGround (0.016s)
---
FluentAssertions.Execution.AssertionFailedException : Expected a value less than 0, but found 0.
---

Already there’s a potential refactoring creating the PlayerMovement class, at some point I’ll probably want to make use of the builder pattern to set those properties, but for now it can stay. It’s also worth pointing out that I set the IsOnGround property to false (even though that’s the default value for bools) to make it more clear what the setup is. Finally note that I’m only checking the y component of the vector, and only checking that it is less than zero, rather than some actual value, asserting on the minimum amount required for the test is key to keeping tests maintainable.

So let’s make this test pass, we can do that with by making the PlayerMovement::Update function be:

public Vector3 Update()
{
    return new Vector3(0.0f, -1.0f);
}

Switch back to Unity and the test passes. Job done.

This is an example of the restraint you need to show when doing the ‘implement’ step of TDD, obviously at some point we’ll need to check the IsOnGround field and change what we do based on the value of that, but at the moment we don’t have a test that requires that behaviour, so the behaviour shouldn’t exist.

Now we’re on to the final step of TDD, refactoring. Is there anything about that code that is bad? Well that magic number sticks out a fair bit, so lets fix that. The entire PlayerMovement class now looks like:

public class PlayerMovement
{
    private const float Gravity = -1.0f;

    public bool IsOnGround { get; set; }

    public Vector3 Update()
    {
        return new Vector3(0.0f, Gravity);
    }
}

The gravity implementation is still really bad, but we don’t need to solve that problem yet, so let’s move on to the next test.

Not Falling

The next obvious test is that if we’re on the ground, we’re not falling:

[Test]
public void ShouldNotFallIfOnGround()
{
    var movement = new PlayerMovement { IsOnGround = true };

    var result = movement.Update();
    result.y.Should().Be(0.0f);
}

This obviously fails:

ShouldNotFallIfOnGround (0.011s)
---
FluentAssertions.Execution.AssertionFailedException : Expected value to be 0, but found -1.
---

So let’s update the PlayerMovement class:

public Vector3 Update()
{
    if (IsOnGround)
    {
        return Vector2.zero;
    }
    return new Vector3(0.0f, Gravity);
}

And now the test passes.

On to the refactor step. I’m pretty happy with both the PlayerMovement class and the tests at the moment, so we move on to the next test.

Falling Faster

I’ve decided that the next test should prove that the player will start to fall faster if they’re not on ground for multiple updates.

The test is:

[Test]
public void ShouldAccelerateWhilstFalling()
{
    var movement = new PlayerMovement { IsOnGround = false };

    var firstResult = movement.Update();
    var secondResult = movement.Update();

    secondResult.y.Should().BeLessThan(firstResult.y);
}

Which fails:

ShouldAccelerateWhilstFalling (0.016s)
---
FluentAssertions.Execution.AssertionFailedException : Expected a value less than -1, but found -1.
---

To make this pass we’ll need to make a quick change to the PlayerMovement class:

public class PlayerMovement
{
    private const float Gravity = -1.0f;

    public bool IsOnGround { get; set; }

    private Vector3 currentVelocity;

    public Vector3 Update()
    {
        if (IsOnGround)
        {
            return Vector2.zero;
        }
        currentVelocity += new Vector3(0.0f, Gravity);
        return currentVelocity;
    }
}

For refactoring, I notice that that Update method is both modifying and returning the currentVelocity field, this is a strange thing to do. I should just expose that as a property. There’s no automatic refactoring that I know of that will do this in one step, so I do it in several:

Covnert the field into an auto-property (all the tests still pass)
Make the ‘IsOnGround’ path set the currentVelocity to zero
Update each of the tests to use the property rather than the return value (checking that they all pass after each one)
Remove the return value

Doing it in these tiny steps made it really easy to spot any mistakes, and as it happened, when I was taking the second step, I forgot to surround the currentVelocity += new Vector3 line in an else, so the ‘ShouldNotFallIfOnGround’ test failed. (numBugsPrevented++)

The PlayerMovement class now looks like this:

public class PlayerMovement
{
    private const float Gravity = -1.0f;

    public bool IsOnGround { get; set; }

    public Vector3 CurrentVelocity { get; private set; }

    public void Update()
    {
        if (IsOnGround)
        {
            CurrentVelocity = Vector2.zero;
        }
        else
        {
            CurrentVelocity += new Vector3(0.0f, Gravity);
        }
    }
}

And the tests look like this:

[Test]
public void ShouldFallIfNotOnGround()
{
    var movement = new PlayerMovement {IsOnGround = false};

    movement.Update();

    movement.CurrentVelocity.y.Should().BeLessThan(0.0f);
}

[Test]
public void ShouldNotFallIfOnGround()
{
    var movement = new PlayerMovement { IsOnGround = true };

    movement.Update();

    movement.CurrentVelocity.y.Should().Be(0.0f);
}

[Test]
public void ShouldAccelerateWhilstFalling()
{
    var movement = new PlayerMovement { IsOnGround = false };

    movement.Update();
    var firstResult = movement.CurrentVelocity;
    movement.Update();

    movement.CurrentVelocity.y.Should().BeLessThan(firstResult.y);
}

I think this is a good place to stop and move over to the integration tests that we’ll need to check that the PlayerMovementBehvaiour sets the IsOnGround property, and applies the CurrentVelocity correctly, so we’ll pick that up in part 2.