Category: Testing

Add tests, discover things!

The benefits that you get from testing your code will never cease to amaze me.

I was working on a new feature and part of it required changing one of our oldest classes. The class was still in Java so I decided, before doing any other work, to convert it to Kotlin. There were no tests so my first step was to add them and make sure that the conversion wasn’t going to break anything.

The class is a simple configuration one that

  • setups a couple of its fields upon construction and
  • exposes its validity state

something like this

public final class Configuration {
private final String agent;
private final String header;
private final String version;
public Configuration(final Storage storage) {
this.agent = storage.get(KEY_AGENT);
this.version = storage.get(KEY_VERSION);
this.header = storage.get(KEY_HEADER) + "/" + VERSION_PREFIX + this.version;
}
public boolean isValid() {
if (getAgent().length() == 0) return false;
if (getVersion().length() == 0) return false;
return getHeader().length() != 0;
}
public String getAgent() {
return agent;
}
public String getHeader() {
return header;
}
public String getVersion() {
return version;
}
}
view raw discover_by_testing_initial.java hosted with ❤ by GitHub

but with many more fields.

Falsely valid

So I started writing tests to cover all cases and the one that failed immediately was

@Test fun `the configuration is not valid when its header is empty`() {
whenever(mockStorage.get(KEY_AGENT)).thenReturn("an agent")
whenever(mockStorage.get(KEY_VERSION)).thenReturn("a version")
whenever(mockStorage.get(KEY_HEADER)).thenReturn("")
val actual = configuration().isValid
assertFalse(actual)
}
view raw discover_by_testing_failed_test.kt hosted with ❤ by GitHub

at first I thought I had an error in the test but looking closely at the production code I saw the problem. Actually the problems. Plural:

  1. Even thought in this case its subtle, the constructor is violating the SRP. It initializes the fields and makes a decision on what will be exposed. This is a job for the getter.
  2. Which brings us to the second problem. Getters are great way to expose information allowing us to make internal changes without breaking anything outside the class. They help us decouple ourselves from the classes that consume us.
    By using the getters instead of the fields, as in isValid, we couple the class with itself making it depend on what will be exposed and not the actual internal state of the class.

For the record the fix is

public final class Configuration {
private final String agent;
private final String header;
private final String version;
public Configuration(final Storage storage) {
this.agent = storage.get(KEY_AGENT);
this.version = storage.get(KEY_VERSION);
this.header = storage.get(KEY_HEADER);
}
public boolean isValid() {
if (agent.length() == 0) return false;
if (version.length() == 0) return false;
return header.length() != 0;
}
public String getAgent() {
return agent;
}
public String getHeader() {
return header + "/" + VERSION_PREFIX + this.version;
}
public String getVersion() {
return version;
}
}
view raw discover_by_testing_fixed.java hosted with ❤ by GitHub

and we managed to figure it out by writing a small, simple test.

Quick feedback

Having a quick feedback loop is a great way to keep yourself focused on the end result and to make less mistakes on the way. In our profession this is achieved with tests.

This bug was hiding for quite some time so the loop in this implementation was literally years!
Aim for shorter time periods! Seconds is the best and to do that is to write the test along side the production code.

I strongly encourage you to follow TDD but it doesn’t matter if its not your cup of tea. Write it after the production code, just do it immediately after.

Create a seam for testing using default values and function references

I learned about seams after reading Micheal Feathers’s book Working Effectively with Legacy Code. In essence a seam is a way to circumvent code that makes testing hard or even impossible.

For example, lets say we have a class that checks if a given task is valid. For reasons that do not interest us that same class makes a connection to another service and sends some data to it. That connection alone makes the class hard to test since we need to have and maintain a connection to that service during testing:

class TaskChecker {
fun check(task: Task): CheckResult {
if (isNotCreatedInCurrentWeek(task)) return Invalid
if (isResolved(task)) return Invalid
if (isNotAssigned(task)) return Invalid
return Valid
}
private fun isNotAssigned(task: Task): Boolean {
if (task.assignedTo != Nobody) return false
val connection = Connection()
val assigner = TaskAssigner(connection)
assigner.add(task)
return true
}
//…
}
view raw creating_a_seam_initial.kt hosted with ❤ by GitHub

In this example, isNotAssigned() makes the necessary checks but also sends the task to TaskAssigner so if we want to write some tests for TaskChecker we need to make sure that assigner is up and running.

Object seams

According to Mr Feathers there are three types of seams. The one that fits our case is called object seam and we are going to use it in order to bypass entirely making a connection and talking to the assigner.

Following the book’s example we end up with this:

class TestingTaskChecker : TaskChecker() {
override fun sendTaskToAssigner(task: Task) {
// do nothing
}
}
open class TaskChecker {
fun check(task: Task): CheckResult {
if (isNotCreatedInCurrentWeek(task)) return Invalid
if (isResolved(task)) return Invalid
if (isNotAssigned(task)) return Invalid
return Valid
}
private fun isNotAssigned(task: Task): Boolean {
if (task.assignedTo != Nobody) return false
sendTaskToAssigner(task)
return true
}
protected open fun sendTaskToAssigner(task: Task) {
val connection = Connection()
val assigner = TaskAssigner(connection)
assigner.add(task)
}
//…
}
view raw creating_a_seam_books_approach.kt hosted with ❤ by GitHub

which does exactly what we want since it provides a way to write tests that do not involve the assigner. We just need to use TestingTaskChecker in our tests and we are good to go.

The downside with this approach is that we had to open our class which might not meet the project’s standards.

Function reference

Lets see what we can do without opening the class.

Just like before we need to extract the behavior that we want to override to its own method but this time we are also going to assign this method to a value and use the value in the calling site:

class TaskChecker {
private val safeSendTaskToAssigner: (Task) -> Unit = ::sendTaskToAssigner
fun check(task: Task): CheckResult {
if (isNotCreatedInCurrentWeek(task)) return Invalid
if (isResolved(task)) return Invalid
if (isNotAssigned(task)) return Invalid
return Valid
}
private fun isNotAssigned(task: Task): Boolean {
if (task.assignedTo != Nobody) return false
safeSendTaskToAssigner(task)
return true
}
private fun sendTaskToAssigner(task: Task) {
val connection = Connection()
val assigner = TaskAssigner(connection)
assigner.add(task)
}
//…
}
view raw creating_a_seam_function_reference.kt hosted with ❤ by GitHub

isNotAssigned() will keep talking with the assigner only this time it does it through safeSendTaskToAssigner.

Default value

Having this function reference means that we can force isNotAssigned() to change its behavior by simply assigning a new value to safeSendTaskToAssigner! And this is what we are going to do:

class TaskChecker(
seamToAssigner: ((Task) -> Unit)? = null
) {
private val safeSendTaskToAssigner: (Task) -> Unit = seamToAssigner ?: ::sendTaskToAssigner
fun check(task: Task): CheckResult {
if (isNotCreatedInCurrentWeek(task)) return Invalid
if (isResolved(task)) return Invalid
if (isNotAssigned(task)) return Invalid
return Valid
}
private fun isNotAssigned(task: Task): Boolean {
if (task.assignedTo != Nobody) return false
safeSendTaskToAssigner(task)
return true
}
private fun sendTaskToAssigner(task: Task) {
val connection = Connection()
val assigner = TaskAssigner(connection)
assigner.add(task)
}
//…
}
view raw creating_a_seam_final.kt hosted with ❤ by GitHub

By default the seam is null which leads in having safeSendTaskToAssigner referencing the original behavior allowing the entire project to keep working as before without any additional changes to other files.

If now we pass a non null value then it gets assigned to safeSendTaskToAssigner and ends up being called instead of sendTaskToAssigner. This way we remove the communication from our flow allowing us to finally write some tests.

Testing

All we need to do is to write our tests by simply creating a checker with a do nothing seam:

@Test fun `a task that is not assigned is invalid`() {
val task = Task(AssignedTo.Nobody)
val taskChecker = TaskChecker {} // <– check with seam
val actual = taskChecker.check(task)
assertThat(actual, equalTo(Invalid))
}
view raw creating_a_seam_test.kt hosted with ❤ by GitHub

Don’t expose production code just for tests…

…or you’ll end up testing how your code does something and not what it does.

Think of it like a box

