From da97e2412a6c91af8a084a6e4f0fdd9016915f7a Mon Sep 17 00:00:00 2001 From: lread Date: Sat, 21 May 2022 14:32:09 -0400 Subject: [PATCH 1/4] road to bb: tweak process support Proc was referencing java.lang.IllegalThreadStateException, an exception that babashka is currently unaware of. Good news is that the code that referenced this is entirely unused. Deleted it along with other unused vars. Also: - marked private vars as such - tweaked proc-test to help maybe diagnose flakyness on Windows on CI --- src/etaoin/proc.clj | 31 ++++--------------------------- test/etaoin/proc_test.clj | 8 ++++++-- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/src/etaoin/proc.clj b/src/etaoin/proc.clj index c598bd76..d921c1a9 100644 --- a/src/etaoin/proc.clj +++ b/src/etaoin/proc.clj @@ -1,24 +1,22 @@ (ns etaoin.proc (:require [clojure.java.io :as io] - [clojure.string :as str]) - (:import java.lang.IllegalThreadStateException - java.io.IOException)) + [clojure.string :as str])) (def windows? (str/starts-with? (System/getProperty "os.name") "Windows")) -(defn get-null-file ^java.io.File +(defn- get-null-file ^java.io.File [] (if windows? (io/file "NUL") (io/file "/dev/null"))) -(defn get-log-file ^java.io.File +(defn- get-log-file ^java.io.File [file-path] (if file-path (io/file file-path) (get-null-file))) -(defn java-params ^"[Ljava.lang.String;" [params] +(defn- java-params ^"[Ljava.lang.String;" [params] (->> params (map str) (into-array String))) @@ -44,27 +42,6 @@ Please ensure you have the driver installed and specify the path to it. For driver installation, check out the official readme file from Etaoin: %s" binary readme-link) {:args args} e))))))) -;; todo store those streams - -(defn alive? [^Process proc] - (.isAlive proc)) - -(defn exit-code [^Process proc] - (try - (.exitValue proc) - (catch IllegalThreadStateException _))) - (defn kill [^Process proc] (.destroy proc)) -;; todo refactor those - -(defn read-out [^Process proc] - (try - (-> proc .getInputStream slurp) - (catch IOException _))) - -(defn read-err [^Process proc] - (try - (-> proc .getErrorStream slurp) - (catch IOException _))) diff --git a/test/etaoin/proc_test.clj b/test/etaoin/proc_test.clj index 293d8980..03323c7b 100644 --- a/test/etaoin/proc_test.clj +++ b/test/etaoin/proc_test.clj @@ -37,8 +37,12 @@ process (proc/run ["chromedriver" (format "--port=%d" port)]) _ (wait-running {:port port :host "localhost"})] (with-chrome {:args ["--no-sandbox"]} driver - (is (= 2 (get-count-chromedriver-instances))) - (proc/kill process)))) + ;; added to diagnose flakyness on windows on CI + (println "automatically chosen port->" (:port driver)) + ;; added to diagnose flakyness on windows on CI + (wait-running {:port (:port driver) :host "localhost"}) + (is (= 2 (get-count-chromedriver-instances)))) + (proc/kill process))) (testing "connect to driver" (let [port 9999 process (proc/run ["chromedriver" (format "--port=%d" port)]) From 29008371ae735c3684cac23d652f6729531514ea Mon Sep 17 00:00:00 2001 From: lread Date: Sat, 21 May 2022 14:47:16 -0400 Subject: [PATCH 2/4] A bit more diagnosis for process test on Windows --- test/etaoin/proc_test.clj | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/etaoin/proc_test.clj b/test/etaoin/proc_test.clj index 03323c7b..4c16485a 100644 --- a/test/etaoin/proc_test.clj +++ b/test/etaoin/proc_test.clj @@ -9,12 +9,14 @@ (defn get-count-chromedriver-instances [] (if proc/windows? - (->> (sh "powershell" "-command" "(Get-Process chromedriver -ErrorAction SilentlyContinue).Path") - :out - str/split-lines - (remove #(str/includes? % "\\scoop\\shims\\")) ;; for the scoop users, exclude the shim process - (filter #(str/includes? % "chromedriver")) - count) + (let [instance-report (-> (sh "powershell" "-command" "(Get-Process chromedriver -ErrorAction SilentlyContinue).Path") + :out + str/split-lines)] + (println "windows chromedriver instance report:" instance-report) + (->> instance-report + (remove #(str/includes? % "\\scoop\\shims\\")) ;; for the scoop users, exclude the shim process + (filter #(str/includes? % "chromedriver")) + count)) (->> (sh "sh" "-c" "ps aux") :out str/split-lines From 3c715619cc8381ffcaa984fcebeca46070674515 Mon Sep 17 00:00:00 2001 From: lread Date: Sat, 21 May 2022 15:27:27 -0400 Subject: [PATCH 3/4] more Windows proc flaky diagnosis --- test/etaoin/proc_test.clj | 62 +++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/test/etaoin/proc_test.clj b/test/etaoin/proc_test.clj index 4c16485a..787be125 100644 --- a/test/etaoin/proc_test.clj +++ b/test/etaoin/proc_test.clj @@ -12,7 +12,12 @@ (let [instance-report (-> (sh "powershell" "-command" "(Get-Process chromedriver -ErrorAction SilentlyContinue).Path") :out str/split-lines)] + ;; more flakiness diagnosis (println "windows chromedriver instance report:" instance-report) + (println "windows full list of running processes:") + (clojure.pprint/pprint (-> (sh "powershell" "-command" "Get-Process | Select Name, Path") + :out + str/split-lines)) (->> instance-report (remove #(str/includes? % "\\scoop\\shims\\")) ;; for the scoop users, exclude the shim process (filter #(str/includes? % "chromedriver")) @@ -23,33 +28,34 @@ (filter #(str/includes? % "chromedriver")) count))) -(deftest test-prevent-process-fork - (testing "certain driver port" - (let [port 9999 - process (proc/run ["chromedriver" (format "--port=%d" port)]) - _ (wait-running {:port port :host "localhost"})] - (is (= 1 (get-count-chromedriver-instances))) - (is (thrown-with-msg? - clojure.lang.ExceptionInfo - #"already in use" - (chrome {:port port}))) - (proc/kill process))) - (testing "random driver port" - (let [port 9999 - process (proc/run ["chromedriver" (format "--port=%d" port)]) - _ (wait-running {:port port :host "localhost"})] - (with-chrome {:args ["--no-sandbox"]} driver +(deftest test-process-forking-port-specified + (let [port 9999 + process (proc/run ["chromedriver" (format "--port=%d" port)]) + _ (wait-running {:port port :host "localhost"})] + (is (= 1 (get-count-chromedriver-instances))) + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"already in use" + (chrome {:port port}))) + (proc/kill process))) + +(deftest test-process-forking-port-random + (let [port 9999 + process (proc/run ["chromedriver" (format "--port=%d" port)]) + _ (wait-running {:port port :host "localhost"})] + (with-chrome {:args ["--no-sandbox"]} driver ;; added to diagnose flakyness on windows on CI - (println "automatically chosen port->" (:port driver)) + (println "automatically chosen port->" (:port driver)) ;; added to diagnose flakyness on windows on CI - (wait-running {:port (:port driver) :host "localhost"}) - (is (= 2 (get-count-chromedriver-instances)))) - (proc/kill process))) - (testing "connect to driver" - (let [port 9999 - process (proc/run ["chromedriver" (format "--port=%d" port)]) - _ (wait-running {:port port :host "localhost"}) - driver (chrome {:host "localhost" :port port :args ["--no-sandbox"]})] - (is (= 1 (get-count-chromedriver-instances))) - (quit driver) - (proc/kill process)))) + (wait-running {:port (:port driver) :host "localhost"}) + (is (= 2 (get-count-chromedriver-instances)))) + (proc/kill process))) + +(deftest test-process-forking-connect-existing + (let [port 9999 + process (proc/run ["chromedriver" (format "--port=%d" port)]) + _ (wait-running {:port port :host "localhost"}) + driver (chrome {:host "localhost" :port port :args ["--no-sandbox"]})] + (is (= 1 (get-count-chromedriver-instances))) + (quit driver) + (proc/kill process))) From 12b1318596753db95b47acd821c634f727e19d64 Mon Sep 17 00:00:00 2001 From: lread Date: Sat, 21 May 2022 16:19:49 -0400 Subject: [PATCH 4/4] More windows flakiness proc testing diagnosis Add a .waitFor after killing process. Move proc-tests out from under `testing` to their own `deftest`s. Have each test use its own port. And dump processes that are running in additon to chromedriver processes found. We'll remove these last bit after we achieve stability. --- src/etaoin/proc.clj | 4 ++-- test/etaoin/proc_test.clj | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/etaoin/proc.clj b/src/etaoin/proc.clj index d921c1a9..1f5986a0 100644 --- a/src/etaoin/proc.clj +++ b/src/etaoin/proc.clj @@ -43,5 +43,5 @@ For driver installation, check out the official readme file from Etaoin: %s" bin {:args args} e))))))) (defn kill [^Process proc] - (.destroy proc)) - + (.destroy proc) + (.waitFor proc)) diff --git a/test/etaoin/proc_test.clj b/test/etaoin/proc_test.clj index 787be125..34c2ba35 100644 --- a/test/etaoin/proc_test.clj +++ b/test/etaoin/proc_test.clj @@ -4,6 +4,7 @@ [clojure.test :refer :all] [etaoin.proc :as proc] [etaoin.test-report] + [clojure.pprint :as pprint] [clojure.string :as str])) (defn get-count-chromedriver-instances @@ -15,9 +16,10 @@ ;; more flakiness diagnosis (println "windows chromedriver instance report:" instance-report) (println "windows full list of running processes:") - (clojure.pprint/pprint (-> (sh "powershell" "-command" "Get-Process | Select Name, Path") - :out - str/split-lines)) + ;; use Get-CimInstance, because Get-Process, does not have commandline available + (pprint/pprint (-> (sh "powershell" "-command" "Get-CimInstance Win32_Process | select name, commandline") + :out + str/split-lines)) (->> instance-report (remove #(str/includes? % "\\scoop\\shims\\")) ;; for the scoop users, exclude the shim process (filter #(str/includes? % "chromedriver")) @@ -29,7 +31,7 @@ count))) (deftest test-process-forking-port-specified - (let [port 9999 + (let [port 9997 process (proc/run ["chromedriver" (format "--port=%d" port)]) _ (wait-running {:port port :host "localhost"})] (is (= 1 (get-count-chromedriver-instances))) @@ -40,14 +42,14 @@ (proc/kill process))) (deftest test-process-forking-port-random - (let [port 9999 + (let [port 9998 process (proc/run ["chromedriver" (format "--port=%d" port)]) _ (wait-running {:port port :host "localhost"})] (with-chrome {:args ["--no-sandbox"]} driver ;; added to diagnose flakyness on windows on CI (println "automatically chosen port->" (:port driver)) ;; added to diagnose flakyness on windows on CI - (wait-running {:port (:port driver) :host "localhost"}) + (wait-running driver) (is (= 2 (get-count-chromedriver-instances)))) (proc/kill process))) @@ -56,6 +58,7 @@ process (proc/run ["chromedriver" (format "--port=%d" port)]) _ (wait-running {:port port :host "localhost"}) driver (chrome {:host "localhost" :port port :args ["--no-sandbox"]})] + (wait-running driver) (is (= 1 (get-count-chromedriver-instances))) (quit driver) (proc/kill process)))