Skip to content
This repository has been archived by the owner on Aug 16, 2023. It is now read-only.

improve test whenRefreshTitleTimeout_throws #171

Open
IvanSyabro opened this issue May 19, 2022 · 1 comment
Open

improve test whenRefreshTitleTimeout_throws #171

IvanSyabro opened this issue May 19, 2022 · 1 comment

Comments

@IvanSyabro
Copy link

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
Copy link

wuziq commented Feb 21, 2023

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants