Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading