Remove chrome's --no-sandbox from tests (#576)

Our tests included the `--no-sandbox` option for chrome.

This seems to have been harmless until recently.
On Windows, it now results in orphaned chrome processes which results in
tests bogging and then, after a long, while failing.

Removing `--no-sandbox` when running on Windows resolves the issue.

The `--no-sandbox` option allows testing under a root user.
Because we don't run tests under root on any OS or environment,
I feel comfortable removing `--no-sandbox` across all OS testing.

This change also includes new bb task `ps`, a bare-bones cross-platform
way to report on running processes. This was useful while diagnosing the
issue so I've left it in.

I also switched from `babashka.process/destroy` to
`babashka.process/destroy-tree` when killing a web driver process.
This did not help with the Windows chrome orphan issue, but I expect it
is generally a better way to go.

Closes #572

CHANGELOG.adoc

@@ -11,6 +11,8 @@ A release with an intentional breaking changes is marked with:
 (https://github.com/tupini07[@tupini07])
 * https://github.com/clj-commons/etaoin/issues/566[#566]: Recognize `:driver-log-level` for Edge
 * bump all deps to current versions
+* tests
+** https://github.com/clj-commons/etaoin/issues/572[#572]: stop using chrome `--no-sandbox` option, it has become problematic on Windows (and we did not need it anyway)
 * docs
 ** https://github.com/clj-commons/etaoin/issues/534[#534]: better describe `etaoin.api/select` and its alternatives
 ** https://github.com/clj-commons/etaoin/issues/536[#536]: user guide examples are now all os agnostic and CI tested via test-doc-blocks on all supported OSes

bb.edn

@@ -58,6 +58,15 @@
                   :task test-matrix/-main}
   drivers        {:doc "[list|kill] any running WebDrivers"
                   :task drivers/-main}
+  ps             {:doc "List processes with matching names (handy for debugging)"
+                  :requires ([helper.ps :as ps]
+                             [doric.core :as doric]
+                             [clojure.string :as str])
+                  :task (let [pattern (re-pattern (str/join "|" *command-line-args*))]
+                          (->> (ps/all-processes)
+                               (filterv (fn [p] (re-find pattern (:command p))))
+                               (doric/table [:pid :start-instant :is-alive :command :arguments])
+                               println))}
   lint           {:doc "[--rebuild] lint source code"
                   :task lint/-main}
   cljdoc-preview {:doc "preview what docs will look like on cljdoc, use --help for args"

doc/01-user-guide.adoc

@@ -2850,7 +2850,7 @@ Here is a `clojure` example:
 
 [source,shell]
 ----
-clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888 :args ["--no-sandbox"]}' -r ide/test.side
+clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888}' -r ide/test.side
 ----
 
 As well as from an uberjar.

script/docker_install.clj

