I've been a big fan of testing ever since I was introduced to it, most of the code I write is written with Test Driven Development. Having a good test suite helps with catching regressions / bugs a lot faster, and gives developers more confidence when merging / releasing their product.

That's why I encourage / expect my teammates to write tests for the important parts of the code (In our case, it's the shared KMM codebase for Android and iOS). When reviewing PRs, I sometimes see testing approaches which could be better, in this series, I'll share my recommendations for improving your tests.

A lot of these tips are related with each other, and sometimes they complement each other. However, I'll try to keep every point straight forward and make it easy to understand without dragging additional information into them.

Topics

  1. Arrange Act Assert
  2. Not creating helper functions
  3. Unnecessary set up / Arrangement
  4. Testing multiple things instead of focusing on one
  5. Not using explicit set-up values in tests, but depending on default or global values

This series will continue as part of my newsletter, where every week I'll cover topics from this list (which might change slightly over time):

  • Overusing mock frameworks
  • Comparing whole complex data structures instead of only the tested fields
  • Not using Soft assertions
  • Not using explicit values in tests, but depending on default / global values
  • Repeating the SUT Algorithm in assertions instead of an explicit value
  • Testing implementation details instead of the outcome
  • Using "private" functions in tests
  • Too generic test case naming
  • Conditions in tests
  • Loops instead of parameterized tests
  • Not focusing on boundary values
  • Not caring about test code quality
  • Always adding tests into only one file
  • Ignoring tests in the CI process
  • Not using mutation testing for verifying if the tests are written correctly

If you're interested and like to see more, checkout the sign-up form at the bottom of this post, or on my homepage.

Note: All the examples here, are written with Kotlin, however, I believe that the tips presented here, could be applied in most modern programming languages.

1.  Not using the Arrange Act Assert (Triple A) Convention

Often times I see developers group the test code like they would on production code, i.e. based on what logic is related. In tests, however, there exists a common convention / pattern which splits the code into three distinct groups separated by a new line:

@Test
fun something() {
   givenApiReturnsSuccess() // arrange / set up
   
   val result = systemUnderTest.fetchDetail("1") // act / action
   
   assertEquals(Detail("1"), result) // assert / verify
}

These groups don't have to be one-liners, but it would be best to keep the Act group as small as possible (preferably one line). Keeping the AAA convention in all the tests helps with understanding:

  • What set-up is needed
  • What action is taken to achieve the outcome
  • What is verified by the test.

2. Not creating helper functions

Tests almost always include some boilerplate, be it set up code, verification and sometimes even the action. Helper functions help address this issue

  • Help with test readability by hiding irrelevant implementation details. With a readable test, it is much easier to understand what the system under test does
  • Protects from code changes, e.g: instead of changing a parameter in every test case, it only needs to be changed in the helper function

Take a look at the following test:

@Test
fun storageItemsAreReturnedWithTimestamp() {
    val item1 = StorageItem(id = 1, name = "")
    val item2 = StorageItem(id = 2, name = "")
    fakeStorage.items = listOf(item1, item2)
    val timestamp = 123
    fakeTimestampProvider.value = timestamp

    val result = systemUnderTest.fetchFromStorage()

    val result1 = result.first { item -> item.id == 1 }
    assertEquals(timestamp, result1.timestamp)
    val result2 = result.first { item -> item.id == 2 }
    assertEquals(timestamp, result2.timestamp)
}

In this example, we have a systemUnderTest (SUT) which retrieves items from a storage and adds the current timestamp to them. However, the code includes some boilerplate that will most likely be present in other test cases, an improved version could look like this:

@Test
fun storageItemsAreReturnedWithTimestamp() {
    fakeStorage.items = listOf(createStorageItem(1), createStorageItem(2))
    val timestamp = 123
    fakeTimestampProvider.value = timestamp

    val result = systemUnderTest.fetchFromStorage()

    verifyItemHasCorrectTimestamp(result, id = 1, expectedTimestamp = timestamp)
    verifyItemHasCorrectTimestamp(result, id = 2, expectedTimestamp = timestamp)
}

private fun createStorageItem(id: String): StorageItem {
    return StorageItem(id = 1, name = "")
}

private fun verifyItemHasCorrectTimestamp(
    items: List<Item>,
    id: Int,
    expectedTimestamp: Int
) {
    val item = result.first { item -> item.id == id }
    assertEquals(expectedTimestamp, item.timestamp)
}

The above code is more verbose judging just by the number of lines, however the more test cases we have, the more we benefit from such helper functions.

Other examples:

fun givenApiReturnsSuccess(id: String, result: Detail) {
   mockk { fetchDetail(id) } returns result
}

fun givenDatabaseContainsItemWithId(id: String) {
   val item = ComplexItem(id = id, name = "", timestamp = 0L)
   fakeStorage.items = listOf(item)
}

fun executeAndAdvanceTime() {
   systemUnderTest.execute()
   advanceTimeBy(milliseconds = 2000)
}

One additional example I like to use is a "testCase" helper function. From time to time you will have a behavior which requires a lot of tests, with slightly different set-up / assertions:

@Test
fun `Timestamp of 0 becomes "00 00 000"`() {
    formatTestCase(0, "00:00:000")
}

@Test
fun `Timestamp of 500 becomes "00 00 500"`() {
    formatTestCase(500, "00:00:500")
}

