16. Ignoring mutation testing and not verifying if the tests are covering all cases
Sometimes when writing complex code, you want to make sure your tests cover every case. To do that, you follow the logic and verify that a test for this exists. This is good, but there is one additional thing you can do to make sure everything is tested: Mutation Testing.
To perform Mutation Testing, all you need to do, is comment out some line or change some condition and then run your tests. If some test fails, then that logic / condition is tested. If everything passes, then it probably means that a test is missing for that chunk of code.
Of course, don't go over board with this and check every line, you should probably focus on parts:
- That you're not sure are tested correctly
- Complex logic that is hard to follow
- Really important parts of the system
Applying Mutation testing will give you more confidence, that the SUT is correctly tested and that any regression will be caught by your Automated test suite.
17. Creating objects in every test instead of having object mothers
This topic is point borrows from the helper functions section. Basically, instead of repeating the same instantiation / creation logic every time, you could create a helper function which creates it for you.
These object mothers can create the SUT and its dependencies or data structures which the SUT uses. This way you increase re-usability and protect from code changes.
Such helper functions also allow you to use default values, making the tests only pass in relevant parameters for that test, which makes them shorter and more explicit.
Here we have a class which we would like to test:
class SomeClass(
val database: Database,
val timestampProvider: TimestampProvider,
val someConverter: SomeConverter,
) {
// ...
}And an object mother for it:
fun createSomeClass(
database: Database = FakeDatabase(),
timestampProvider: TimestampProvider = FakeTimestampProvider(),
someConverter: SomeConverter = SomeConverter(timestampProvider),
): SomeClass {
return SomeClass(database, timestampProvider, someConverter)
}This object mother provides for us with some default values, which we won't have to provide when using it.
@Before
fun setUp() {
systemUnderTest = createSomeClass(fakeDatabase, fakeTimestampProvider)
}
@Test
fun hasCorrectInitialValue() {
assertEquals(true, systemUnderTest.isInitialized)
}In most cases, you will probably want to pass in some dependencies from the test, so they can be verified. However, implementation details like "SomeConverter" can be ignored, because the object mother will pass it in.
These types of object mother come in handy when you have to create this class in multiple tests, or you want to test instantiation logic by re-creating the SUT inside a test case.
This type of function can also be used for data structures:
data class AwkwardDataStructure(
val valueA: OtherStructure,
val valueB: OtherStructure,
val valueC: OtherStructure
)
data class OtherStructure(
val id: String,
val type: String,
)
fun createAwkwardDataStructure(
valueA: OtherStructure = OtherStructure("", ""),
valueB: OtherStructure = OtherStructure("", ""),
valueC: OtherStructure = OtherStructure("", ""),
): AwkwardDataStructure
return AwkwardDataStructure(valueA, valueB, valueC)
}Thanks to this function, we don't need to repeat so much boilerplate every time we create the awkward data structure. If a test only focuses on valueA, we just pass in valueA the other ones use the default values. This makes the test easier to understand, because we know to only focus on this one value.
18. Not using explicit data structures in tests, but referencing previously created ones
Tests should explicitly tell you what they're focusing on, without having to hunt down the answer why the test behaved the way it did. Hiding the important values in a global variable will make the test harder to understand.
For examples sake, we have an SUT that takes in an Article Schema and based on its enum type returns a different article:
enum class ArticleType {
Text,
Image,
}
data class ArticleSchema(
val headline: String,
val content: String,
val type: ArticleType,
val imageUrls: List<String>?,
)
data class TextArticle(
val headline: String,
val content: String,
)
data class ImageArticle(
val headline: String,
val content: String,
val imageUrls: List<String>,
)The image schema property is nullable, so there might be a case, where we want an image article, but we don't have an image, then it should be converted to a normal text one.
Here's a test which has all the data structures prepared beforehand:
private val TEXT_ARTICLE_SCHEMA =
ArticleSchema("Text", "Content", ArticleType.Text, null)
private val IMAGE_ARTICLE_SCHEMA =
ArticleSchema("Image", "Content", ArticleType.Image, listOf("url"))
private val IMAGE_ARTICLE_WITHOUT_URLS_SCHEMA =
ArticleSchema("Image", "Content", ArticleType.Image, null)
private val TEXT_ARTICLE =
TextArticle("Text", "Content")
private val IMAGE_ARTICLE =
ImageArticle("Image", "Content", listOf("url"))
private val TEXT_ARTICLE_FROM_IMAGE_ARTICLE =
TextArticle("Image", "Content")
@Test
fun `Article Schema with text type results in TextArticle`() {
val result = systemUnderTest.convert(TEXT_ARTICLE_SCHEMA)
assertEquals(result, TEXT_ARTICLE)
}
@Test
fun `Article Schema with image type results in ImageArticle`() {
val result = systemUnderTest.convert(IMAGE_ARTICLE_SCHEMA)
assertEquals(result, IMAGE_ARTICLE)
}
@Test
fun `Article Schema with image type but no urls results in TextArticle`() {
val result = systemUnderTest.convert(IMAGE_ARTICLE_WITHOUT_URLS_SCHEMA)
assertEquals(result, TEXT_ARTICLE_FROM_IMAGE_ARTICLE)
}Using the already defined data structure in every test makes the tests shorter, however in order to understand them we have to look at the data structure definitions. Additionally, if we have an edge case (like image without URLs) we need to also hunt down what variable which is responsible for that behavior.
A better approach would be to create the data structure in the test and only specify what is relevant for that test:
@Test
fun `Article Schema with text type results in TextArticle`() {
val schema = createSchema("Text", "Content", ArticleType.Text)
val result = systemUnderTest.convert(schema)
assertEquals(result, TextArticle("Text", "Content"))
}
@Test
fun `Article Schema with image type results in ImageArticle`() {
val schema = createSchema("Image", "Content", ArticleType.Image, listOf("url"))
val result = systemUnderTest.convert(schema)
assertEquals(result, ImageArticle("Image", "Content", listOf("url")))
}
@Test
fun `Article Schema with image type but no urls results in TextArticle`() {
val schema = createSchema("Image", "Content", ArticleType.Image, null)
val result = systemUnderTest.convert(schema)
assertEquals(result, TextArticle("Image", "Content"))
}
private fun createSchema(
headline: String,
content: String,
type: ArticleType,
imageUrls: List<String>? = null,
): ArticleSchema {
return ArticleSchema(headline, content, type, imageUrls)
}This makes the tests a little longer, but a lot more straight forward. To help with repetitions, a parameterized test case could be created and to improve the readability additional string values could be extracted and maybe even more helper functions.
Generally, the more relevant information we have in the test body, the easier it should be to understand what's going on. However, too much information will obfuscate it, try to find a middle ground of verbosity and readability.
19. Not caring about test code quality
A lot of the times, test code is neglected, which makes it "rot", with time such rot will make the tests hard to change in order to fit the production code. Failing tests will be removed, and new tests won't be added. This causes our automated test suite to be a lot less reliable, and with time it can cause us a lot of issues.
To avoid this, it is important that you clean up / refactor your test code from time to time just as you would production code. Clean tests are easy to fix, understand and maintain, which in the long run speeds up our development because regressions are caught a lot faster.
Just like with writing tests, it is an up-front investment, because writing tests is extra code. In the long run, these tests are a lot better at caching errors than humans are. They be executed in a fraction of what manual testing can achieve.
Keep your tests clean because:
- hard to understand tests will be hard to fix
- hard to fix tests will be probably removed
- without tests, more and more bugs will land in production
- fixing these bugs will slow down development
20. Always adding tests into only one file
Sometimes a class starts small, but more and more logic is added to it. If the class is not split up, or it doesn't make sense to do so, then tests will grow with it, but in a much faster rate.
This will result in a test file that has over 500 lines of code, which might seem good, because all edge cases are covered. However, big files are hard to reason about, they might have a lot of set-up or helper functions which are only used in a portion of tests.
To help with this, instead of having one a 1:1 relation for test file and SUT, you can split out the test classes and group them by relevant logic. If some group of tests only focus on one behavior, then it's a good candidate for a separate file.
Take for example an SUT which manages comments with the following data structure:
data class Comment(
val text: String,
val postDateIso: String = "",
val upVotes: Int = 0,
val downVotes: Int = 0,
val wasReported: Boolean = false,
)The system can vote, report, sort and also parses the text from HTML. The test could look like this:
class CommentsTest {
@Test fun `Comment content is correctly parsed from HTML`() { /**/ }
@Test fun `Up voting a neutral comment increases upVotes`() { /**/ }
@Test fun `Up voting an up voted comment does not update votes`() { /**/ }
@Test fun `Up voting a down voted comment reduces downVotes and increases upVotes`() { /**/ }
@Test fun `Down voting a neutral comment increases upVotes`() { /**/ }
@Test fun `Down voting an down voted comment does not update votes`() { /**/ }
@Test fun `Down voting an up voted comment reduces upVotes and increases downVotes`() { /**/ }
@Test fun `Initially comment is not reported`() { /**/ }
@Test fun `Reporting comment sets the correct flag`() { /**/ }
@Test fun `Sorting comments by newest works correctly`() { /**/ }
@Test fun `Sorting comments by oldest works correctly`() { /**/ }
@Test fun `Sorting comments by up votes works correctly`() { /**/ }
@Test fun `Sorting comments by down votes works correctly`() { /**/ }
// more tests....
private fun setUpParsing() { /**/ }
private fun setUpVoting() { /**/ }
private fun setUpReporting() { /**/ }
private fun setUpSorting() { /**/ }
}That's a lot of tests for a lot of edge cases, just by looking at their names we can see that they are grouped by functionality however they are still in one file which can grow exponentially and become really hard to manage.
A better approach would be to extract these tests to specific test files:
class CommentsVotingTest {
@Test fun `Up voting a neutral comment increases upVotes`() { /**/ }
@Test fun `Up voting an up voted comment does not update votes`() { /**/ }
@Test fun `Up voting a down voted comment reduces downVotes and increases upVotes`() { /**/ }
@Test fun `Down voting a neutral comment increases upVotes`() { /**/ }
@Test fun `Down voting an down voted comment does not update votes`() { /**/ }
@Test fun `Down voting an up voted comment reduces upVotes and increases downVotes`() { /**/ }
private fun setUpVoting() { /**/ }
}
class CommentsSortingTest {
@Test fun `Sorting comments by newest works correctly`() { /**/ }
@Test fun `Sorting comments by oldest works correctly`() { /**/ }
@Test fun `Sorting comments by up votes works correctly`() { /**/ }
@Test fun `Sorting comments by down votes works correctly`() { /**/ }
private fun setUpSorting() { /**/ }
}
class CommentsMiscellaneousTest {
@Test fun `Comment content is correctly parsed from HTML`() { /**/ }
@Test fun `Initially comment is not reported`() { /**/ }
@Test fun `Reporting comment sets the correct flag`() { /**/ }
// more tests....
private fun setUpParsing() { /**/ }
private fun setUpReporting() { /**/ }
}This way, all the behavior specific set-up or helper functions are only present in that test file, which makes the other tests files smaller and easier to reason about. For behaviors which have only a small amount of tests, and they can't be sensibly grouped, they could stay in the original file which I usually name a MiscellaneousTest.
If you want to change something with sorting, you will probably only need to look into the Sorting test, which is smaller, and only focused on that behavior. The only problem with this approach is the tests, which are in the "Miscellaneous" are harder to find. However, this is only an issue, because the other tests are so nicely grouped.
* An SUT object mother should be used for these test files to avoid repeating the same boilerplate.
21. Ignoring test automation in the CI process
Sometimes we want to introduce a quick change to our project and forget to run our test suit. If the PR is approved, and we don't have any test automation, we will probably merge it. Sometimes these quick changes introduce bugs, and without automation it is really easy to introduce bugs to our main branch.
Tests should be run as often as possible to ensure that nothing is broken, we can't only rely on our human memory to remember about them. They should run automatically on every change we want to introduce to our codebase.
This is where a Code Review CI/CD process comes in handy, anytime we want to merge something to the main branch, the CI will run our tests and if nothing breaks it will allow us to merge the change. However, if we introduce a regression, the CI will let us know and hopefully block us from merging until we fix the issue.
The CI is an important tool in modern development, it helps us with eliminating the human error (i.e. forgetfulness) part of software development.
