android / codelab-kotlin-coroutines

Kotlin Coroutines codelab
Apache License 2.0
552 stars 268 forks source link

improve test whenRefreshTitleTimeout_throws #171

Open IvanSyabro opened 2 years ago

IvanSyabro commented 2 years ago

In 10 step of coroutines codelab https://developer.android.com/codelabs/kotlin-coroutines#9 user is expected to write unit test which checks for the timeout of TitleRepository.refreshTitle()

Current version looks next:

@Test(expected = TitleRefreshError::class)
    fun whenRefreshTitleTimeout_throws() = runBlockingTest {
        val network = MainNetworkCompletableFake()
        val subject = TitleRepository(
            network,
            TitleDaoFake("title")
        )
        launch {
            subject.refreshTitle()
        }
        advanceTimeBy(5_000)
    }

It's not clear why this test runs successfully, because if we change value in advanceTimeBy() to lower one, for example 4_000, test still completes successfully and throws TitleRefreshError as expected. From this behaviour it may seem that test doesn't work correctly and returns false negative result no matter of timeout value passed to advanceTimeBy().

I suggest to add line of code that actually adds value to network, besides, this method is already in the .zip archive example.

@Test(expected = TitleRefreshError::class)
    fun whenRefreshTitleTimeout_throws() = runBlockingTest {
        val network = MainNetworkCompletableFake()
        val subject = TitleRepository(
            network,
            TitleDaoFake("title")
        )
        launch {
            subject.refreshTitle()
        }
        advanceTimeBy(5_000)
        network.sendCompletionToAllCurrentRequests("value")
    }

And it that case if we change advanceTimeBy(5_000) to advanceTimeBy(4_000) test will fail. That will show that test really works and doesn't return false negatives

wuziq commented 1 year ago

With a smaller value for advanceTimeBy(), it's still not clear why the test doesn't fail on its own without the additional line you suggested. Isn't the coroutine running refreshTitle() incomplete when the test ends? Maybe I'm not clear on why your suggested line is needed.