Fix WebRTC JS tests #5132
Fix WebRTC JS tests #5132
Conversation
WalkthroughUpdates Kotlin wrappers catalog version, adds ChromeHeadless media-stream mocking flags, removes internal JS interop conversion helpers, adds ExperimentalWasmJsInterop opt-ins across WebRTC JS/WASM files, refactors DataChannel/Browser interop conversions, and updates WebRTC media tests to create and inspect tracks explicitly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{kt,kts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.kt📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.kt (2)
87-93: Text messages won’t match JsString. Use String (or typeof) check.In browsers, e.data for text is a primitive string, not JsString. Current check can miss text and fall through.
- when { - e.data is JsString -> { - emitMessage(WebRtc.DataChannel.Message.Text(e.data.toString())) - } + when { + e.data is String -> { + emitMessage(WebRtc.DataChannel.Message.Text(e.data as String)) + }Alternatively:
when (jsTypeOf(e.data)) { "string" -> ... }.
97-100: Await Blob.arrayBuffer() before converting.Blob.arrayBuffer() returns a Promise. You must await it before toByteArray().
+import kotlinx.coroutines.await @@ - val buffer = (e.data as Blob).arrayBuffer() - emitMessage(WebRtc.DataChannel.Message.Binary(buffer.toByteArray())) + val buffer = (e.data as Blob).arrayBuffer().await() + emitMessage(WebRtc.DataChannel.Message.Binary(buffer.toByteArray()))ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Browser.kt (1)
94-103: Wrong enum used for ICE connection “completed” state.Mapping uses RTCIceGatheringState.complete inside RTCIceConnectionState mapping; should be RTCIceConnectionState.completed.
internal fun RTCIceConnectionState?.toKtor(): WebRtc.IceConnectionState = when (this) { RTCIceConnectionState.new -> WebRtc.IceConnectionState.NEW RTCIceConnectionState.checking -> WebRtc.IceConnectionState.CHECKING RTCIceConnectionState.connected -> WebRtc.IceConnectionState.CONNECTED RTCIceConnectionState.disconnected -> WebRtc.IceConnectionState.DISCONNECTED RTCIceConnectionState.failed -> WebRtc.IceConnectionState.FAILED RTCIceConnectionState.closed -> WebRtc.IceConnectionState.CLOSED - RTCIceGatheringState.complete -> WebRtc.IceConnectionState.COMPLETED + RTCIceConnectionState.completed -> WebRtc.IceConnectionState.COMPLETED else -> error("Unknown ice connection state: $this") }
🧹 Nitpick comments (3)
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.kt (1)
65-67: Send typed array, not its underlying buffer.Avoid sending the backing ArrayBuffer; send the Int8Array view to preserve length/offset correctness.
- channel.send(bytes.toInt8Array().buffer) + channel.send(bytes.toInt8Array())ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/JsWebRtcMediaTest.kt (2)
42-45: Avoid string round‑trip for Boolean assertion.Assert Boolean directly or via safe cast; this is clearer and avoids locale/format surprises.
- assertEquals(true, settings1.echoCancellation.toString().toBoolean()) + assertTrue(settings1.echoCancellation == true)
72-77: Use tolerance for Double comparison.Floating values can be non‑exact; use delta.
- assertEquals(2.0, settings2.aspectRatio) + assertEquals(2.0, settings2.aspectRatio, 0.001)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
build-settings-logic/src/main/kotlin/ktorsettings.dependency-resolution-management.settings.gradle.kts(1 hunks)karma/chrome_bin.js(1 hunks)ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Utils.js.kt(0 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Browser.kt(4 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.kt(3 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/MediaDevices.kt(2 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/PeerConnection.kt(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Rtp.kt(2 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.kt(2 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/JsWebRtcMediaTest.kt(2 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt(2 hunks)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Utils.wasmJs.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Utils.js.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Rtp.ktbuild-settings-logic/src/main/kotlin/ktorsettings.dependency-resolution-management.settings.gradle.ktsktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Browser.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/PeerConnection.ktktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Utils.wasmJs.ktktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/JsWebRtcMediaTest.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/MediaDevices.ktktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Rtp.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Browser.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/PeerConnection.ktktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Utils.wasmJs.ktktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/JsWebRtcMediaTest.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.ktktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/MediaDevices.ktktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt
🧠 Learnings (1)
📚 Learning: 2025-08-15T09:12:47.243Z
Learnt from: zibet27
PR: ktorio/ktor#5044
File: ktor-client/ktor-client-webrtc/ktor-client-webrtc-rs/common/rust/senders.rs:88-96
Timestamp: 2025-08-15T09:12:47.243Z
Learning: In the webrtc Rust crate version used in the Ktor WebRTC RS implementation, the RTCRtpCodingParameters struct's rtx field is NOT optional and can be accessed directly as params.rtx.ssrc without needing Option handling.
Applied to files:
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Rtp.kt
🧬 Code graph analysis (1)
ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/JsWebRtcMediaTest.kt (1)
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/MediaDevices.kt (1)
getNative(100-102)
🔇 Additional comments (9)
build-settings-logic/src/main/kotlin/ktorsettings.dependency-resolution-management.settings.gradle.kts (1)
29-29: LGTM!The kotlin-wrappers catalog version update to 2025.10.8 aligns with the PR objective to use new ByteArray extensions and enables the experimental Wasm/JS interop features used throughout the WebRTC module.
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Rtp.kt (1)
4-5: LGTM!The opt-in annotation and imports properly enable experimental Wasm/JS interop features, allowing usage of
toArray()on JS iterables throughout the file. This change is consistent with the PR-wide adoption of ExperimentalWasmJsInterop.Also applies to: 16-17
karma/chrome_bin.js (1)
17-19: LGTM!The Chrome headless flags correctly enable fake media device and UI mocking, directly addressing the test blocking issue described in the PR objectives. These flags prevent Chrome from displaying permission dialogs and ensure tests use fake media devices instead of attempting to access real hardware.
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/PeerConnection.kt (1)
4-4: LGTM!The opt-in annotation and imports enable experimental Wasm/JS interop features consistently with other WebRTC module files. The
toArray()function is now available from kotlin-wrappers, replacing previously internal conversion helpers.Also applies to: 10-11
ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt (1)
1-1: LGTM!The test file properly adopts the experimental Wasm/JS interop opt-in, maintaining consistency with the production code changes.
Also applies to: 10-10
ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Utils.wasmJs.kt (1)
5-6: LGTM!The opt-in annotation enables experimental Wasm/JS interop features. The removal of internal conversion helpers (
toArray()andtoJs()) is appropriate as these are now provided by kotlin-wrappers 2025.10.8, eliminating the need for manual implementations.ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/MediaDevices.kt (1)
4-4: LGTM!The opt-in annotation and imports enable experimental Wasm/JS interop features for converting JS iterables to arrays. The
toArray()function is now used for extracting audio and video tracks from media streams, replacing internal conversion helpers.Also applies to: 14-15
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.kt (1)
5-6: LGTM!The opt-in annotation enables experimental Wasm/JS interop features across the shared utilities. The removal of internal conversion helpers (ByteArray ↔ ArrayBuffer, JsArray expect declarations) is appropriate as these are now provided by kotlin-wrappers 2025.10.8, avoiding code duplication.
Also applies to: 19-19
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/DataChannel.kt (1)
111-113: onerror payload may differ across browsers; add fallback.RTCDataChannel error events aren’t consistently shaped. Consider a defensive fallback to avoid null messages.
- channel.onerror = eventHandler(coroutineScope) { e -> - eventsEmitter.emitDataChannelEvent(DataChannelEvent.Error(this, e.error.message)) - } + channel.onerror = eventHandler(coroutineScope) { e -> + val msg = (e.asDynamic().error?.message ?: e.asDynamic().message ?: "Unknown data channel error") as String + eventsEmitter.emitDataChannelEvent(DataChannelEvent.Error(this, msg)) + }
…RtcDataChannel`.
Subsystem
WebRTC Client.
Motivation
JS tests were failing lately because of the
Chrome for Testingupdate. Previous tests were based on the assumption that using the real browser media API should throw aNotFoundError. Moreover, Chrome was ignoring useful flags for testing like--use-fake-device-for-media-streamor--use-fake-ui-for-media-stream. Now, when we were trying to access the Browser Media API, it was "showing" an alert window and blocking the test.Solution
Now, we can use mock media streams by enabling
--use-fake-device-for-media-streamand--use-fake-ui-for-media-stream, see Chromium options.Extra:
kotlin-wrappers, which added useful extensions forByteArray(before, we wrote them manually).