Random error testing `test-combined-actions`
Closed this issue · 13 comments
Version
SHA: 1c611d which is in my own private branch, but I don't think it has anything to do with the version since the error is with a test that hasn't been touched by me and the failure is transient.
Platform
Mac OS X
version: 15.0
arch: aarch64
Java 17.0.12 - /usr/bin/java
openjdk version "17.0.12" 2024-07-16 LTS
OpenJDK Runtime Environment Zulu17.52+17-CA (build 17.0.12+7-LTS)
OpenJDK 64-Bit Server VM Zulu17.52+17-CA (build 17.0.12+7-LTS, mixed mode, sharing)
Babashka 1.4.192 - /opt/homebrew/bin/bb
babashka v1.4.192
Image Magick 7.1.1 - /opt/homebrew/bin/magick
Version: ImageMagick 7.1.1-38 Q16-HDRI aarch64 22398 https://imagemagick.org
Chrome 129.0.6668.59 - /Applications/Google Chrome.app
129.0.6668.59
Chrome Webdriver 129.0.6668.58 - /opt/homebrew/bin/chromedriver
ChromeDriver 129.0.6668.58 (81a06fb873a9b386848719cf9f93e59579fb5d4b-refs/branch-heads/6668@{#1318})
Firefox 130.0.1 - /Applications/Firefox.app
130.0.1
Firefox Webdriver 0.35.0 - /opt/homebrew/bin/geckodriver
geckodriver 0.35.0
Edge 129.0.2792.52 - /Applications/Microsoft Edge.app
129.0.2792.52
Edge Webdriver 129.0.2792.46 - /Users/dave/bin/msedgedriver
Microsoft Edge WebDriver 129.0.2792.46 (0d180bf4cf2beea3c5eaa92922c3f33004c1dd1c)
Safari 18.0 - /Applications/Safari.app
18.0
Safari Webdriver 18.0 - /System/Cryptexes/App/usr/bin/safaridriver
Included with Safari 18.0 (20619.1.26.31.6)
Warnings
- Version mismatch: Chrome 129.0.6668.59 != Chrome Webdriver 129.0.6668.58
- Version mismatch: Edge 129.0.2792.52 != Edge Webdriver 129.0.2792.46
Symptom
The test-combined-actions test failed randomly. See output below.
Reproduction
Cannot easily reproduce. There is a race condition of some sort. Rerunning the test suite resulted in no errors.
Actual behavior
Running the test suite resulted in the following failure.
=== test-combined-actions [jvm][safari]
.... input key and mouse click
FAIL in (test-combined-actions) (api_test.clj:1232)
input key and mouse click
expected: (str/ends-with? (e/get-url *driver*) "?login=1&password=2&message=3")
actual: (not (str/ends-with? "http://localhost:57030/test.html" "?login=1&password=2&message=3"))
Expected behavior
I would expect the test suite to pass and not fail randomly.
Diagnosis
None.
Here's the source for test-combine-actions
(deftest test-combined-actions
(testing "input key and mouse click"
(let [input (e/query *driver* :simple-input)
password (e/query *driver* :simple-password)
textarea (e/query *driver* :simple-textarea)
submit (e/query *driver* :simple-submit)
keyboard (-> (e/make-key-input)
e/add-double-pause
(e/with-key-down "\uE01B")
e/add-double-pause
(e/with-key-down "\uE01C")
e/add-double-pause
(e/with-key-down "\uE01D"))
mouse (-> (e/make-mouse-input)
(e/add-pointer-click-el input)
e/add-pause
(e/add-pointer-click-el password)
e/add-pause
(e/add-pointer-click-el textarea)
e/add-pause
(e/add-pointer-click-el submit))]
(e/perform-actions *driver* keyboard mouse)
(e/wait 1)
(is (str/ends-with? (e/get-url *driver*) "?login=1&password=2&message=3")))))I think the problem here is that submission as performed by a keystroke or a pointer click (e.g., add-pointer-click-el) via perform-actions results in a form submission, but perform-actions doesn't wait for the new page returned by the server to load. Unlike go, perform-actions returns as soon as the last action is performed, but before the consequences of that action (a form submission and response page return) is fully completed. The (e/wait 1) in the code is just throwing in a delay, but it doesn't guarantee that the page load has occurred or that the browser has updated the URL such that the call to get-url will return the new URL. Rather than an arbitrary delay, we need code that waits for the URL to change from the original state, before the perform-actions, to something new before testing it against the expected URL parameters. Since this basic test is used throughout the test suite (i.e., testing the returned URL for a specific pattern after the form submission), we should probably settle on an idiom that works and use it throughout the test suite.
Hmmm... yeah. I suppose we could wait for the URL to be what we expect it to be... using wait-predicate perhaps.
Fix welcome!
OK, so I just came up with a new little macro, wait-url-change, that wraps a form that kicks off a page change without waiting and then waits for the URL to change before progressing. So, test-combined-actions now looks like the following. I also went through and updated all tests that rely on testing the URL to use this macro. In the process I found a couple of tests that were broken and actually not testing anything because of the way the tests were written. I'll submit a PR shortly.
(defmacro wait-url-change
[& body]
`(let [old-url# (e/get-url *driver*)]
~@body
(e/wait-predicate (fn [] (not= old-url# (e/get-url *driver*)))
{:timeout 30 ; 30 second timeout total
:interval 0.100 ;
:message "Timeout waiting for URL change"})))
(deftest test-combined-actions
(testing "input key and mouse click"
(let [input (e/query *driver* :simple-input)
password (e/query *driver* :simple-password)
textarea (e/query *driver* :simple-textarea)
submit (e/query *driver* :simple-submit)
keyboard (-> (e/make-key-input)
e/add-double-pause
(e/with-key-down "\uE01B")
e/add-double-pause
(e/with-key-down "\uE01C")
e/add-double-pause
(e/with-key-down "\uE01D"))
mouse (-> (e/make-mouse-input)
(e/add-pointer-click-el input)
e/add-pause
(e/add-pointer-click-el password)
e/add-pause
(e/add-pointer-click-el textarea)
e/add-pause
(e/add-pointer-click-el submit))]
(wait-url-change
(e/perform-actions *driver* keyboard mouse))
(is (str/ends-with? (e/get-url *driver*) "?login=1&password=2&message=3")))))BTW, this also cuts out several generic and Safari-specific wait calls, which should speed up the test suite slightly.
Hm. I'm still seeing errors in CI, specifically with Firefox, which seems to do something odd with the URL when it's loading. The error is:
=== test-submit [bb][firefox]
FAIL in (test-submit) (/home/runner/work/etaoin/etaoin/test/etaoin/api_test.clj:203)
expected: (str/ends-with? (e/get-url *driver*) "?login=1&***")
actual: (not (str/ends-with? "about:blank" "?login=1&***"))
I don't see this on my MacOS development machine with the latest Firefox. I copied the above error from the CI output. It seems like Firefox might be setting the URL to about:blank for a period, and then setting it to the final URL later (so, two transitions). We wait for the first transition, but not the second and still end up with a race. I think we're headed in the right direction, but perhaps we need to wait for the URL to change and to contain a specific string or something, so we can avoid about:blank. I'll noodle on it. @lread , if you have any thoughts, let me know.
I like your idea of wait-url-change, but given the behaviour you've discovered with Firefox, it might not be as practical as waiting for a URL to change to a specific value or perhaps a regex.
I’ll make some changes and see if I can get something reliable.
Here's the new code. This works on my machine. Will push a commit as part of the existing PR and see if it passes CI tests.
(defmacro wait-url-change
[re & body]
`(let [old-url# (e/get-url *driver*)]
~@body
(e/wait-predicate (fn [] (let [new-url# (e/get-url *driver*)]
(and (not= old-url# new-url#)
(re-find ~re new-url#))))
{:timeout 30 ; 30 second timeout total
:interval 0.100 ;
:message "Timeout waiting for URL change"})))BTW, there may be a Firefox bug here that we should report. I would not expect the URL in the browser to change to "about:blank" following a form submission. I would expect it to transition to whatever the POST URL would be, possibly followed by another redirect from the server, as is typical with form submissions. But "about:blank" just seems odd.
Thanks, Dave!
Feel free to raise a bug against geckodriver and/or Firefox, but there are plenty of odd behaviours in WebDrivers and Browsers. If they aren't show-stoppers, I tend to ignore them and move on.