Improving code quality - Session 4: Testing with breaking a door

Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.

This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.

Testing with breaking a door

The following IntAdder keeps the total of integer values given to add. When flush is called, it resets the current total to zero and returns the total just before it was reset.

val adder = IntAdder()

adder.add(1)
adder.add(2)
println(adder.flush()) // Shows 3

adder.add(100)
adder.add(200)
println(adder.flush()) // Shows 300

Let's assume the implementation of IntAdder is as follows.

class IntAdder {
    private var currentSum: Int = 0

    fun add(value: Int) {
        currentSum += value
    }

    fun flush(): Int {
        val result = currentSum
        currentSum = 0
        return result
    }
}

Up to this point, the code is sufficient to fulfill the specifications, and we assume there are no issues.

Next, suppose a developer decides to write unit tests for IntAdder and changes the visibility of currentSum from private to internal. In such cases, using annotations like @VisibleForTesting from the Guava library can indicate that the visibility is not intended for production code. Additionally, the visibility intended for testing can also be verified using lint tools.

class IntAdder {
    var currentSum: Int = 0
        @VisibleForTesting internal get
        private set

Using this currentSum, the following unit test was created to verify if the current total is correct.

// Unit test code
adder.add(100)
assertEquals(100, adder.currentSum)

adder.add(200)
assertEquals(300, adder.currentSum)

assertEquals(300, adder.flush())
assertEquals(0, adder.currentSum)

Is there an issue with this unit test code?

Testing only with open windows

In unit testing, it's preferable to test whether the observable behavior matches the specifications rather than the internal details. In other words, the main points to check in unit testing are:

• Function return values or exceptions
• Interactions with objects provided from outside (objects given as actual arguments, objects given as constructor arguments, injected objects, etc.)

Testing return values

In the case of the IntAdder implementation, we were keeping the current total as a number. However, this could change with specification changes. For example, if it becomes necessary to get a "history of value changes", the property var currentSum: Int might change to something like val valueHistory: MutableList<Int>. Even in such a case, the tests for add and flush should still be valid. However, the existing tests would not compile because the currentSum property itself has changed.

To solve this problem, instead of writing tests that peek at the value of currentSum, it's better to test the function's return values.

// Unit test code
adder.add(100)
adder.add(200)
assertEquals(300, adder.flush())
assertEquals(0, adder.flush())

Testing interactions with objects provided externally

To give an example of interactions with objects provided externally, let's add a new feature called "transaction" to IntAdder. We will use an object given as a constructor argument, transactionLogger, for the transaction implementation.

class IntAdder(private val transactionLogger: TransactionLogger) {
    ...

    fun inTransaction(action: Adder.() -> Unit) { 
        ... // Use `transactionLogger` here
    }
}

To test the interaction with transactionLogger, you should prepare a mock (in the language of test doubles, a test spy) of transactionLogger and verify the function calls made to it.

val logger: TransactionLogger = mock()
val adder = IntAdder(logger)

adder.inTransaction { add(100) }

inOrder(logger) {
    verify(logger).write(100)
    verify(logger).commit()
}

Of course, it's not necessary to mock every object provided externally. If the object is self-contained within the test environment and the function under test can be adequately observed, passing a non-mock object is not an issue.

---

In summary: In unit testing, it's better to verify that the behavior matches the specifications rather than focusing on the internal workings.

Keywords: unit test, dependency, specification