Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure only one xcodebuild process, cleanup unwanted retries #2097

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+85 −387
Open

Ensure only one xcodebuild process, cleanup unwanted retries #2097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e231d9
fix: remove retries and simplify driver open logic
amanjeetsingh150 Nov 17, 2024
56ef472
fix: reuse pool and tweak timeouts
amanjeetsingh150 Nov 18, 2024
8d216a8
fix: inject the okhttp client
amanjeetsingh150 Nov 18, 2024
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
1 change: 0 additions & 1 deletion maestro-client/src/main/java/maestro/Errors.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,4 @@ sealed class MaestroException(override val message: String) : RuntimeException(m
sealed class MaestroDriverStartupException(override val message: String): RuntimeException() {
class AndroidDriverTimeoutException(message: String): MaestroDriverStartupException(message)
class AndroidInstrumentationSetupFailure(message: String): MaestroDriverStartupException(message)
class IOSDriverTimeoutException(message: String): MaestroDriverStartupException(message)
}
53 changes: 17 additions & 36 deletions maestro-client/src/main/java/maestro/drivers/IOSDriver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import hierarchy.AXElement
import ios.IOSDevice
import ios.IOSDeviceErrors
import maestro.*
import maestro.MaestroDriverStartupException.*
import maestro.UiElement.Companion.toUiElement
import maestro.UiElement.Companion.toUiElementOrNull
import maestro.utils.*
Expand All @@ -35,6 +34,7 @@ import okio.source
import org.slf4j.LoggerFactory
import util.XCRunnerCLIUtils
import java.io.File
import java.net.SocketTimeoutException
import java.util.UUID
import kotlin.collections.set

Expand All @@ -51,7 +51,7 @@ class IOSDriver(
}

override fun open() {
awaitLaunch()
iosDevice.open()
}

override fun close() {
Expand All @@ -63,7 +63,7 @@ class IOSDriver(
}

override fun deviceInfo(): DeviceInfo {
return runDeviceCall { iosDevice.deviceInfo().toCommonDeviceInfo() }
return runDeviceCall("deviceInfo") { iosDevice.deviceInfo().toCommonDeviceInfo() }
}

override fun launchApp(
Expand Down Expand Up @@ -96,11 +96,11 @@ class IOSDriver(
}

override fun tap(point: Point) {
runDeviceCall { iosDevice.tap(point.x, point.y) }
runDeviceCall("tap") { iosDevice.tap(point.x, point.y) }
}

override fun longPress(point: Point) {
runDeviceCall { iosDevice.longPress(point.x, point.y, 3000) }
runDeviceCall("longPress") { iosDevice.longPress(point.x, point.y, 3000) }
}

override fun pressKey(code: KeyCode) {
Expand All @@ -114,7 +114,7 @@ class IOSDriver(
KeyCode.LOCK to "lock",
)

runDeviceCall {
runDeviceCall("pressKey") {
keyCodeNameMap[code]?.let { name ->
iosDevice.pressKey(name)
}
Expand All @@ -126,7 +126,7 @@ class IOSDriver(
}

override fun contentDescriptor(excludeKeyboardElements: Boolean): TreeNode {
return runDeviceCall { viewHierarchy(excludeKeyboardElements) }
return runDeviceCall("contentDescriptor") { viewHierarchy(excludeKeyboardElements) }
}

private fun viewHierarchy(excludeKeyboardElements: Boolean): TreeNode {
Expand Down Expand Up @@ -194,7 +194,7 @@ class IOSDriver(
}

override fun isKeyboardVisible(): Boolean {
return runDeviceCall { iosDevice.isKeyboardVisible() }
return runDeviceCall("isKeyboardVisible") { iosDevice.isKeyboardVisible() }
}

override fun swipe(
Expand All @@ -206,7 +206,7 @@ class IOSDriver(
val startPoint = start.coerceIn(maxWidth = deviceInfo.widthGrid, maxHeight = deviceInfo.heightGrid)
val endPoint = end.coerceIn(maxWidth = deviceInfo.widthGrid, maxHeight = deviceInfo.heightGrid)

runDeviceCall {
runDeviceCall("swipe") {
waitForAppToSettle(null, null)
iosDevice.scroll(
xStart = startPoint.x.toDouble(),
Expand Down Expand Up @@ -360,7 +360,7 @@ class IOSDriver(
}

override fun takeScreenshot(out: Sink, compressed: Boolean) {
runDeviceCall { iosDevice.takeScreenshot(out, compressed) }
runDeviceCall("takeScreenshot") { iosDevice.takeScreenshot(out, compressed) }
}

override fun startScreenRecording(out: Sink): ScreenRecording {
Expand All @@ -372,7 +372,7 @@ class IOSDriver(

override fun inputText(text: String) {
// silently fail if no XCUIElement has focus
runDeviceCall { iosDevice.input(text = text) }
runDeviceCall("inputText") { iosDevice.input(text = text) }
}

override fun openLink(link: String, appId: String?, autoVerify: Boolean, browser: Boolean) {
Expand All @@ -384,7 +384,7 @@ class IOSDriver(
}

override fun eraseText(charactersToErase: Int) {
runDeviceCall { iosDevice.eraseText(charactersToErase) }
runDeviceCall("eraseText") { iosDevice.eraseText(charactersToErase) }
}

override fun setProxy(host: String, port: Int) {
Expand Down Expand Up @@ -421,7 +421,7 @@ class IOSDriver(
}

override fun setPermissions(appId: String, permissions: Map<String, String>) {
runDeviceCall {
runDeviceCall("setPermissions") {
iosDevice.setPermissions(appId, permissions)
}
}
Expand Down Expand Up @@ -456,43 +456,24 @@ class IOSDriver(
}

private fun isScreenStatic(): Boolean {
return runDeviceCall { iosDevice.isScreenStatic() }
return runDeviceCall("isScreenStatic") { iosDevice.isScreenStatic() }
}

private fun awaitLaunch() {
val startTime = System.currentTimeMillis()

while (System.currentTimeMillis() - startTime < getStartupTimeout()) {
runCatching {
iosDevice.open()
return
}
Thread.sleep(100)
}

throw IOSDriverTimeoutException("Maestro iOS driver did not start up in time")
}

private fun <T> runDeviceCall(call: () -> T): T {
private fun <T> runDeviceCall(callName: String, call: () -> T): T {
return try {
call()
} catch (socketTimeoutException: SocketTimeoutException) {
throw MaestroException.DriverTimeout("iOS driver timed out while doing $callName call")
} catch (appCrashException: IOSDeviceErrors.AppCrash) {
throw MaestroException.AppCrash(appCrashException.errorMessage)
}
}

private fun getStartupTimeout(): Long = runCatching {
System.getenv(MAESTRO_DRIVER_STARTUP_TIMEOUT).toLong()
}.getOrDefault(SERVER_LAUNCH_TIMEOUT_MS)

companion object {
const val NAME = "iOS Simulator"

private val LOGGER = LoggerFactory.getLogger(IOSDevice::class.java)

private const val SERVER_LAUNCH_TIMEOUT_MS = 15000L
private const val MAESTRO_DRIVER_STARTUP_TIMEOUT = "MAESTRO_DRIVER_STARTUP_TIMEOUT"

private const val ELEMENT_TYPE_CHECKBOX = 12
private const val ELEMENT_TYPE_SWITCH = 40
private const val ELEMENT_TYPE_TOGGLE = 41
Expand Down
87 changes: 0 additions & 87 deletions maestro-client/src/test/java/maestro/xctestdriver/XCTestDriverClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,6 @@ import java.net.InetAddress

class XCTestDriverClientTest {

@Test
fun `it should return correct message in case of TimeoutException with 3 retries`() {
// given
val mockWebServer = MockWebServer()
// do not enqueue any response
mockWebServer.start(InetAddress.getByName( "localhost"), 22087)
val httpUrl = mockWebServer.url("/deviceInfo")

// when
val simulator = MockXCTestInstaller.Simulator(
installationRetryCount = 0,
shouldInstall = false
)
val mockXCTestInstaller = MockXCTestInstaller(simulator)
val xcTestDriverClient = XCTestDriverClient(
mockXCTestInstaller,
XCTestClient("localhost", 22087)
)

// then
assertThrows<XCUITestServerError.NetworkError> {
xcTestDriverClient.deviceInfo(httpUrl)
}
mockXCTestInstaller.assertInstallationRetries(5)
mockWebServer.shutdown()
}

@Test
fun `it should return the 4xx response as is without retrying`() {
// given
Expand Down Expand Up @@ -138,66 +111,6 @@ class XCTestDriverClientTest {
mockWebServer.shutdown()
}

@Test
fun `it should return correct message in case of UnknownHostException without retries`() {
// given
val mockWebServer = MockWebServer()
mockWebServer.enqueue(
MockResponse()
.setSocketPolicy(SocketPolicy.DISCONNECT_AT_START)
)
mockWebServer.start(InetAddress.getByName( "localhost"), 22087)
val httpUrl = mockWebServer.url("http://nonexistent-domain.local")

// when
val simulator = MockXCTestInstaller.Simulator(
installationRetryCount = 0,
shouldInstall = false
)
val mockXCTestInstaller = MockXCTestInstaller(simulator)
val xcTestDriverClient = XCTestDriverClient(
mockXCTestInstaller,
XCTestClient("localhost", 22087)
)

// then
assertThrows<XCUITestServerError.NetworkError> {
xcTestDriverClient.deviceInfo(httpUrl)
}
mockXCTestInstaller.assertInstallationRetries(0)
mockWebServer.shutdown()
}

@Test
fun `it should return correct message in case of ConnectExceptions with 3 retries`() {
// given
val mockWebServer = MockWebServer()
mockWebServer.enqueue(
MockResponse()
.setSocketPolicy(SocketPolicy.DISCONNECT_DURING_REQUEST_BODY)
)
mockWebServer.start(InetAddress.getByName( "localhost"), 22087)
val httpUrl = mockWebServer.url("/deviceInfo")
mockWebServer.shutdown()

// when
val simulator = MockXCTestInstaller.Simulator(
installationRetryCount = 0,
shouldInstall = false
)
val mockXCTestInstaller = MockXCTestInstaller(simulator)
val xcTestDriverClient = XCTestDriverClient(
mockXCTestInstaller,
XCTestClient("localhost", 22087)
)

// then
assertThrows<XCUITestServerError.NetworkError> {
xcTestDriverClient.deviceInfo(httpUrl)
}
mockXCTestInstaller.assertInstallationRetries(5)
}

companion object {

@JvmStatic
Expand Down
Loading
Loading