diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/AwaitConnection.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/AwaitConnection.kt new file mode 100644 index 000000000..ae9ff2db9 --- /dev/null +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/AwaitConnection.kt @@ -0,0 +1,28 @@ +package dev.kdriver.core.browser + +import kotlinx.coroutines.delay + +/** + * Polls [isConnected] until it succeeds, at most [maxTries] times, waiting [intervalMillis] after + * each failed attempt. + * + * Extracted from `DefaultBrowser.start` so the polling rule can be tested without a real browser. + * It used to be written as `repeat(maxTries) { if (testConnection()) return@repeat; delay(…) }`, + * which does not do what it reads like: `return@repeat` returns from the *lambda*, so it is a + * `continue`, not a `break`. The loop therefore never stopped early — and since the skipped line was + * the `delay`, a browser that answered on the first try still got all the remaining attempts fired + * at it back to back, with no wait in between. + * + * @return true as soon as [isConnected] succeeded, false if all [maxTries] attempts failed. + */ +internal suspend fun awaitConnection( + maxTries: Int, + intervalMillis: Long, + isConnected: suspend () -> Boolean, +): Boolean { + repeat(maxTries) { + if (isConnected()) return true + delay(intervalMillis) + } + return false +} diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index e211166e2..d5f744dc2 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -262,11 +262,10 @@ open class DefaultBrowser( http = HTTPApi(config.host ?: "127.0.0.1", config.port ?: error("Port not set")) delay(config.browserConnectionTimeout) - repeat(config.browserConnectionMaxTries) { - if (testConnection()) return@repeat - delay(config.browserConnectionTimeout) + val connected = awaitConnection(config.browserConnectionMaxTries, config.browserConnectionTimeout) { + testConnection() } - logger.info("Connection to browser established") + if (connected) logger.info("Connection to browser established") val info = info ?: run { // Say what actually failed. Without this the only visible symptom is a 30s wait and a diff --git a/core/src/jvmTest/kotlin/dev/kdriver/core/browser/AwaitConnectionTest.kt b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/AwaitConnectionTest.kt new file mode 100644 index 000000000..cc24b607d --- /dev/null +++ b/core/src/jvmTest/kotlin/dev/kdriver/core/browser/AwaitConnectionTest.kt @@ -0,0 +1,67 @@ +package dev.kdriver.core.browser + +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class AwaitConnectionTest { + + @Test + fun awaitConnection_whenTheBrowserAnswersImmediately_stopsPolling() = runTest { + var attempts = 0 + + val connected = awaitConnection(maxTries = 60, intervalMillis = 500) { + attempts++ + true + } + + assertTrue(connected) + // The whole point: one answer is enough. The previous `repeat { … return@repeat }` kept + // going for all 60 tries — and skipped the delay while doing so, so the browser got 60 + // requests back to back right after it started. + assertEquals(1, attempts) + } + + @Test + fun awaitConnection_whenTheBrowserAnswersLate_stopsAtThatAttempt() = runTest { + var attempts = 0 + + val connected = awaitConnection(maxTries = 60, intervalMillis = 500) { + attempts++ + attempts >= 3 + } + + assertTrue(connected) + assertEquals(3, attempts) + } + + @Test + fun awaitConnection_whenTheBrowserNeverAnswers_triesExactlyMaxTimes() = runTest { + var attempts = 0 + + val connected = awaitConnection(maxTries = 5, intervalMillis = 500) { + attempts++ + false + } + + assertFalse(connected) + assertEquals(5, attempts) + } + + @Test + fun awaitConnection_waitsBetweenAttempts() = runTest { + var attempts = 0 + val start = testScheduler.currentTime + + awaitConnection(maxTries = 4, intervalMillis = 500) { + attempts++ + false + } + + // One interval per failed attempt, the last one included: that trailing wait is what the + // caller's "waited for N ms" error message counts. + assertEquals(4 * 500L, testScheduler.currentTime - start) + } +}