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.

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

Fix random error in with-driver-tests #681

Merged
merged 7 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ A release with an intentional breaking changes is marked with:
== Unreleased

* Changes
** {issue}676[#676]: Fix new driver creation so that it sidesteps some underlying Firefox race conditions and improves CI test stability. ({person}dgr[@dgr])
** {issue}679[#679]: Add `new-window` function that exposes WebDriver's New Window endpoint. ({person}dgr[@dgr])

== v1.1.42 - 2024-09-27 [[v1.1.42]]
Expand Down
15 changes: 12 additions & 3 deletions script/fake_driver.clj
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,23 @@
(defn make-handler [_opts]
(fn handle-request [{:keys [request-method uri] :as _req}]
(cond
;; Get Status
(and (= :get request-method) (= "/status" uri))
{:status 200
:headers {"Content-Type" "application/json"}
:body (json/generate-string {:value {:ready true
:message "I'm ready"}})}
;; Create Session
(and (= :post request-method) (= "/session" uri))
{:status 200
:headers {"Content-Type" "application/json"}
:body (json/generate-string {:sessionId (random-uuid)})}
(and (= :get request-method) (= "/status" uri))
:body (json/generate-string {:value {:sessionId (random-uuid)}})}
;; Get Window Handle
(and (= :get request-method) (re-matches #"/session/[^/]+/window" uri))
{:status 200
:headers {"Content-Type" "application/json"}
:body (json/generate-string {:ready true})}
:body (json/generate-string {:value (random-uuid)})}
;; Fake Driver is... well... fake.
:else
{:status 404})))

Expand Down
27 changes: 24 additions & 3 deletions src/etaoin/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -3465,6 +3465,7 @@
(:post-run-actions driver))
(recur (inc try-num) (:exception res)))))))))


(defn- -connect-driver
"Connects to a running Webdriver server.

Expand Down Expand Up @@ -3546,9 +3547,29 @@
(drv/set-capabilities (:capabilities defaults-global))
(drv/set-capabilities (get-in defaults [type :capabilities]))
(drv/set-capabilities capabilities)))
caps (:capabilities driver)
session (create-session driver caps)]
(assoc driver :session session)))
caps (:capabilities driver)]
(loop [n 1]
;; Wait for driver to be ready before creating the first session.
dgr marked this conversation as resolved.
Show resolved Hide resolved
(wait-predicate
(fn [] (-> (get-status driver) :ready))
{:timeout 30
:interval 0.100
:message "Timeout waiting for WebDriver to become ready after creation."})
(let [driver (assoc driver :session (create-session driver caps))
good-session (try (get-window-handle driver)
dgr marked this conversation as resolved.
Show resolved Hide resolved
true
(catch Exception e
(prn e)
lread marked this conversation as resolved.
Show resolved Hide resolved
false))
]
dgr marked this conversation as resolved.
Show resolved Hide resolved
(if good-session
driver
(do
(delete-session driver)
(if (< n 3) ; max attempts = 3
(recur (inc n))
(throw+ {:type :etaoin/retries
dgr marked this conversation as resolved.
Show resolved Hide resolved
:message "Could not create a good session after muiltiple retries"}))))))))
dgr marked this conversation as resolved.
Show resolved Hide resolved

(defn disconnect-driver
"Returns new `driver` after disconnecting from a running WebDriver process.
Expand Down
42 changes: 26 additions & 16 deletions test/etaoin/unit/proc_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,30 @@

(deftest http-error-on-create-proc-alive
;; when etaoin tries to create a session return an http error
(with-redefs [client/http-request (fn [_] {:status 400})]
(let [ex (try
(e/with-firefox _driver
(is false "should not reach here"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-error (:type exd)))
(is (= 400 (:status exd)))
(is (= nil (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances))))))
(let [orig-http-request client/http-request]
(with-redefs [client/http-request (fn [{:keys [method uri] :as params}]
;; allow get status and create session through, fail on everything else
(if (and (= :get method) (str/ends-with? uri "/status"))
(orig-http-request params)
{:status 400}))]
(let [ex (try
(e/with-firefox _driver
(is false "should not reach here"))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
(is (= :etaoin/http-error (:type exd)))
(is (= 400 (:status exd)))
(is (= nil (-> exd :driver :process :exit)))
(is (= 0 (get-count-firefoxdriver-instances)))))))

(deftest http-exception-after-create-proc-now-dead
(let [orig-http-request client/http-request]
(with-redefs [client/http-request (fn [{:keys [method uri] :as params}]
;; allow create session through, fail on everything else
(if (and (= :post method) (str/ends-with? uri "/session"))
;; allow get status and create session through, fail on everything else
(if (or (and (= :get method) (str/ends-with? uri "/status"))
(and (= :post method) (str/ends-with? uri "/session"))
(and (= :get method) (re-find #"/session/[^/]+/window" uri)))
(orig-http-request params)
(throw (ex-info "read timeout" {}))))]
(let [ex (try
Expand All @@ -130,7 +137,8 @@
(is (= 1 (get-count-firefoxdriver-instances)))
(proc/kill (:process driver))
;; we'll now fail on this call
(e/go driver "https://clojure.org"))
(e/go driver "https://clojure.org")
(is false "Should have thrown an exception."))
(catch Throwable ex
{:exception ex}))
exd (-> ex :exception ex-data)]
Expand All @@ -143,8 +151,10 @@
;; unlikely, we know we just talked to the driver because it returned an http error, but for completeness
(let [orig-http-request client/http-request]
(with-redefs [client/http-request (fn [{:keys [method uri] :as params}]
;; allow create session through, fail on everything else
(if (and (= :post method) (str/ends-with? uri "/session"))
;; allow get status and create session through, fail on everything else
(if (or (and (= :get method) (str/ends-with? uri "/status"))
(and (= :post method) (str/ends-with? uri "/session"))
(and (= :get method) (re-find #"/session/[^/]+/window" uri)))
(orig-http-request params)
{:status 418}))]
(let [ex (try
Expand Down
4 changes: 4 additions & 0 deletions test/etaoin/unit/unit_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
(with-redefs
[etaoin.impl.proc/run (fn [_ _] {:some :process})
e/wait-running identity
e/get-status (fn [_] {:ready true :message "I'm ready"})
e/create-session (fn [_ _] "session-key")
e/get-window-handle (fn [_] "ABCDEFG")
proc/kill identity
e/delete-session identity
util/get-free-port (constantly 12345)]
Expand Down Expand Up @@ -142,7 +144,9 @@
[etaoin.impl.proc/run (fn [_ _]
(swap! run-calls inc)
{:some :process})
e/get-status (fn [_] {:ready true :message "I'm ready"})
e/create-session (fn [_ _] "session-key")
e/get-window-handle (fn [_] "ABCDEFG")
proc/kill (fn [_]
(swap! kill-calls inc))
e/delete-session identity
Expand Down
Loading