Repository navigation
Add run, hot reload, and DevTools inspector UI tests for IntelliJ IDEA 2026.2 - #9212
cj-radcliff wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds integration tests and supporting utilities for the Flutter IntelliJ plugin, including tests for DevTools/Inspector UI and Run/Hot Reload functionality, along with helper classes for managing an Android emulator, selecting the browser engine, and interacting with the Dart VM service. The review feedback identifies critical issues regarding process and file descriptor leaks in AndroidEmulator.kt, a thread leak from repeated HttpClient instantiation in VmServiceClient.kt, potential hanging futures on WebSocket errors, and a style guide violation concerning the use of the double-bang !! operator.
| exec(adb, "wait-for-device").waitFor(5, TimeUnit.MINUTES) | ||
| val deadline = System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(5) | ||
| while (System.currentTimeMillis() < deadline) { | ||
| val booted = exec(adb, "shell", "getprop", "sys.boot_completed").inputStream.bufferedReader().readText().trim() | ||
| if (booted == "1") { | ||
| println("Emulator booted") | ||
| return | ||
| } | ||
| Thread.sleep(3_000) | ||
| } | ||
| error("Emulator $avdName did not finish booting") |
There was a problem hiding this comment.
[MUST-FIX] The processes spawned by exec are not properly managed. exec(adb, "wait-for-device") does not destroy the process if it times out, and the loop calling exec(adb, "shell", "getprop", ...) spawns a new process every 3 seconds without waiting for it or destroying it, leading to a severe process and file descriptor leak. We should ensure all spawned processes are waited for and forcibly destroyed if they exceed their timeouts.
val waitProcess = exec(adb, "wait-for-device")
if (!waitProcess.waitFor(5, TimeUnit.MINUTES)) {
waitProcess.destroyForcibly()
error("Emulator wait-for-device timed out")
}
val deadline = System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(5)
while (System.currentTimeMillis() < deadline) {
val checkProcess = exec(adb, "shell", "getprop", "sys.boot_completed")
val booted = runCatching {
checkProcess.inputStream.bufferedReader().use { it.readText().trim() }
}.getOrDefault("")
if (!checkProcess.waitFor(5, TimeUnit.SECONDS)) {
checkProcess.destroyForcibly()
}
if (booted == "1") {
println("Emulator booted")
return
}
Thread.sleep(3_000)
}
error("Emulator $avdName did not finish booting")| class VmServiceClient(wsUri: String) : AutoCloseable { | ||
| private val nextId = AtomicInteger() | ||
| private val pending = ConcurrentHashMap<String, CompletableFuture<JsonObject>>() | ||
| private val webSocket: WebSocket = HttpClient.newHttpClient().newWebSocketBuilder() | ||
| .buildAsync(URI(wsUri), Listener()) | ||
| .get(30, TimeUnit.SECONDS) |
There was a problem hiding this comment.
[MUST-FIX] Creating a new HttpClient instance on every instantiation of VmServiceClient causes a severe thread leak. Since VmServiceClient is instantiated repeatedly in a polling loop inside waitForAppTitle (every 2 seconds for up to 1 minute), this spawns dozens of selector and worker threads that are not cleaned up. We should reuse a single HttpClient instance by defining it in a companion object.
class VmServiceClient(wsUri: String) : AutoCloseable {
private companion object {
private val httpClient = HttpClient.newHttpClient()
}
private val nextId = AtomicInteger()
private val pending = ConcurrentHashMap<String, CompletableFuture<JsonObject>>()
private val webSocket: WebSocket = httpClient.newWebSocketBuilder()
.buildAsync(URI(wsUri), Listener())
.get(30, TimeUnit.SECONDS)| fun stop() { | ||
| process?.let { | ||
| runCatching { exec(adb, "emu", "kill").waitFor(30, TimeUnit.SECONDS) } | ||
| it.destroy() | ||
| } | ||
| process = null | ||
| } |
There was a problem hiding this comment.
[CONCERN] The process spawned by exec(adb, "emu", "kill") is not destroyed if it times out, which can leak process resources. We should ensure the process is forcibly destroyed if it does not complete within the 30-second timeout.
fun stop() {
process?.let {
runCatching {
val killProcess = exec(adb, "emu", "kill")
if (!killProcess.waitFor(30, TimeUnit.SECONDS)) {
killProcess.destroyForcibly()
}
}
it.destroy()
}
process = null
}| private inner class Listener : WebSocket.Listener { | ||
| private val buffer = StringBuilder() | ||
|
|
||
| override fun onText(webSocket: WebSocket, data: CharSequence, last: Boolean): CompletionStage<*>? { | ||
| buffer.append(data) | ||
| if (last) { | ||
| val message = runCatching { JsonParser.parseString(buffer.toString()).asJsonObject }.getOrNull() | ||
| buffer.setLength(0) | ||
| message?.get("id")?.asString?.let { pending.remove(it)?.complete(message) } | ||
| } | ||
| webSocket.request(1) | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
[CONCERN] If the WebSocket connection is closed or encounters an error, any pending futures in pending will hang until they hit the 60-second timeout. Implementing onError and onClose to complete these futures exceptionally will make the tests fail immediately and robustly.
private inner class Listener : WebSocket.Listener {
private val buffer = StringBuilder()
override fun onText(webSocket: WebSocket, data: CharSequence, last: Boolean): CompletionStage<*>? {
buffer.append(data)
if (last) {
val message = runCatching { JsonParser.parseString(buffer.toString()).asJsonObject }.getOrNull()
buffer.setLength(0)
message?.get("id")?.asString?.let { pending.remove(it)?.complete(message) }
}
webSocket.request(1)
return null
}
override fun onError(webSocket: WebSocket, error: Throwable) {
pending.values.forEach { it.completeExceptionally(error) }
pending.clear()
}
override fun onClose(webSocket: WebSocket, statusCode: Int, reason: String): CompletionStage<*>? {
pending.values.forEach { it.completeExceptionally(IllegalStateException("WebSocket closed: $statusCode $reason")) }
pending.clear()
return null
}
}| } | ||
| println("VM service: $vmServiceUri") | ||
|
|
||
| val app = vmServiceUri!! |
There was a problem hiding this comment.
[CONCERN] Avoid using the double-bang !! operator as per the repository style guide. Use requireNotNull instead to safely assert non-nullability with a descriptive error message.
| val app = vmServiceUri!! | |
| val app = requireNotNull(vmServiceUri) { "VM service URI was not found" } |
References
- NEVER use the double-bang !! operator. Use ?., ?:, or if (x != null). (link)
Stacked on #9211 (and #9209).
Adds UI integration tests and device/browser helpers for running, hot reloading, and inspecting Flutter apps in IntelliJ IDEA 2026.2:
Tests
RunAppAndHotReloadUITest: launches a Flutter app from the IDE on Chrome (FLUTTER_TEST_DEVICE=chrome, default) or an Android emulator (FLUTTER_TEST_DEVICE=emulator), connects to its Dart VM Service (ext.flutter.debugDumpApp) to verify the displayed app bar title, editslib/main.dart, triggersFlutter.Toolbar.ReloadAction, verifies the hot reload succeeded without restarting and updated the live widget tree, and stops the app.DevToolsInspectorUITest: starts a debug session and opens the Flutter Inspector and Flutter DevTools right-hand tool windows, verifying embedded browser rendering (or the JxBrowser missing-license fallback whenFLUTTER_TEST_BROWSER=jxbrowserwithout a bundled license key).Helpers & Docs
utils/AndroidEmulator.kt: starts and cleanly shuts down an Android AVD (adb emu kill) on the test display.utils/VmServiceClient.kt: minimaljava.net.http.WebSocketJSON-RPC 2.0 client for queryingext.flutter.debugDumpAppon the running app.utils/BrowserEngine.kt: configures JxBrowser vs. JCEF (io.flutter.enableJcefBrowser) and checks whetherFlutter.zipcontains a bundled JxBrowser license key.testSrc/integration/README.md: documents the new tests andFLUTTER_TEST_DEVICE/FLUTTER_TEST_AVD/FLUTTER_TEST_BROWSERenvironment variables.