Random error testing test-combined-actions

[dgr]

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.

[dgr]

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.

[lread]

Hmmm... yeah. I suppose we could wait for the URL to be what we expect it to be... using wait-predicate perhaps.

[lread]

The root problem is probably the same for #646.

[lread]

Fix welcome!

[dgr]

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")))))

[dgr]

And yes, I think this is the same problem as #646.

[dgr]

BTW, this also cuts out several generic and Safari-specific wait calls, which should speed up the test suite slightly.

[dgr]

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.

[lread]

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.

[dgr]

I’ll make some changes and see if I can get something reliable.

[dgr]

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"})))

[dgr]

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.

[lread]

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.