spekframework / spek

A specification framework for Kotlin
Other
2.23k stars 179 forks source link

Many tests suddenly fail with Spek 2.0.13 #923

Closed svenjacobs closed 4 years ago

svenjacobs commented 4 years ago

Starting with Spek 2.0.13 suddenly many tests fail that have not been changed in any way. Just Spek was updated. The error messages are something like:

io.mockk.MockKException: Bad recording sequence. State: AnsweringState
    at io.mockk.impl.recording.states.CallRecordingState.cancelAndThrowBadRecordingState(CallRecordingState.kt:32)
    at io.mockk.impl.recording.states.CallRecordingState.estimateCallRounds(CallRecordingState.kt:20)
    at io.mockk.impl.recording.CommonCallRecorder.estimateCallRounds(CommonCallRecorder.kt:55)
    at io.mockk.impl.eval.RecordedBlockEvaluator.record(RecordedBlockEvaluator.kt:46)
    at io.mockk.impl.eval.EveryBlockEvaluator.every(EveryBlockEvaluator.kt:30)
    at io.mockk.MockKDsl.internalCoEvery(API.kt:98)
    at io.mockk.MockKKt.coEvery(MockK.kt:116)
    at de.penny.core.domain.banner.usecase.ObserveActiveBannerUseCaseTest$1$fetchBannersUseCase$2.invoke(ObserveActiveBannerUseCaseTest.kt:23)
    at de.penny.core.domain.banner.usecase.ObserveActiveBannerUseCaseTest$1$fetchBannersUseCase$2.invoke(ObserveActiveBannerUseCaseTest.kt:19)
    at org.spekframework.spek2.runtime.lifecycle.MemoizedValueAdapter.get(MemoizedValueAdapter.kt:28)
    at org.spekframework.spek2.runtime.lifecycle.MemoizedValueAdapter.getValue(MemoizedValueAdapter.kt:22)
    at de.penny.core.domain.banner.usecase.ObserveActiveBannerUseCaseTest$1$1$1$3.invoke(ObserveActiveBannerUseCaseTest.kt:74)
    at de.penny.core.domain.banner.usecase.ObserveActiveBannerUseCaseTest$1$1$1$3.invoke(ObserveActiveBannerUseCaseTest.kt:19)
    at org.spekframework.spek2.runtime.scope.TestScopeImpl.execute(scopes.kt:136)
    at org.spekframework.spek2.runtime.Executor$execute$result$2$exception$1$job$1.invokeSuspend(Executor.kt:82)
    at ???(Coroutine boundary.?(?)
    at org.spekframework.spek2.runtime.Executor$execute$result$2.invokeSuspend(Executor.kt:78)
    at org.spekframework.spek2.runtime.Executor.executeSafely(Executor.kt:119)
    at org.spekframework.spek2.runtime.Executor.execute(Executor.kt:56)
    at org.spekframework.spek2.runtime.Executor$execute$result$2$1.invokeSuspend(Executor.kt:70)
    at org.spekframework.spek2.runtime.Executor$execute$result$2.invokeSuspend(Executor.kt:59)
    at org.spekframework.spek2.runtime.Executor.executeSafely(Executor.kt:119)
    at org.spekframework.spek2.runtime.Executor.execute(Executor.kt:56)
    at org.spekframework.spek2.runtime.Executor$execute$result$2$1.invokeSuspend(Executor.kt:70)
    at org.spekframework.spek2.runtime.Executor$execute$result$2.invokeSuspend(Executor.kt:59)
    at org.spekframework.spek2.runtime.Executor.executeSafely(Executor.kt:119)
    at org.spekframework.spek2.runtime.Executor.execute(Executor.kt:56)
    at org.spekframework.spek2.runtime.Executor$execute$result$2$1.invokeSuspend(Executor.kt:70)
    at org.spekframework.spek2.runtime.Executor$execute$result$2.invokeSuspend(Executor.kt:59)
    at org.spekframework.spek2.runtime.Executor.executeSafely(Executor.kt:119)
    at org.spekframework.spek2.runtime.Executor.execute(Executor.kt:56)

MockK version 1.10.0. This is the same MockK version that was used with Spek 2.0.12 where tests still work.

raniejade commented 4 years ago

Thanks for the report @svenjacobs. Are you able to provide a reproducer? My guess is that mockk doesn't play well with coroutines.

svenjacobs commented 4 years ago

My guess is that mockk doesn't play well with coroutines.

Did something change between 2.0.12 and 2.0.13 regarding coroutines? The release doesn't mention any major change...

MockK and coroutines work fine with 2.0.12. Tests have not been changed.

raniejade commented 4 years ago

I think one change is that, discovery is now done within a coroutine to support asynchronous discovery: https://github.com/spekframework/spek/blob/2.0.x/spek-runtime/src/commonMain/kotlin/org/spekframework/spek2/runtime/SpekRuntime.kt#L48. But it is turned off by default, a coroutine is started but it is effectively single threaded and using the same thread as before (in 2.0.12).

Can you give me a sample that fails?

svenjacobs commented 4 years ago

Here's a simple test that fails. The problematic line is 19 (coEvery):

class StringProvider {
    suspend fun world() = "world"
}

class Example(
    private val stringProvider: StringProvider,
) {

    suspend fun hello(): String {
        delay(250)
        return stringProvider.world()
    }
}

object ExampleTest : Spek(
    {
        val stringProvider by memoized {
            mockk<StringProvider> {
                coEvery { world() } returns "world 2"
            }
        }
        val example by memoized {
            Example(stringProvider)
        }

        describe("ExampleTest") {

            it("should work ;)") {
                runBlockingTest {
                    example.hello() shouldBeEqualTo "world 2"
                }

                coVerify {
                    stringProvider.world()
                }
            }
        }
    }
)

Spek 2.0.13 MockK 1.10.0 Kluent 1.61

svenjacobs commented 4 years ago

Interestingly the tests do not fail when run individually from IDE but fail when run with ./gradlew test.

raniejade commented 4 years ago

A few things to test out (I'll setup a project myself later, can't right now):

  1. What's the behaviour when adding org.gradle.parallel=false to gradle.properties.
  2. What's the behaviour when you move coEvery { world() } returns "world 2" to a beforeEachTest.
svenjacobs commented 4 years ago

In both cases tests unfortunately still fail :(

raniejade commented 4 years ago

Weird, hmm. I'll have to investigate some more. Can you wrap stringProvider.world() in a runBlockingTest?

coVerify {
  runBlockingTest { stringProvider.world() }
}

The idea here is to call stringProvider.world() using the same dispatcher.

svenjacobs commented 4 years ago

Neither wrapping coVerify nor coEvery in runBlockingTest did fix this.

raniejade commented 4 years ago

Should be resolved in #925.

svenjacobs commented 4 years ago

Any chance of a new release with this bugfix soon? 😃

raniejade commented 4 years ago

Maybe in two weeks time, I’m really having trouble finding time for my personal projects at the moment. 😢

raniejade commented 4 years ago

@svenjacobs Can you try this build out? https://bintray.com/spekframework/spek-dev/spek2/2.0.14-alpha.0.4%2B88d54d2 Hopefully it fixes the issue :crossed_fingers:

svenjacobs commented 4 years ago

@svenjacobs Can you try this build out? https://bintray.com/spekframework/spek-dev/spek2/2.0.14-alpha.0.4%2B88d54d2 Hopefully it fixes the issue 🤞

Yes, tests work with this version 👍 I just compared it to 2.0.13 were tests fail.

raniejade commented 4 years ago

Awesome, I'll start the release process sometime this week when I get the time. Thanks for checking!

raniejade commented 4 years ago

@svenjacobs sorry for the delay, 2.0.14 should be out now.