Skip to content

Conversation

aSemy
Copy link
Contributor

@aSemy aSemy commented Jun 16, 2024

- Convert timeout assertions to common Kotlin
- Deprecate the JVM only versions
- Update tests to use virtual time
@aSemy aSemy mentioned this pull request Jun 17, 2024
20 tasks
aSemy added 3 commits June 18, 2024 00:30
…lete

# Conflicts:
#	kotest-assertions/kotest-assertions-core/src/jvmMain/kotlin/io/kotest/matchers/concurrent/suspension/concurrent.kt
@aSemy
Copy link
Contributor Author

aSemy commented Jun 17, 2024

Hmmmm JVM tests work, but WasmJS fails

./gradlew :kotest-assertions:kotest-assertions-core:wasmJsTest --stacktrace

[...]

Disconnected (0 times) , because no message in 30000 ms.
java.lang.IllegalStateException: command '/Users/dev/.local/state/gradle/nodejs/node-v22.0.0-darwin-arm64/bin/node' exited with errors (exit code: 1)

> Task :kotest-assertions:kotest-assertions-core:allTests FAILED

@OliverO2
Copy link
Contributor

Seems like wasmJsBrowserTest fails with a single test exceeding the (presumably karma) timeout of 30 seconds. I could take a look cannot promise results today.

@aSemy
Copy link
Contributor Author

aSemy commented Jun 19, 2024

@aSemy
Copy link
Contributor Author

aSemy commented Jun 19, 2024

I got some help and we figured out the problem: the Kotest Gradle Plugin isn't applied to Kotest projects, so the tests can't be discovered or launched.

@OliverO2
Copy link
Contributor

Why does this lead to the timeout? I'd expect it the test run to finish on wasmJsBrowserTest without reporting any test. I'll try to investigate,

@OliverO2
Copy link
Contributor

OliverO2 commented Jun 19, 2024

The test times out because the dependency implementation(projects.kotestFramework.kotestFrameworkEngine) is missing in commonTest. For Wasm, the function startUnitTests is missing in that case:

// The JS launcher set up by the Kotlin Gradle plugin calls this function in addition to `main()`.
@OptIn(ExperimentalJsExport::class)
@JsExport
internal fun startUnitTests() {
// The Kotlin compiler would insert test invocations here for kotlin-test.
// This mechanism is not used with Kotest.
}

The Kotlin test framework crashes as it tries to call the function on the headless browser before it could launch the test framework. Karma loses its patience and complains after 30 seconds without any response.

@aSemy
Copy link
Contributor Author

aSemy commented Jun 20, 2024

Thanks @OliverO2! However, the tests need testCoroutineScheduler, and I think this isn't available on Wasm yet? #4077

@OliverO2
Copy link
Contributor

That's correct, any JS platform test (including wasmJs) would fail if it requires a runTest invocation.

Kotest effectively does not fully self-test on non-JVM targets anyway due to the missing Kotest compiler plugin. While it appears to run non-JVM tests, without the plugin in place, test discovery actually does not take place.

This could be corrected, but would require our build scripts to use the build's own version of the Kotest compiler plugin. A previous compiler plugin version from a repo could be chosen, but this would be unstable in case of incompatible changes in the code generated by the plugin.

@aSemy aSemy marked this pull request as ready for review June 21, 2024 15:36
@aSemy aSemy requested review from sksamuel, Kantis and OliverO2 June 23, 2024 17:38
Copy link
Member

@Kantis Kantis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but added a couple of questions for things I don't understand

@aSemy aSemy marked this pull request as draft July 19, 2024 07:51
@aSemy aSemy marked this pull request as ready for review July 19, 2024 09:51
@aSemy aSemy requested a review from Kantis July 19, 2024 09:51
Copy link
Contributor

@OliverO2 OliverO2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@aSemy aSemy added this pull request to the merge queue Jul 19, 2024
Merged via the queue into master with commit ee86a33 Jul 19, 2024
7 checks passed
@aSemy aSemy deleted the adam/feat/commonize-should-complete branch July 19, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants