Overhaul retry/polling logic (#99)

Sources/Element.swift

@@ -48,27 +48,33 @@ public struct Element {
     /// Clicks this element.
     public func click(retryTimeout: TimeInterval? = nil) throws {
         let request = Requests.ElementClick(session: session.id, element: id)
-        try retryUntil(retryTimeout ?? session.defaultRetryTimeout) {
+        let result = try poll(timeout: retryTimeout ?? session.defaultRetryTimeout) {
             do {
+                // Immediately bubble most failures, only retry on element not interactable.
                 try webDriver.send(request)
-                return true
+                return PollResult.success(nil as ErrorResponse?)
             } catch let error as ErrorResponse where error.status == .winAppDriver_elementNotInteractable {
-                return false
+                return PollResult.failure(error)
             }
         }
+
+        if let notInteractableError = result.value { throw notInteractableError }
     }
 
     /// Clicks this element via touch.
     public func touchClick(kind: TouchClickKind = .single, retryTimeout: TimeInterval? = nil) throws {
         let request = Requests.SessionTouchClick(session: session.id, kind: kind, element: id)
-        try retryUntil(retryTimeout ?? session.defaultRetryTimeout) {
+        let result = try poll(timeout: retryTimeout ?? session.defaultRetryTimeout) {
             do {
+                // Immediately bubble most failures, only retry on element not interactable.
                 try webDriver.send(request)
-                return true
+                return PollResult.success(nil as ErrorResponse?)
             } catch let error as ErrorResponse where error.status == .winAppDriver_elementNotInteractable {
-                return false
+                return PollResult.failure(error)
             }
         }
+
+        if let notInteractableError = result.value { throw notInteractableError }
     }
 
     /// Search for an element by name, starting from this element.

Sources/Poll.swift

@@ -0,0 +1,55 @@
+import class Foundation.Thread
+import struct Foundation.TimeInterval
+import struct Dispatch.DispatchTime
+
+/// Calls a closure repeatedly with exponential backoff until it reports success or a timeout elapses.
+/// - Returns: The result from the last invocation of the closure.
+internal func poll<Value>(
+        timeout: TimeInterval,
+        initialPeriod: TimeInterval = 0.001,
+        work: () throws -> PollResult<Value>) rethrows -> PollResult<Value> {
+    let startTime = DispatchTime.now()
+    var result = try work()
+    if result.success { return result }
+
+    var period = initialPeriod
+    while true {
+        // Check if we ran out of time and return the last result
+        let elapsedTime = Double(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds) / 1_000_000_000
+        let remainingTime = timeout - elapsedTime
+        if remainingTime < 0 { return result }
+
+        // Sleep for the next period and retry
+        let sleepTime = min(period, remainingTime)
+        Thread.sleep(forTimeInterval: sleepTime)
+
+        result = try work()
+        if result.success { return result }
+
+        period *= 2 // Exponential backoff
+    }
+}
+
+/// Calls a closure repeatedly with exponential backoff until it reports success or a timeout elapses.
+/// - Returns: Whether the closure reported success within the expected time.
+internal func poll(
+        timeout: TimeInterval,
+        initialPeriod: TimeInterval = 0.001,
+        work: () throws -> Bool) rethrows -> Bool {
+    try poll(timeout: timeout, initialPeriod: initialPeriod) {
+        PollResult(value: Void(), success: try work())
+    }.success
+}
+
+internal struct PollResult<Value> {
+    let value: Value
+    let success: Bool
+
+    static func success(_ value: Value) -> PollResult<Value> {
+        PollResult(value: value, success: true)
+    }
+
+    static func failure(_ value: Value) -> PollResult<Value> {
+        PollResult(value: value, success: false)
+    }
+}

Sources/Session.swift

@@ -24,7 +24,9 @@ public class Session {
     }
 
     /// A TimeInterval specifying max time to spend retrying operations.
-    public var defaultRetryTimeout: TimeInterval = 1.0
+    public var defaultRetryTimeout: TimeInterval = 1.0 {
+        willSet { precondition(newValue >= 0) }
+    }
 
     /// The title of this session such as the tab or window text.
     public var title: String {
@@ -100,16 +102,22 @@ public class Session {
     // Helper for findElement functions above.
     internal func findElement(startingAt element: Element?, using: String, value: String, retryTimeout: TimeInterval?) throws -> Element? {
         precondition(element == nil || element?.session === self)
+
         let request = Requests.SessionElement(session: id, element: element?.id, using: using, value: value)
-        let element = try retryUntil(retryTimeout ?? defaultRetryTimeout) {
+
+        let elementId = try poll(timeout: retryTimeout ?? defaultRetryTimeout) {
+            let elementId: String?
             do {
-                let responseValue = try webDriver.send(request).value
-                return Element(in: self, id: responseValue.element)
+                // Allow errors to bubble up unless they are specifically saying that the element was not found.
+                elementId = try webDriver.send(request).value.element
             } catch let error as ErrorResponse where error.status == .noSuchElement {
-                return nil
+                elementId = nil
             }
-        }
-        return element
+
+            return PollResult(value: elementId, success: elementId != nil)
+        }.value
+
+        return elementId.map { Element(in: self, id: $0) }
     }
 
     /// Moves the pointer to a location relative to the current pointer position or an element.

Sources/WinAppDriver/Win32ProcessTree.swift

@@ -1,3 +1,4 @@
+import struct Foundation.TimeInterval
 import WinSDK
 
 /// Starts and tracks the lifetime of a process tree using Win32 APIs.
@@ -17,10 +18,29 @@ internal class Win32ProcessTree {
         }
     }
 
-    func terminate() throws {
+    var exitCode: DWORD? {
+        get throws {
+            var result: DWORD = 0
+            guard WinSDK.GetExitCodeProcess(handle, &result) else {
+                throw Win32Error.getLastError(apiName: "GetExitCodeProcess")
+            }
+            return result == WinSDK.STILL_ACTIVE ? nil : result
+        }
+    }
+
+    func terminate(waitTime: TimeInterval?) throws {
+        precondition((waitTime ?? 0) >= 0)
+
         if !TerminateJobObject(jobHandle, UINT.max) {
             throw Win32Error.getLastError(apiName: "TerminateJobObject")
         }
+
+        if let waitTime {
+            let milliseconds = waitTime * 1000
+            let millisecondsDword = milliseconds > Double(DWORD.max) ? INFINITE : DWORD(milliseconds)
+            let waitResult = WaitForSingleObject(handle, millisecondsDword)
+            assert(waitResult == WAIT_OBJECT_0, "The process did not terminate within the expected time interval.")
+        }
     }
 
     deinit {

Sources/WinAppDriver/WinAppDriver.swift

@@ -14,24 +14,26 @@ public class WinAppDriver: WebDriver {
     public static var defaultExecutablePath: String {
         "\(WindowsSystemPaths.programFilesX86)\\Windows Application Driver\\\(executableName)"
     }
+    public static let defaultStartWaitTime: TimeInterval = 1.0
 
     private let httpWebDriver: HTTPWebDriver
-    private var processTree: Win32ProcessTree?
+    private let processTree: Win32ProcessTree?
 
     private init(httpWebDriver: HTTPWebDriver, processTree: Win32ProcessTree? = nil) {
         self.httpWebDriver = httpWebDriver
         self.processTree = processTree
     }
 
-    public static func attach(ip: String = defaultIp, port: Int = WinAppDriver.defaultPort) -> WinAppDriver {
+    public static func attach(ip: String = defaultIp, port: Int = defaultPort) -> WinAppDriver {
         let httpWebDriver = HTTPWebDriver(endpoint: URL(string: "http://\(ip):\(port)")!)
         return WinAppDriver(httpWebDriver: httpWebDriver)
     }
 
     public static func start(
         executablePath: String = defaultExecutablePath,
-        ip: String = WinAppDriver.defaultIp,
-        port: Int = WinAppDriver.defaultPort) throws -> WinAppDriver {
+        ip: String = defaultIp,
+        port: Int = defaultPort,
+        waitTime: TimeInterval? = defaultStartWaitTime) throws -> WinAppDriver {
 
         let processTree: Win32ProcessTree
         do {
@@ -40,25 +42,28 @@ public class WinAppDriver: WebDriver {
             throw StartError(message: "Call to Win32 \(error.apiName) failed with error code \(error.errorCode).")
         }
 
-        // This gives some time for WinAppDriver to get up and running before
-        // we hammer it with requests, otherwise some requests will timeout.
-        Thread.sleep(forTimeInterval: 1.0)
-
         let httpWebDriver = HTTPWebDriver(endpoint: URL(string: "http://\(ip):\(port)")!)
+
+        // Give WinAppDriver some time to start up
+        if let waitTime {
+            // TODO(#40): This should be using polling, but an immediate url request would block forever
+            Thread.sleep(forTimeInterval: waitTime)
+
+            if let earlyExitCode = try? processTree.exitCode {
+                throw StartError(message: "WinAppDriver process exited early with error code \(earlyExitCode).")
+            }
+        }
+
         return WinAppDriver(httpWebDriver: httpWebDriver, processTree: processTree)
     }
 
     deinit {
         if let processTree {
             do {
-                try processTree.terminate()
+                try processTree.terminate(waitTime: TimeInterval.infinity)
             } catch {
-                assertionFailure("TerminateProcess failed with error \(error).")
+                assertionFailure("WinAppDriver did not terminate within the expected time: \(error).")
             }
-
-            // Add a short delay to let process cleanup happen before we try
-            // to launch another instance.
-            Thread.sleep(forTimeInterval: 1.0)
         }
     }
 

Tests/WinAppDriverTests/RequestsTests.swift

@@ -10,7 +10,12 @@ class RequestsTests: XCTestCase {
         _app = Result { try MSInfo32App(winAppDriver: WinAppDriver.start()) }
     }
 
-    override func setUpWithError() throws { try XCTSkipIf(app == nil)}
+    override func setUpWithError() throws {
+        if case .failure(let error) = Self._app {
+            throw XCTSkip("Failed to start test app: \(error)")
+        }
+    }
+
     override class func tearDown() { _app = nil }
 
     func testStatusReportsWinAppDriverOnWindows() throws {
@@ -66,8 +71,11 @@ class RequestsTests: XCTestCase {
         // Normally we should be able to read the text back immediately,
         // but the MSInfo32 "Find what" edit box seems to queue events
         // such that WinAppDriver returns before they are fully processed.
-        try retryUntil(0.5) { try app.findWhatEditBox.text == str }
-        XCTAssertEqual(try app.findWhatEditBox.text, str)
+        XCTAssertEqual(
+            try poll(timeout: 0.5) {
+                let text = try app.findWhatEditBox.text
+                return PollResult(value: text, success: text == str)
+            }.value, str)
     }
 
     func testSendKeysWithAcceleratorsGivesFocus() throws {

Tests/WinAppDriverTests/TimeoutTests.swift

@@ -14,7 +14,9 @@ class TimeoutTests: XCTestCase {
     }
 
     override func setUpWithError() throws {
-        try XCTSkipIf(winAppDriver == nil)
+        if case .failure(let error) = Self._winAppDriver {
+            throw XCTSkip("Failed to start WinAppDriver: \(error)")
+        }
     }
 
     func startApp() throws -> Session {