5 Beginner Testing Mistakes I Noticed While Working with Less Experienced Developers
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.
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 was completed as part of my Members only content, here's the full list of topics which I wrote about:
7. Testing implementation details instead of the outcome
8. Comparing whole complex data structures instead of only the tested fields
9. Not using Soft assertions
10. Repeating the SUT Algorithm in assertions instead of an explicit value
11. Not focusing on boundary values
12. Using "private" functions in tests
13. Too generic test case naming
14. Conditions in tests
15. Regular loops instead of Parameterized / Table Tests
16. Ignoring mutation testing and not verifying if the tests are covering all cases
17. Creating objects in every test instead of having object mothers
18. Not using explicit data structures in tests, but referencing previously created ones
19. Not caring about test code quality
20. Always adding tests into only one file
21. Ignoring test automation in the CI process
These parts are available here:
If you're interested and like have access to them subscribe to my newsletter or sign up right here on my website (No passwords required).
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.