@@ -45,10 +45,10 @@
 
 (defn- wrap-chrome
   "The Selenium chrome docker image wraps the chrome launcher, so I'm going with the flow here.
-  I don't fully understand if --no-sandbox is required for docker images, but if Selenium is doing it,
-  I'm not interested in figuring out if we don't need to as well.
+  I think the --no-sandbox option is required when running as a root user which is often the case
+  for docker images.
 
-  They also do the umask thing... so mimicing that as well."
+  Selenium also do the umask thing... so mimicing that as well."
   []
   (status/line :head "Wrapping chrome launcher")
   (let [launcher (-> (shell/command {:out :string}

script/drivers.clj

@@ -1,11 +1,10 @@
 (ns drivers
   (:require [babashka.fs :as fs]
             [doric.core :as doric]
+            [helper.ps :as ps]
             [helper.os :as os]
             [helper.main :as main]
-            [lread.status-line :as status])
-  ;; noice! bb is JDK 11 so we have ProcessHandle
-  (:import (java.lang ProcessHandle)))
+            [lread.status-line :as status]))
 
 (def web-drivers
   [{:name "Chrome" :bin "chromedriver"}
@@ -14,19 +13,8 @@
    {:name "Safari" :bin "safaridriver"}
    {:name "PhantomJS" :bin "phantomjs"}])
 
-(defn all-processes []
-  (for [p (-> (ProcessHandle/allProcesses) .iterator iterator-seq)
-        :when (some-> p .info .command .isPresent)
-        :let [info (.info p)
-              command (-> info .command .get)
-              arguments (when (-> info .arguments .isPresent)
-                          (->> info .arguments .get (into [])))]]
-    {:handle p
-     :command command
-     :arguments arguments}))
-
 (defn driver-processes []
-  (->> (all-processes)
+  (->> (ps/all-processes)
        (keep (fn [p]
                (let [pfname (-> p :command fs/file-name)
                      pfname (if (= :win (os/get-os))
@@ -52,6 +40,7 @@
 (defn report[processes]
   (->> processes
        (doric/table (keep identity [{:name :name :title "WebDriver"}
+                                    :pid
                                     :command
                                     ;; arguments are don't seem to populate on Windows
                                     (when (not= :win (os/get-os)) :arguments)]))

script/helper/ps.clj

@@ -0,0 +1,18 @@
+(ns helper.ps
+  ;; noice! bb uses a modern JDK so we have ProcessHandle
+  (:import (java.lang ProcessHandle)))
+
+(defn all-processes []
+  (for [p (-> (ProcessHandle/allProcesses) .iterator iterator-seq)
+        :when (some-> p .info .command .isPresent)
+        :let [info (.info p)
+              command (-> info .command .get)
+              arguments (when (-> info .arguments .isPresent)
+                          (->> info .arguments .get (into [])))
+              start-instant (-> info .startInstant .get)]]
+    {:pid (.pid p)
+     :is-alive (.isAlive p)
+     :start-instant start-instant
+     :handle p
+     :command command
+     :arguments arguments}))

src/etaoin/ide/main.clj

@@ -4,7 +4,7 @@
   Example:
 
   ```shell
-  clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888 :args [\"--no-sandbox\"]}' -f /path/to/script.side
+  clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888}' -f /path/to/script.side
   ```
 
   See the [User Guide](/doc/01-user-guide.adoc#selenium-ide-cli) for more info.
@@ -63,7 +63,7 @@ This is a CLI interface for running Selenium IDE files.
 Usage examples:
 
 ;; from clojure
-clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888 :args [\"--no-sandbox\"]}' -r ide/test.side
+clojure -M -m etaoin.ide.main -d firefox -p '{:port 8888}' -r ide/test.side
 
 ;; from a jar
 java -cp .../poject.jar -m etaoin.ide.main -d firefox -p '{:port 8888}' -f ide/test.side

src/etaoin/impl/proc.cljc

@@ -49,7 +49,7 @@ For driver installation, check out the Etaoin user guide: %s" binary user-guide-
 (defn kill
   "Ask `p` to die. Use [[result]] to get exit code if you need it."
   [p]
-  (p/destroy p)
+  (p/destroy-tree p)
   @p)
 
 (defn result

test/etaoin/api_test.clj

@@ -40,7 +40,7 @@
   [:firefox :chrome :safari])
 
 (def default-opts
-  {:chrome  {:args ["--no-sandbox"]}
+  {:chrome  {}
    :firefox {}
    :safari  {}
    :edge    {:args ["--headless"]}})

test/etaoin/ide_test.clj

@@ -31,7 +31,7 @@
   (let [base-url       (-> "static" io/resource str)
         test-file-path (-> "ide/test.side" io/resource str)]
     (doseq [type drivers]
-      (e/with-driver type {:args ["--no-sandbox"]} driver
+      (e/with-driver type driver
         (e/go driver base-url)
         (binding [*driver*         driver
                   *base-url*       base-url

test/etaoin/unit/proc_test.clj

@@ -56,7 +56,7 @@
         process (proc/run ["chromedriver" (format "--port=%d" port)])]
     (try
       (e/wait-running {:port port :host "localhost"})
-      (e/with-chrome {:args ["--no-sandbox"]} driver
+      (e/with-chrome driver
         ;; added to diagnose flakyness on windows on CI
         (println "automatically chosen port->" (:port driver))
         ;; added to diagnose flakyness on windows on CI
@@ -71,7 +71,7 @@
     (try
       (e/wait-running {:port port :host "localhost"})
       ;; should connect, not launch
-      (let [driver (e/chrome {:host "localhost" :port port :args ["--no-sandbox"]})]
+      (let [driver (e/chrome {:host "localhost" :port port})]
         (is (= 1 (get-count-chromedriver-instances)))
         (e/quit driver))
       (finally
@@ -83,7 +83,7 @@
     (try
       (e/wait-running {:port port :host "localhost"})
       (let [;; should connect, not launch
-            driver  (e/chrome {:webdriver-url (format "http://localhost:%d" port) :args ["--no-sandbox"]})]
+            driver  (e/chrome {:webdriver-url (format "http://localhost:%d" port)})]
 
 
         (is (= 1 (get-count-chromedriver-instances)))
@@ -172,8 +172,7 @@
       (with-redefs [client/http-request (fn [_] (throw (ex-info "read timeout" {})))]
         ;; attempt connect, not launch
         (let [ex (try
-                   (e/with-chrome {:webdriver-url (format "http://localhost:%d" port)
-                                   :args ["--no-sandbox"]} _driver
+                   (e/with-chrome {:webdriver-url (format "http://localhost:%d" port)} _driver
                      (is false "should not get here"))
                    (catch Throwable ex
                      {:exception ex}))

test/etaoin/unit/unit_test.clj

@@ -96,7 +96,7 @@
 (deftest test-chrome-profile
   (fs/with-temp-dir [chrome-dir {:prefix "chrome-dir"}]
     (let [profile-path (str (fs/file chrome-dir "chrome-profile"))]
-      (e/with-chrome {:profile profile-path :args ["--no-sandbox"]} driver
+      (e/with-chrome {:profile profile-path} driver
         (e/go driver "chrome://version")
         (is profile-path
             (e/get-element-text driver :profile_path))))))