The original article was published on September 5, 2024.
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.
"Architecture" under construction
Let's assume you are creating an application for sending and receiving messages. The data model for the messages to be sent and received is defined as follows.
class MessageModel internal constructor(
val messageId: String,
val sender: UserId,
val contentModel: MessageContentModel
)
sealed interface MessageContentModel {
class Text(
val messageText: String
): MessageContentModel
class Image(
val imageUri: Uri,
...
): MessageContentModel
...
class ExternalResource(
val resourceId: ...,
...
): MessageContentModel
}As indicated by MessageContentModel, there are several types of messages. In this case, the type is determined by the first character of messageId. (Furthermore, assume that the specification of this messageId cannot be changed due to historical reasons.)
- "t": Text
- "i": Image
- ...
- "e": External resource
For example, since the first character of the ID "t02af6d450d056" is "t", this is the ID of a text message.
Let's assume that the creation logic for instances of ExternalResource is completely different from other MessageContentModel instances. In that case, the class providing MessageModel (...Dao) may also be implemented separately for ExternalResource and others.
class MainMessageDao {
fun queryMessageModel(messageId: String): MessageModel? { ... }
}
class ExternalResourceMessageDao {
fun queryMessageModel(messageId: String): MessageModel? { ... }
}If there is no message corresponding to messageId, the function queryMessageModel returns null. Therefore, passing the ID of a text message to ExternalResourceMessageModelDao.queryMessageModel will return null.
Using MainMessageDao and ExternalResourceMessageDao according to the message type is cumbersome and can cause bugs. Therefore, we created a class called MessageModelRepository that has the role of selecting the correct ...Dao as follows. Here, the supports function of each ...Dao returns a Boolean indicating whether it corresponds to the given messageId.
class MessageModelRepository(
private val mainMessageDao: MainMessageDao,
private val externalResourceMessageDao: ExternalResourceMessageDao,
...
) {
fun queryMessageModel(messageId: String): MessageModel? {
... // other code
val dao = when {
MainMessageDao.supports(messageId) -> mainMessageDao
ExternalResourceMessageDao.supports(messageId) -> externalResourceMessageDao
else -> null
}
...
return dao?.queryMessageModel(messageId)
}
companion object {
fun isMainMessage(messageId: String): Boolean = ...
fun isExternalResourceMessage(messageId: String): Boolean = ...
}
}By doing this, the side using MessageModelRepository can obtain MessageModel without being aware of the existence of ...Dao.
Below is the test code for MessageModelRepository.
class MessageModelRepositoryTest {
@get:Rule
val mockitoRule: MockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS)
private val mockMainMessageDao: MainMessageDao = mock()
private val mockExternalResourceMessageDao: ExternalResourceMessageDao = mock()
private lateinit var subject: MessageModelRepository
@Before
fun setUp() {
... // Setup code
subject = MessageModelRepository(
mockMainMessageDao,
mockExternalResourceMessageDao,
...,
)
}
@Test
fun queryMessageModel_returnsNull_forMalformedId() {
val nullMessage = subject.queryMessageModel(MALFORMED_MESSAGE_ID)
assertNull(nullMessage)
verify(mockMainMessageDao, never()).queryMessageModel(any())
verify(mockExternalResourceMessageDao, never()).queryMessageModel(any())
}
@Test
fun queryMessageModel_returnsModelOfMainMessage() {
mockMainMessageDao.stub {
on { queryMessageModel(TEXT_MESSAGE_ID) } doReturn
TestMessageModels.TEXT
on { queryMessageModel(IMAGE_MESSAGE_ID) } doReturn
TestMessageModels.IMAGE
...
}
assertIs(subject.queryMessageModel(TEXT_MESSAGE_ID))
assertIs(subject.queryMessageModel(IMAGE_MESSAGE_ID))
...
}
@Test
fun queryMessageModel_returnsModelOfExternalResource() {
mockExternalResourceMessageDao.stub {
on { queryMessageModel(EXTERNAL_RESOURCE_MESSAGE_ID) } doReturn
TestMessageModels.EXTERNAL_RESOURCE
}
val model = subject.queryMessageModel(EXTERNAL_RESOURCE_MESSAGE_ID)
assertIs(model)
}
companion object {
private const val MALFORMED_MESSAGE_ID = "x123456"
private const val TEXT_MESSAGE_ID = "t123456"
...
}
}This test confirms the following behaviors are correct.
- If the format of the message ID is incorrect, null is returned and no DAO is used.
- For types such as text and image, only
MainMessageDaois used (guaranteed byStrictness.STRICT_STUBS). - For the external resource type, only
ExternalResourceMessageDaois used (guaranteed byStrictness.STRICT_STUBS).
This test covers all cases of val dao = when {. What else should be tested in addition to this?
Tests dedicated to construction
The above test is sufficient to prove that "the current code is correct". However, it cannot detect bugs due to future changes. When a new MessageContentModel and its corresponding DAO are added, it is possible to forget to update MessageModelRepository. In that case, the new type of ID will simply be treated as an invalid ID, and the bug of implementation omission may be overlooked. For example, if MessageContentModel.Foo and FooMessageDao are added and the update of queryMessageModel is forgotten, it will become a bug, but it is difficult to notice it at runtime.
To prevent this, it is good to compare the "list of tested MessageContentModel" and the "list of defined MessageContentModel" as follows.
...
@Test
fun queryMessageModel_returnsModelOfMainMessage() {
mockMainMessageDao.stub {
on { queryMessageModel(TEXT_TEST_DATA.id) } doReturn
TestMessageModels.TEXT
on { queryMessageModel(IMAGE_TEST_DATA.id) } doReturn
TestMessageModels.IMAGE
...
}
assertTypeOf(TEXT_TEST_DATA.type, subject.queryMessageModel(TEXT_TEST_DATA.id))
assertTypeOf(IMAGE_TEST_DATA.type, subject.queryMessageModel(IMAGE_TEST_DATA.id))
...
}
@Test
fun queryMessageModel_returnsModelOfExternalResource() {
mockExternalResourceMessageDao.stub {
on { queryMessageModel(EXTERNAL_RESOURCE_TEST_DATA.id) } doReturn
TestMessageModels.EXTERNAL_RESOURCE
}
val model = subject.queryMessageModel(EXTERNAL_RESOURCE_TEST_DATA.id)
assertTypeOf(EXTERNAL_RESOURCE_TEST_DATA.type, model)
}
@Test
fun ensureCompletenessOfMessageContentModel() {
val knownTypes: Set<KClass> = setOf(
TEXT_TEST_DATA.type,
IMAGE_TEST_DATA.type,
...,
EXTERNAL_RESOURCE_TEST_DATA.type
)
ContentModel::class.sealedSubclasses.forEach { type ->
assertTrue(
type in knownTypes,
"Unknown type: $type. Update [MessageModelRepository] to cover it."
)
}
}
private class TestData(val id: String, val type: KClass)
companion object {
@Suppress("UNUSED_PARAMETER") // `type` is to specify `T` without a type parameter.
private inline fun assertTypeOf(type: KClass, actual: Any?) =
assertTrue(actual is T)
private val TEXT_TEST_DATA = TestData("t123456", MessageContentModel.Text::class)
private val IMAGE_TEST_DATA = TestData("i123456", MessageContentModel.Image::class)
...
private val EXTERNAL_RESOURCE_TEST_DATA = TestData("e123456", MessageContentModel.ExternalResource::class)
}By doing this, when a new type is added, the test will fail with the message "You must also update MessageModelRepository", so the developer will know what to do and can prevent implementation omissions.
In a nutshell
Tests should be able to accommodate not only the current code but also future changes.
Keywords: test, completeness, specification update