spekframework / spek

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

Make discovery of scopes concurrent #906

Closed raniejade closed 4 years ago

raniejade commented 4 years ago

Apparently, Classgraph scanning is already done in parallel - so instead of allowing to disable it, I just added a property that allows how concurrent it can be (passing 1 will effectively make the scan synchronous). The biggest change in this PR is with how scopes are filtered, previously it was always synchronous. Scope filtering can now be done asynchronously, the implementation under the hood uses Kotlin's coroutines. To enable the new behaviour, you can set the spek2.discovery.concurrent property.

raniejade commented 4 years ago

// cc @arturbosch

raniejade commented 4 years ago

new build with this change is out: https://bintray.com/spekframework/spek-dev/spek2/2.0.13-alpha.0.2%2B19cf9da

arturbosch commented 4 years ago

new build with this change is out: https://bintray.com/spekframework/spek-dev/spek2/2.0.13-alpha.0.2%2B19cf9da

Unfortunately it crashes with a missing kotlin.time.MonoClock message:

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 1.
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:564)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
    at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
    at com.sun.proxy.$Proxy5.stop(Unknown Source)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.stop(TestWorker.java:133)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:564)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
    at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
    at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
    at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
    at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
    at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56)
    at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'spek2' failed to discover tests
    at org.junit.platform.launcher.core.DefaultLauncher.discoverEngineRoot(DefaultLauncher.java:189)
    at org.junit.platform.launcher.core.DefaultLauncher.discoverRoot(DefaultLauncher.java:168)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:132)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
    ... 25 more
Caused by: java.lang.NoClassDefFoundError: kotlin/time/MonoClock
    at org.spekframework.spek2.runtime.SpekRuntime.discover(SpekRuntime.kt:111)
    at org.spekframework.spek2.junit.SpekTestEngine.discover(SpekTestEngine.kt:92)
    at org.junit.platform.launcher.core.DefaultLauncher.discoverEngineRoot(DefaultLauncher.java:181)
    ... 31 more
Caused by: java.lang.ClassNotFoundException: kotlin.time.MonoClock
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:602)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    ... 34 more

the classgraph verbose is just for the alpha releases right? It spams the console :D

the console output hints that also the main sources are scanned for test discovery?

2020-08-01-104901_1910x832_scrot

Edit: linking https://github.com/spekframework/spek/pull/906

arturbosch commented 4 years ago

@raniejade Spek is already using Kotlin 1.4 ? If yes the MonoClock typealias was removed ?

raniejade commented 4 years ago

the classgraph verbose is just for the alpha releases right? It spams the console :D

My mistake, I was supposed to remove that. I'll fix it.

I'll work on a fix in a few minutes.

raniejade commented 4 years ago

I can't upgrade to 1.4 in the 2.0.x branch, I'll just make sure that I don't use classes that were removed in 1.4. Spek is still at 1.3.7x.

arturbosch commented 4 years ago

I can't upgrade to 1.4 in the 2.0.x branch, I'll just make sure that I don't use classes that were removed in 1.4. Spek is still at 1.3.7x.

That's strange. If both detekt and spek uses Kotlin 1.3.72, it shouldn't crash with missing class ...

raniejade commented 4 years ago

That is indeed weird, hmmm. Maybe a classpath conflicts (i.e duplicate class)?

arturbosch commented 4 years ago

That is indeed weird, hmmm. Maybe a classpath conflicts (i.e duplicate class)?

We have a constraint on 1.3.72 so running gradle dependencies for testRuntimeClasspath upgrades the Kotlin 1.3.61 dependencies from spek 2.0.12 and kotlinx-coroutines-core to 1.3.72.