How do I test a method which requires another method of the same class to be invoked first?

Question

I have a simple class which looks like this:

class SpecialList {
    private List<Integer> varList;

    SpecialList() {
        varList = new ArrayList<>();
    }

    void addVar(int i) {
        varList.add(i);
    }

    boolean isVarListSizeFive() {
        if (varList.length() == 5) {
            return true;
        }

        return false;
    }
}

I want to write tests for this class.

1. There is no getter for the private member varList. In my test for addVar() how do I check if the varList is in the correct state? Is my only option to define a getter, or use reflection to modify a private variable (which seems like an anti-pattern)?

2. The setter for varList is the other method I'm testing addVar(). To test isVarListSizeFive() I would have to call addVar() first 5 times to modify varList before testing it. Isn't it an anti-pattern to call another method first to test this method?

What is the best way to test this class? Do I have to modify this class just to achieve testing?

Answer 1

A unit test should test a single unit or component in isolation. But you get to decide what that unit/component is. The correct boundaries for the system under test depend a lot on context. Usually, the unit is one object or one procedure/method. But it is generally impossible to test a single method in isolation, precisely because you need to set up object state first.

Here, the property you're trying to verify isn't “this method returns true for a certain internal state”, but “for all states of the object this method returns false, except for this certain state”. This viewpoint makes it clearer that you're verifying something about the entire object, and the object's state depends entirely on the addVar() calls.

Here's how I would write that test. Since we're trying to verify a property of the object that holds in multiple states, the arrange/act/assert or given/when/then pattern is not a good fit, instead I check the property at every step:

void testVarListSizeFiveProperty() {
    SpecialList o = NewSpecialList();
    assertFalse(o.isVarListSizeFive()); // property is false in the initial state

    // for the first 4 elements, the property will remain false
    for (int i = 0; i < 4; i++) {
      o.add(1+i);
      assertFalse(o.isVarListSizeFive());
    }

    // after the 5th element, the property will be true
    o.add(5);
    assertTrue(o.isVarListSizeFive());

    // for all remaining elements, the property will be false again
    for (int i = 5; i < 100; i++) {
      o.add(1+i);
      assertFalse(o.isVarListSizeFive());
    }
}

Note that this also violates a literal reading of the “only one assert per test” rule. That rule isn't about assert() functions, it is about only asserting one property of the system under test, so that test failures clearly indicate the problem. Here, the test is only asserting this certain property, even though it is a multi-step procedure.

This becomes more apparent in integration tests or acceptance tests where one test often exercises an entire use case, consisting of multiple steps. Those steps cannot run in isolation but they are dependent on each other. In that case, it is perfectly normal and necessary to have a large test script that performs multiple checks in intermediary results.

Sometimes, there is the temptation to modify a design in order to make it more testable. There are both good and bad aspects to this.

• It is good to design testable systems. E.g. small, pure (stateless) components can help, as can dependency inversion.

  Every piece of code has at least two consumers: dependent code, and the tests. It is obvious that the code needs to be designed to fulfil the requirements of dependent code, but testability is also a legitimate requirement.

  Here, you're having difficulty because your object has internal states: 7 states corresponding to list sizes {0,1,2,3,4,5,more}, but luckily in a simple linear order.

• In some cases, it is good to add a “maintenance hatch” to your objects that you can use to access internal state for your tests.

  Here, that might be a protected constructor to inject a particular list state for your tests. However, I think it would be preferable to keep testing the state machine directly.

• But it is not good to entirely sacrifice the design just for testing. This increases the API surface of your objects, and can make it more difficult to ensure encapsulation.

  For example, adding a getVarList() accessor that publicly exposes the internal list sounds easy, but will now make it impossible to ensure that the list will only be modified per the addVar() method. You would lose encapsulation. While this doesn't add more states in this specific case, the state machine you'd have to test is far more complex because state transitions are no longer restricted through the addVar() method.

  Getters and setters should be added when this follows from the requirements from dependent code, but (in most cases) not just to make testing more convenient. It is better to test an object through it's externally observable behaviour, where possible.

Answer 2

The code example you provided is somewhat nonsensical, but I assume this is intended as a trivial example, not real code. My answer is written under that assumption.

---

AAA

The basic AAA pattern helps in understanding how to structure your tests.

• Arrange - Prepare the necessary environment for the test
• Act - Test the component
• Assert - Confirm the test meets the expectations

A very basic example would be:

[Test]
public void Person_Name_is_automatically_capitalized()
{
    // Arrange
    var person = new Person();
    var name = "john";

    // Act
    person.Name = name;

    // Assert
    person.Name.Should().Be("John");
}

Unit tests that focus on a specific public behavior of an object tend to have act statements that contain nothing more than var result = myObject.MyMethod(myParams); (barring the var result for void methods, of course)

I personally prefer putting those comments in my test specifically to separate the method body into its three separate components, and it's generally not worth actually creating three submethods for this (it creates way too much bloat for short tests).

How do I test a method which requires another method of the same class to be invoked first?

This first method is not the method under test, and therefore it doesn't belong to the "act" step, it belong to the "arrange" step.

There's nothing wrong with calling this first method, but it is important to consider this as part of the arrangement, because it highlights that means that this method is assumed to work correctly.

[Test]
public void isVarListSizeFive_returns_true_when_5_items_added()
{
    // Arrange
    var list = new SpecialList();
    list.addVar(1);
    list.addVar(1);
    list.addVar(1);
    list.addVar(1);
    list.addVar(1);

    // Act
    var result = list.isVarListSizeFive();

    // Assert
    result.Should().Be(true);
}

You might wonder that this test will fail if there's a bug in addVar, and not in isVarListSizeFive. That is correct. However, there are two important realizations here:

• That is the nature of the beast when you already chose to build your SpecialList class in a way that it is impossible to properly use isVarListSizeFive without first relying on addVar
• addVar should also have its own unit tests, and those should be failing at the same time as well. When you see both of these methods' test failing, and you remember/notice that the isVarListSizeFive unit test inherently depends on addVar in the arrange logic, this suggests to first inspect addVar as it is the common root for both test failures.

If you don't like having one method's tests rely on the implicit correctness of another method, then don't implement your classes the way you have. This is really a matter of personal choice. In either case, you've got a sturdy testing strategy.

Answer 3

To test isVarListSizeFive() I would have to call addVar() first 5 times to modify varList before testing it. Isn't it an anti-pattern to call another method first to test this method?

This is exactly how I would do it, so I'm going to vote "no, it is not an anti-pattern".

One thing to notice here, is that in your testing of isVarListSizeFive you are also necessarily invoking your constructor (either directly within the test, or indirectly via some fixture).

It's probably more appropriate to understand the state machine as your unit (test subject), rather than trying to partition it into multiple units.

Alternative spelling: a dogmatically constrained "unit test" is not necessarily the right tool to investigate mistakes that have been introduced in this code.

---

As a thought experiment, consider this implementation of SpecialList::add

    void addVar(int i) {
        // varList.add(i);
    }

Clearly, the code is broken. Furthermore, it's broken in a way that should be really easy to catch with automated checks.

But if our rule is that a "unit test" can only include one method, then you are left with a choice of either inadequate coverage or encapsulation violating inspection of the implementation details.

---

An alternative might be to change your design, so that you can get at the thing you really want to test without the baggage in the way.

Here, that might look like:

class SpecialList {
    private List<Integer> varList;

    SpecialList() {
        varList = new ArrayList<>();
    }

    void addVar(int i) {
        varList.add(i);
    }

    boolean isVarListSizeFive() {
        return isVarListSizeFive(this.varList);
    }

    // This might want a different name
    // and an argument type that isn't mutable.
    static boolean isVarListSizeFive(List<Integer> varList) {
        if (varList.length() == 5) {
            return true;
        }

        return false;
    }
}

In this design, you can write checks against your SizeFive logic without jumping through the hoops of creating and manipulating a SpecialList first.

Complicated code should be easy to test.

Another alternative would be to treat the varList as a sort of collaborator

class SpecialList {
    private List<Integer> varList;

    SpecialList() {
        this(new ArrayList<>());
    }

    SpecialList(List<Integer> varList) {
        this.varList = varList;
    }

Now you can invoke methods on SpecialList, and measure their effect using your handle to the same data structure.

Answer 4

As written, you could:

• Call the Add method x times in the setup of your (assumed) unit test
• Use reflection on the private member

A unit test should test a single method. For stateful classes it is sometimes necessary to call other methods on the class in the setup portion.

If this 'smells' bad to you, then you are beginning to appreciate one of the appeals of pure functions (where a given set of inputs always yields the same output).