@Test
fun `Timestamp of 59999 becomes "00 59 999"`() {
    formatTestCase(59999, "00:59:999")
}

private fun formatTestCase(
    timestamp: Long,
    expectedString: String
) {
    val result = systemUnderTest.format(timestamp)

    result shouldBe expectedString
}

These types of helper functions can be even more complex, as long as they help with the readability and maintainability of the test.

However, there is one important point I'd like to address before going to the next point. Don't force out these helper functions, as the name suggests, they should help you and not make your life harder.

There are cases where it's better to leave the code inline without any helper functions. Like with everything in programming, this knowledge comes with experience, as long as the helper functions make the tests easier to understand and maintain, go for it.

3. Unnecessary set up / Arrangement

Sometimes when creating a new test case, we copy an existing one, and change it to fit a new test case. When doing, so we sometimes copy more than we need, tests should only contain code which is relevant to that particular test case. More code in the tests, means it will be harder to understand, and sometimes even confuse the reader because some parts are irrelevant.

Imagine, we have a function which always retrieves something from a database cache (the function never calls the API):

@Test
fun databaseIsUsedToRetrieveCachedValue() {
   val item = Item(...)
   givenDatabaseContainsItem(item)
   givenApiReturnsSuccess(item)
   
   val result = systemUnderTest.fetchFromStorage()
   
   assertEquals(item, result)
}

The above test is confusing for two reasons: looking at the set-up we can take a guess that the action uses both the database and the API, additional confusion is added, because we're not sure if the item comes from the database or from the API. Only inspecting / debugging the production code would give us the answer.

Keeping our test code focused on only the relevant things will make our test cases easier to understand. In this case, removing the givenApiReturnsSuccess would reduce the confusion by a lot:

@Test
fun databaseIsUsedToRetrieveCachedValue() {
   val item = Item(...)
   givenDatabaseContainsItem(item)
   
   val result = systemUnderTest.fetchFromStorage()
   
   assertEquals(item, result)
}

4. Testing multiple things instead of focusing on one

Most tests should focus on one particular behavior of the system under test. However, sometimes tests are created in ways that verify multiple things. At first sight it might seem beneficial, because less code should make it easier to understand, right?

@Test
fun stateAfterInitializingIsCorrect() {
    givenApiReturns(listOf(1, 2, 3))

    systemUnderTest.initialize()
    
    assertEquals(false, systemUnderTest.isLoading)
    assertEquals(listOf(), systemUnderTest.selectedItems)
    assertEquals(listOf(1, 2, 3), systemUnderTest.items)
}

Well not exactly, the problems arise, when the tests stop passing. Then you have to first look at the test code, and if that doesn't suffice you'll have to debug it, which is not ideal... That's why it's better to break it up into more test cases:

@Test
fun isLoadingIsFalseAfterInitializing() {
    systemUnderTest.initialize()
    
    assertEquals(false, systemUnderTest.isLoading)
}

@Test
fun selectedItemsAreEmptyAfterInitializing() {
    systemUnderTest.initialize()
    
    assertEquals(listOf(), systemUnderTest.selectedItems)
}

@Test
fun itemsAreRetrievedFromTheApiOnInitialization() {
    givenApiReturns(listOf(1, 2, 3))
    
    systemUnderTest.initialize()
    
    assertEquals(listOf(1, 2, 3), systemUnderTest.items)
}

Having separate test cases for the verification involves more code, but the tests should be easier to understand, because they focus only on one aspect of the SUT. The benefit of that is, that if the test fails, the name should be more than enough to understand what's wrong with the SUT (As long as the test name is descriptive...)

It's important to understand and separate things that are different in each test case.

5. Not using explicit set-up values in tests, but depending on default or global values

Tests should be as explicit as possible (while not hindering its readability).

Sometimes when we define a test double, we might be tempted to put default values in order to reduce the amount of code we need to write in our tests. In my opinion this is not the best approach, test doubles should not have hidden values which make the tests pass, takes this for example:

interface Config {
    val numberOfRepetitions: Int
}

class FakeConfig : Config {
    override var numberOfRepetitions: Int = 3
}

class SomeTest {
    // ...

    @Test
    fun apiCallOnFailShouldBeRepeatedTheCorrectAmountOfTimes() {
        mockApi.shouldReturnError()

        systemUnderTest.fetch()

        assertEquals(3, mockApi.fetchCallCount)
    }
}

In the example above, we have a hidden default value in inside FakeConfig. This value is not set explicitly in the tests, so without delving into the implementation details, we won't know where the magic number 3 comes from. Of course, we could rename the test to be even more wordy, but it's better to let the test code speak for itself:

class FakeConfig : Config {
    override var numberOfRepetitions: Int = 0
}

class SomeTest {
    ...

    @Test
    fun apiCallOnFailShouldBeRepeatedTheCorrectAmountOfTimes() {
        mockApi.shouldReturnError()
        val repetitionAmount = 3
        fakeConfig.numberOfRepetitions = repetitionAmount

        systemUnderTest.fetch()

        assertEquals(repetitionAmount, mockApi.fetchCallCount)
    }
}

In my opinion, the above test is much easier to understand and the systemUnderTest behavior is much more explicit, even if it takes up more lines.

If you're interested in seeing the other points, I invite you to sign up to my newsletter using the form below.