Skip to content

Commit 3ac4337

Browse files
colinrtwhitecolinrtwhite
authored
Attempt to make JS and WASM tests less flaky. (#3310)
1 parent 2fd917a commit 3ac4337

File tree

6 files changed

+167
-25
lines changed

6 files changed

+167
-25
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ jobs:
5555
cache-cleanup: true
5656

5757
- name: Unit tests
58-
run: ./gradlew allTests testDebugUnitTest validateDebugScreenshotTest verifyPaparazziDebug verifyRoborazziAndroidHostTest verifyRoborazziJvm
58+
run: ./test.sh --skip-checks --skip-instrumentation-tests
5959

6060
unit-tests-macos:
6161
name: Unit tests (macOS)

‎buildSrc/src/main/kotlin/coil3/multiplatform.kt‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,23 @@ fun Project.addAllMultiplatformTargets(
1717
plugins.withId("org.jetbrains.kotlin.multiplatform") {
1818
extensions.configure<KotlinMultiplatformExtension> {
1919
applyCoilHierarchyTemplate()
20+
val sharedKarmaConfigDirectory = rootProject.projectDir.resolve("karma.config.d")
2021

2122
jvm()
2223

2324
js {
2425
browser {
2526
testTask {
26-
useKarma { useChromeHeadless() }
27+
useKarma {
28+
useChromeHeadless()
29+
useConfigDirectory(sharedKarmaConfigDirectory)
30+
}
2731
}
2832
}
2933
nodejs {
3034
testTask {
35+
// Compose Multiplatform is not supported on nodejs.
3136
enabled = false
32-
useMocha {
33-
timeout = "60s"
34-
}
3537
}
3638
}
3739
binaries.executable()
@@ -45,33 +47,40 @@ fun Project.addAllMultiplatformTargets(
4547
.buildDirectory
4648
.dir("wasm/packages/coil-root${path.replace(":", "-")}-test/kotlin")
4749
.map { it.asFile }
50+
val wasmTestModuleProvider = skikoDirProvider.map {
51+
it.resolve("coil-root${path.replace(":", "-")}-test.uninstantiated.mjs")
52+
}
4853

4954
browser {
5055
testTask {
51-
useKarma { useChromeHeadless() }
52-
}
53-
}
54-
nodejs {
55-
testTask {
56-
enabled = false
56+
useKarma {
57+
useChromeHeadless()
58+
useConfigDirectory(sharedKarmaConfigDirectory)
59+
}
5760
doFirst {
58-
val skikoModule = skikoDirProvider.get().resolve("skiko.mjs")
59-
if (skikoModule.isFile) {
60-
// The generated Skiko module disables its Node-specific loader
61-
// with `if (false)`. Rewrite it so the Node path runs and the
62-
// tests can load `skiko.wasm`.
63-
val original = skikoModule.readText()
64-
val patched = original.replaceFirst(
65-
"if (false) {",
66-
"if (ENVIRONMENT_IS_NODE) {"
61+
val wasmTestModule = wasmTestModuleProvider.get()
62+
if (wasmTestModule.isFile) {
63+
// Kotlin/Wasm checks `process.release.name` to detect Node.js.
64+
// When webpack injects a browser `process` shim without `release`,
65+
// this throws and leaves Karma browser tests stuck idle.
66+
val original = wasmTestModule.readText()
67+
val patched = original.replace(
68+
"(process.release.name === 'node')",
69+
"(process.release && process.release.name === 'node')",
6770
)
6871
if (patched != original) {
69-
skikoModule.writeText(patched)
72+
wasmTestModule.writeText(patched)
7073
}
7174
}
7275
}
7376
}
7477
}
78+
nodejs {
79+
testTask {
80+
// Compose Multiplatform is not supported on nodejs.
81+
enabled = false
82+
}
83+
}
7584
binaries.executable()
7685
binaries.library()
7786
}

‎karma.browser.d/00-disable-wasm-streaming.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Chrome occasionally stalls while compiling large wasm modules with
2+
// instantiateStreaming during Karma runs. Compile from ArrayBuffer instead.
3+
(function patchInstantiateStreaming() {
4+
if (typeof WebAssembly === "undefined") return;
5+
if (typeof WebAssembly.instantiate !== "function") return;
6+
if (typeof WebAssembly.instantiateStreaming !== "function") return;
7+
8+
const instantiate = WebAssembly.instantiate.bind(WebAssembly);
9+
10+
WebAssembly.instantiateStreaming = async function instantiateStreamingPatched(
11+
source,
12+
imports,
13+
...rest
14+
) {
15+
const response = await Promise.resolve(source);
16+
const bytes = await response.arrayBuffer();
17+
return instantiate(bytes, imports, ...rest);
18+
};
19+
})();

‎karma.config.d/00-disable-wasm-streaming.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Files in this directory run inside Karma's Node config context. Inject a
2+
// browser-side shim that patches instantiateStreaming before test modules load.
3+
const path = require("path");
4+
const rootDir = path.resolve(config.basePath, "../../../../");
5+
6+
config.files = config.files || [];
7+
config.files.unshift({
8+
pattern: path.join(
9+
rootDir,
10+
"karma.browser.d",
11+
"00-disable-wasm-streaming.js",
12+
),
13+
included: true,
14+
served: true,
15+
watched: false,
16+
});

‎karma.config.d/10-timeouts.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Wasm startup can block the browser main thread long enough to exceed Karma's
2+
// defaults on local machines. Increase launcher and activity timeouts so local
3+
// browser tests remain stable without disabling them.
4+
config.client = config.client || {};
5+
config.client.mocha = config.client.mocha || {};
6+
config.client.mocha.timeout = 60000;
7+
8+
config.browserNoActivityTimeout = 600000;
9+
config.captureTimeout = 600000;
10+
config.browserDisconnectTimeout = 10000;
11+
config.browserDisconnectTolerance = 2;
12+
config.processKillTimeout = 30000;
13+
14+
// Use a custom launcher derived from ChromeHeadless to keep its known-stable
15+
// defaults while still avoiding fixed remote debugging port collisions.
16+
config.customLaunchers = config.customLaunchers || {};
17+
18+
if (config.browsers && config.browsers.includes("ChromeHeadless")) {
19+
config.customLaunchers.ChromeHeadlessCoil = {
20+
base: "ChromeHeadless",
21+
flags: [
22+
"--remote-debugging-port=0",
23+
// Keep managed/user extensions out of test browsers to reduce flakiness.
24+
"--disable-extensions",
25+
"--disable-component-extensions-with-background-pages",
26+
"--disable-background-networking",
27+
"--disable-default-apps",
28+
],
29+
};
30+
config.browsers = config.browsers.map((browser) =>
31+
browser === "ChromeHeadless" ? "ChromeHeadlessCoil" : browser
32+
);
33+
}

‎test.sh‎

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,71 @@
11
#!/bin/bash
2-
set -e
2+
set -eo pipefail
33

4-
# Run separately to work around https://github.com/diffplug/spotless/issues/1572.
5-
./gradlew checkLegacyAbi spotlessCheck
6-
./gradlew allTests testDebugUnitTest connectedDebugAndroidTest validateDebugScreenshotTest verifyPaparazziDebug verifyRoborazziAndroidHostTest verifyRoborazziJvm
4+
skip_checks=false
5+
skip_instrumentation_tests=false
6+
7+
for arg in "$@"; do
8+
case "$arg" in
9+
--skip-checks)
10+
skip_checks=true
11+
;;
12+
--skip-instrumentation-tests)
13+
skip_instrumentation_tests=true
14+
;;
15+
--help|-h)
16+
cat <<'EOF'
17+
Usage: ./test.sh [options]
18+
19+
Options:
20+
--skip-checks Skip checkLegacyAbi and spotlessCheck.
21+
--skip-instrumentation-tests Skip connectedDebugAndroidTest.
22+
--help, -h Show this help message.
23+
EOF
24+
exit 0
25+
;;
26+
*)
27+
echo "Unknown option: $arg" >&2
28+
exit 1
29+
;;
30+
esac
31+
done
32+
33+
test_tasks=(
34+
allTests
35+
testDebugUnitTest
36+
validateDebugScreenshotTest
37+
verifyPaparazziDebug
38+
verifyRoborazziAndroidHostTest
39+
verifyRoborazziJvm
40+
)
41+
42+
if [[ "$skip_instrumentation_tests" != true ]]; then
43+
test_tasks+=(connectedDebugAndroidTest)
44+
fi
45+
46+
js_wasm_test_tasks=(
47+
jsTest
48+
wasmJsTest
49+
)
50+
51+
js_wasm_excluded_tasks=(
52+
-x jsBrowserTest
53+
-x jsNodeTest
54+
-x jsTest
55+
-x wasmJsBrowserTest
56+
-x wasmJsNodeTest
57+
-x wasmJsTest
58+
)
59+
60+
if [[ "$skip_checks" != true ]]; then
61+
# Run separately to work around https://github.com/diffplug/spotless/issues/1572.
62+
./gradlew checkLegacyAbi spotlessCheck
63+
fi
64+
65+
# Run all non-JS/Wasm tests without a worker cap.
66+
./gradlew "${test_tasks[@]}" "${js_wasm_excluded_tasks[@]}"
67+
68+
# Constrain parallelism so JS/Wasm browser tests don't overwhelm local machines
69+
# and get stuck in Chrome/Karma restart loops.
70+
max_workers="${MAX_WORKERS:-1}"
71+
./gradlew --max-workers="$max_workers" "${js_wasm_test_tasks[@]}"

0 commit comments

Comments
 (0)