Web: ensure Skiko runtime is initialized before JS browser tests start#5621
Conversation
configureWebApplication now wires JS test targets with generated Karma config that: - serves js-reexport-symbols.mjs, skiko.mjs, and skiko.wasm, - injects a loader before test bundle execution, - delays window.__karma__.loaded until js-reexport-symbols.mjs is imported and api.awaitSkiko is resolved. This removes Skiko init race in jsBrowserTest for projects without explicit app executable setup and prevents runtime failures like missing/invalid org_jetbrains_skia_* symbols.
There was a problem hiding this comment.
Pull request overview
This PR addresses a Kotlin/JS browser test flakiness/race by ensuring the Skiko runtime is initialized before Karma starts executing the test bundle, and adds an integration test project to prevent regressions (CMP-4906).
Changes:
- Added a generated Karma config fragment + loader injection to delay
window.__karma__.loaded()until Skiko init is complete for JS browser tests. - Added a new
misc/jsNoExecutableTeststest project that reproduces the “no executable configured” scenario for JS UI tests. - Added an integration test entry invoking
jsBrowserTestfor the new project.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/web/internal/configureWebApplication.kt | Adds Karma config generation and loader logic to wait for Skiko initialization before JS tests start. |
| gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/GradlePluginTest.kt | Adds an integration test that runs jsBrowserTest for the new reproduction project. |
| gradle-plugins/compose/src/test/test-projects/misc/jsNoExecutableTests/build.gradle | Introduces a minimal JS browser + Compose UI test project without an executable binary configuration. |
| gradle-plugins/compose/src/test/test-projects/misc/jsNoExecutableTests/settings.gradle | Adds Gradle settings (plugin management + repositories) for the new test project. |
| gradle-plugins/compose/src/test/test-projects/misc/jsNoExecutableTests/gradle.properties | Configures Kotlin daemon JVM args for the new test project. |
| gradle-plugins/compose/src/test/test-projects/misc/jsNoExecutableTests/src/commonMain/kotlin/Main.kt | Adds a small composable used by the test. |
| gradle-plugins/compose/src/test/test-projects/misc/jsNoExecutableTests/src/commonTest/kotlin/MainTest.kt | Adds a Compose UI test that validates rendering, exercising Skiko initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| testTask.dependsOn(compilationProcessResourcesTaskName) | ||
|
|
||
| val configDirectoryPath = configDir.get().asFile | ||
| (testTask.testFramework as? KotlinKarma)?.useConfigDirectory(configDirectoryPath) |
There was a problem hiding this comment.
Just in case, useConfigDirectory overrides the default directory. If a user has their own configuration in karma.config.d, as far as I understand it stops to work
There was a problem hiding this comment.
Added a copying configs from the default directory (if they exist). For a custom config dir we don't have an API to access it.
Reflect user-provided Karma configurations in the generated directory to preserve compatibility when overriding the default `karma.config.d`. Update Skiko test setup to prioritize serving dependencies in the correct order.
Shagen Ogandzhanian (Schahen)
left a comment
There was a problem hiding this comment.
My comments doesn't worth trying to implement quite honestly - as overall make sense as a temporary decision
| .then((mod) => mod?.api?.awaitSkiko || Promise.resolve()); | ||
| } | ||
| skikoReady.then(() => originalLoaded()).catch((error) => { | ||
| const message = error && error.stack ? error.stack : String(error); |
There was a problem hiding this comment.
the same - we can safely use null coalescing
|
|
||
| const hasLoader = files.some((entry) => | ||
| entry === loaderFile || | ||
| (entry && typeof entry === "object" && entry.pattern === loaderFile) |
There was a problem hiding this comment.
Not a big deal but === wont be a real benefit for anything
Ensure `js-reexport-symbols.mjs` is dynamically resolved from Karma-served files to avoid hardcoded paths.
Shagen Ogandzhanian (Schahen)
left a comment
There was a problem hiding this comment.
Ignore my very minor comments, as a temporary solution to achieve what we want looks absolutely justified
…r test setup (#5628) Ensure JS browser tests for Compose UI that depend on Skiko properly declare an executable binary. Fails with a clear error message if the configuration is invalid. Adds related test case (`testJsNoExecutableTests`). Reverted fix: #5621 Fixes https://youtrack.jetbrains.com/issue/CMP-4906. ## Testing Added new test ## Release Notes ### Fixes - Gradle Plugin - Add clear error message if JS browser tests configuration for Compose UI is invalid (doesn't have declared an executable binary).
configureWebApplicationnow wires JS test targets with generated Karma config that:js-reexport-symbols.mjs,skiko.mjs, andskiko.wasm,window.__karma__.loadeduntiljs-reexport-symbols.mjsis imported andapi.awaitSkikois resolved.This removes Skiko init race in jsBrowserTest for projects without explicit app executable setup and prevents runtime failures like missing/invalid
org_jetbrains_skia_*symbols.Fixes https://youtrack.jetbrains.com/issue/CMP-4906
Testing
Added test case for the issue
UPDATE
Reverted: #5628
Release Notes
N/A