No matter what we consider to be a unit, be it a function, a class or an entire module, we should aim in testing it as we intend to consume it in the rest of our code.

This helps us in viewing the unit as a black box that accepts an input and provides an output. We don’t care what’s inside the box. We don’t care how the box handles our input. We only care about the outcome. This is what we need to assert.

Why do we write tests?

We write tests to make sure our code behaves as we intended it to. We write tests to document this behavior. We write tests to have a safety net whenever we wish to change the code but not its behavior.

The key here is behavior. Testing has nothing to do with implementation.

If we expose internal parts of our box we check how the box works and we couple it with our tests meaning that each time we make a change inside the box we have to change our tests too.

By definition this results in losing the ability to refactor.

Test it as it is meant to be used

When consuming a unit of code in production we respect its API and use it as is.

If we start testing individual parts of our box we might be certain that these parts work properly but we don’t know if their integration works too since we have asserted results that where the outcome of a flow that will never occur in our program. In other words we will never consume the API this way.

If testing the box as it’s meant to be used seems difficult then there is something wrong with the box’s API and by exposing code is like hiding all the dirt under the carpet. Eventually we will have to deal with it.

Test doubles: dummies, stubs, mocks, fakes

While testing we tend to replace some of the unit’s collaborators with mocks as it is accustomed to call them. The problem with that name is that it is not accurate. The real name of those mocks is test doubles and there are four of them with mock being one of the types.

One reason for this misnaming is the wide usage of mocking frameworks that do not separate the types between them (I am looking at you mockito).

So, lets try to define the four types and see when it is best to use them. We will be using a made up browser and its history and will not use any framework. Just theory:

interface History {
fun push(url: URL)
fun pop(): URL
fun peek(): URL
}
class Browser(
private val history: History
) {
var activeURL: URL? = null
private set
fun visit(url: URL) {
activeURL = if (url == URL("http://default"))
history.peek() else
url
history.push(activeURL!!)
}
fun back() {
history.pop()
activeURL = history.peek()
}
}
view raw test_doubles__usecase.kt hosted with ❤ by GitHub

Dummies

A dummy is the test double that we use whenever we know that the collaborator will not be used:

@Test fun `a newly created browser does not have an active URL`() {
val browser = Browser(dummyHistory)
assertThat(browser.activeURL, absent())
}
@Test fun `a visited URL is an active URL`() {
val browser = Browser(dummyHistory)
browser.visit(URL("https://www.le0nidas.gr"))
assertThat(browser.activeURL, equalTo(URL("https://www.le0nidas.gr")))
}
private val dummyHistory = object : History {
override fun push(url: URL) {
}
override fun pop(): URL {
TODO("Not yet implemented")
}
override fun peek(): URL {
TODO("Not yet implemented")
}
}
view raw test_doubles__dummies.kt hosted with ❤ by GitHub

For example in the tests above we just need to check the browser’s active URL. We know that this does not evolve the browser’s history so we pass a collaborator that does nothing on every method call.

Stubs

A stub is the test double that we use whenever the collaborator is being used to query values:

@Test fun `if the visited URL is the default then redirect to the last visited from the browser's history`() {
val browser = Browser(StubHistory(lastVisited = URL("https://www.le0nidas.gr")))
browser.visit(URL("http://default"))
assertThat(browser.activeURL, equalTo(URL("https://www.le0nidas.gr")))
}
private class StubHistory(
private val lastVisited: URL
) : History {
override fun push(url: URL) {
}
override fun pop(): URL {
TODO("Not yet implemented")
}
override fun peek(): URL {
return lastVisited
}
}
view raw test_doubles__stubs.kt hosted with ❤ by GitHub

For example in the test above we feed the browser with a pre-populated history since we know that the browser will need to peek for the last visited URL.

Mocks

A mock is the test double that we use whenever the collaborator is being used to perform an action:

@Test fun `every visited URL gets saved to the browser's history`() {
val mockHistory = MockHistory()
val browser = Browser(mockHistory)
browser.visit(URL("https://www.le0nidas.gr"))
mockHistory.verifySavedUrlIs(expectedURL = URL("https://www.le0nidas.gr"))
}
private class MockHistory : History {
private var savedURL: URL? = null
override fun push(url: URL) {
savedURL = url
}
override fun pop(): URL {
TODO("Not yet implemented")
}
override fun peek(): URL {
TODO("Not yet implemented")
}
fun verifySavedUrlIs(expectedURL: URL) {
assertThat(savedURL, equalTo(expectedURL))
}
}
view raw test_doubles__mocks.kt hosted with ❤ by GitHub

For example in the test above we need to make sure that the browser saves the provided URL to its history so we use a collaborator that can verify this behavior.

Fakes

A fake is the test double that we use whenever we need the collaborator to provide us a usable business logic:

@Test fun `going back restores the previously visited URL`() {
val browser = Browser(FakeHistory())
browser.visit(URL("https://www.le0nidas.gr"))
browser.visit(URL("https://www.google.com"))
browser.back()
assertThat(browser.activeURL, equalTo(URL("https://www.le0nidas.gr")))
}
private class FakeHistory : History {
private val urls = mutableListOf<URL>()
override fun push(url: URL) {
urls.add(0, url)
}
override fun pop(): URL {
return urls.removeAt(0)
}
override fun peek(): URL {
return urls[0]
}
}
view raw test_doubles__fakes.kt hosted with ❤ by GitHub

For example in the test above we need a history instance that works as expected (a simple stack) but without the hassle of having a database or using the file system.

Final thoughts

Having your own test doubles per case makes the code simpler and more readable but does that mean that we should remove our mocking frameworks? In my opinion no. Having a framework saves you a lot of time and keeps things consistent, especially in big projects with lots of developers.

Knowing the theory behind something is always good since it lays a common foundation for discussions and decisions. A mix of the two, framework and theory, could be achieved and help the test code in readability.
For example, we can keep using Mockito’s mock but name the variable stubBlahBlah if is used as a stub. This way the reader will know what to expect.

PS #1: Spock testing framework, besides being a great tool, provides a way to separate stubs from mocks not just in semantics but in usage too (ex: you cannot verify something when using a stub)

PS #2: There is another type of test double called Spy which is a toned down mock that helps in keeping state when a certain behavior takes place but does not verify it.

Don’t share constants between production and test code

Building upon my previous post and the trick of being specific in the values the code respects, one pattern that I’ve noticed which can easily lead in many false positive tests is sharing a constant value between production and test code.

If the test code reads the value from the production, any change that was done by mistake will not affect the test which will continue to pass!

21 yeas of age

Lets say that we have two services, one checks if a customer can enter a casino and the other if she can buy alcohol. For both cases the law states that the minimum legal age is 21 years old.

The code has a configuration file, a domain and two modules for each service:

// Production code:
// configuration
object Config {
const val MIN_LEGAL_AGE = 21
}
// domain
class Person(val age: Int)
// entrance module
fun canEnterCasino(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// alcohol module
fun canBuyAlcohol(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// Test code:
// entrance module
fun `a customer can enter the casino when she is older than 21 years of age`() {
val twentyOneYearOld = Person(Config.MIN_LEGAL_AGE)
val actual = canEnterCasino(twentyOneYearOld)
assertTrue(actual)
}
// alcohol module
fun `a customer can buy alcohol when she is older than 21 years of age`() {
val twentyOneYearOld = Person(Config.MIN_LEGAL_AGE)
val actual = canBuyAlcohol(twentyOneYearOld)
assertTrue(actual)
}
view raw dont_share_constants__sharing.kt hosted with ❤ by GitHub

As you can see the tests consume the minimum age directly from the production code but the test suite passes, life is good.

Then one day, the law changes and the minimum legal age for entering a casino drops to 20 years! Simple change, not much of a challenge for the old timers so the task is being given to the new teammate who does not know all modules yet and is also a junior software engineer.
She sees the test, changes the value in the name to 20, sees the config, changes the constant’s value to 20, runs the test suite, everything passes, life is good! Only that it isn’t because the casino’s software now allows selling alcohol to 20 year olds!

Keep them separate

If the test code did not use the production’s code

// Production code:
// configuration
object Config {
const val MIN_LEGAL_AGE = 20
}
// domain
class Person(val age: Int)
// entrance module
fun canEnterCasino(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// alcohol module
fun canBuyAlcohol(person: Person): Boolean {
return person.age >= Config.MIN_LEGAL_AGE
}
// Test code:
// entrance module
fun `a customer can enter the casino when she is older than 20 years of age`() {
val twentyOneYearOld = Person(20)
val actual = canEnterCasino(twentyOneYearOld)
assertTrue(actual) // passed
}
// alcohol module
fun `a customer can buy alcohol when she is older than 21 years of age`() {
val twentyOneYearOld = Person(21)
val actual = canBuyAlcohol(twentyOneYearOld)
assertTrue(actual) // failed
}
view raw dont_share_constants__not_sharing.kt hosted with ❤ by GitHub

then, after changing the constant’s value, the test suite would fail alerting the software engineer that something has broken forcing her to figure it out and craft another solution.

Your tests can also be your documentation

Tests help as make sure that our code works, provide us a safety net when we need to refactor and, when having proper test names, can be a good documentation describing what the code does.
The last one can be especially helpful for both newcomers that need to understand the system and old timers that haven’t visited the code for a while!

A couple of tricks for achieving good names are:

  1. Avoid describing how the code does something and try to describe what it does
    For example:
    calling add(item) results in calling recalculate
    is way too specific without providing anything meaningful, or anything that we wouldn’t get from reading the code.
    On the other hand:
    a recalculation of the order's value takes place every time a new item gets added
    shares an important information about the Order‘s behavior when adding an item.
  2. Avoid being too abstract
    For example:
    a customer can buy alcohol when she is of legal age
    can help the reader understand how the code behaves but in a documentation you need specific values.
    So:
    a customer can buy alcohol when she is older than 21 years of age
    is much better because it also provides the exact threshold that our code considers for allowing someone to buy alcohol

Good practices: First write the test then fix the bug

You get a report about a bug. You open the app, follow the steps to reproduce it and, as mentioned in the report, your app is misbehaving. What’s next?

You can either dig immediately in the code and fix the bug or you can re-reproduce the bug, only this time in a test. The second. Always go with the second option.

Here is why:

  1. From now on you will have a regression test.
    Meaning that if a change in the code breaks what you fixed you’ll get notified from the test suite and not your users
  2. It keeps you focused / You know when you finished.
    This is a benefit you get from TDD in general. When the test passes the bug is fixed and you can move to your next task. Also, since you have to make the test pass, anything else that popped up during your research for the bug can wait (I usually write it down to a notepad I keep next my keyboard).
  3. You get a better understanding of the code.
    By trying to write the test you get a better knowledge of how things are connected and communicate. Especially if you are new to a project this will boost your understanding significantly.
  4. You discover more corner cases.
    There are times that by writing this one test and seeing what inputs a class/function can have, you wonder how will the app behave under certain values. Finish the task at hand and then add a test for each case you want to explore. You might end up solving more bugs!

TIL: @NullAndEmptySource in JUnit5

This is a clear case of RTFM!

I wanted to make sure that a function will return null when given a null or empty string and what I ended up doing was something like this:

internal class CreateNameTest {
@ParameterizedTest
@ValueSource(strings = ["null", "", " "])
fun `there is no creation when the provided value is null or empty or blank`(providedValue: String?) {
val value: String? = if (providedValue == "null")
null else
providedValue
assertThat(createName(value), absent())
}
}
view raw initial_parameterized_test.kt hosted with ❤ by GitHub

@ValueSource does not accept null values so I passed it indirectly!

I didn’t like it so after, finally, reading the JUnit5 documentation I learned that the library had me covered from the beginning by providing three annotations exactly for this use case:

@NullSource, @EmptySource and @NullOrEmptySource are meant to be used whenever we need to check the behavior of our code when given null or empty inputs.

So the test changes to:

internal class CreateNameTest {
@ParameterizedTest
@NullAndEmptySource
@ValueSource(strings = [" "])
fun `there is no creation when the provided value is null or empty or blank`(providedValue: String?) {
assertThat(createName(providedValue), absent())
}
}
view raw parameterized_test_with_proper_annotations.kt hosted with ❤ by GitHub

PS: for the curious, the absent() is part of the hamkrest library by Nat Pryce (yes, that Nat Pryce)