From 12497ba391874387a89c95ea3d2e0eacca25a11a Mon Sep 17 00:00:00 2001 From: lread Date: Thu, 9 May 2024 12:37:14 -0400 Subject: [PATCH 1/2] dev: add Eastwood linting For some other Etaoin wip, I wanted to address some reflection warnings for new code and noticed we did not have any existing checks for reflection. I find Eastwood helps out with this nicely. In addition to reflection warnings, I also addressed some other issues Eastwood reported: - A booboo in a test assertion for `etaoin.unit.unit-test/test-chrome-profile`. Fixed! - URL constructors are deprecated, starting with JDK 20. I switched to using lambdaisland/uri. - it noticed `is` assert messages that were not obviously strings. I ^String type hinted the message generating fn. - it thought string `"always"` for a constant truthy looked suspicious. I switched to the more conventional `:always`. - several tests had empty assertions like `(is true "text found")` or `(is 1)`. Perhaps some linter once warned about a test without assertions? We don't have such a thing today, so I turfed these meaningless assertions. The unwritten assertion for these tests is that they do not throw an exception. --- bb.edn | 6 +++++- deps.edn | 10 +++++++++- src/etaoin/api.clj | 6 ++++-- src/etaoin/api2.clj | 2 ++ src/etaoin/dev.clj | 2 ++ src/etaoin/ide/flow.clj | 2 ++ src/etaoin/ide/impl/api.clj | 6 ++++-- src/etaoin/ide/impl/spec.clj | 1 + src/etaoin/ide/main.clj | 2 ++ src/etaoin/impl/client.cljc | 2 ++ src/etaoin/impl/driver.clj | 2 ++ src/etaoin/impl/proc.cljc | 4 +++- src/etaoin/impl/util.clj | 13 +++++-------- src/etaoin/impl/xpath.clj | 2 ++ src/etaoin/keys.clj | 2 ++ src/etaoin/query.clj | 2 ++ test/etaoin/api_test.clj | 14 +++----------- test/etaoin/ide_test.clj | 23 ++++++++--------------- test/etaoin/unit/unit_test.clj | 4 ++-- 19 files changed, 62 insertions(+), 43 deletions(-) diff --git a/bb.edn b/bb.edn index 6da23a45..f02343c5 100644 --- a/bb.edn +++ b/bb.edn @@ -58,8 +58,12 @@ :task test-matrix/-main} drivers {:doc "[list|kill] any running WebDrivers" :task drivers/-main} - lint {:doc "[--rebuild] lint source code" + lint-kondo {:doc "[--rebuild] lint source code with clj-kondo" :task lint/-main} + lint-eastwood {:doc "Run Eastwood linter on source code" + :task (shell/clojure "-M:eastwood")} + lint {:doc "Run all linters" + :depends [lint-kondo lint-eastwood]} cljdoc-preview {:doc "preview what docs will look like on cljdoc, use --help for args" :task cljdoc-preview/-main} tools-versions {:doc "report on tools versions" diff --git a/deps.edn b/deps.edn index a6e9e3e1..d969c3eb 100644 --- a/deps.edn +++ b/deps.edn @@ -7,7 +7,8 @@ slingshot/slingshot {:mvn/version "0.12.2"} cheshire/cheshire {:mvn/version "5.12.0"} org.clojure/tools.cli {:mvn/version "1.1.230"} - org.clojure/tools.logging {:mvn/version "1.3.0"}} + org.clojure/tools.logging {:mvn/version "1.3.0"} + lambdaisland/uri {:mvn/version "1.19.155"}} :aliases {:1.11 {:replace-deps {org.clojure/clojure {:mvn/version "1.11.2"}}} :1.12 {:replace-deps {org.clojure/clojure {:mvn/version "1.12.0-alpha9"}}} @@ -47,6 +48,13 @@ :clj-kondo {:extra-deps {clj-kondo/clj-kondo {:mvn/version "2024.03.13"}} :main-opts ["-m" "clj-kondo.main"]} + :eastwood {:extra-deps {jonase/eastwood {:mvn/version "1.4.2"}} + :extra-paths ["test"] + :main-opts ["-m" "eastwood.lint" {:source-paths ["src"] + :test-paths ["test"] + :add-linters [:performance] + :exclude-linters [:local-shadows-var]}]} + :build {:deps {io.github.clojure/tools.build {:mvn/version "0.10.0"} slipset/deps-deploy {:mvn/version "0.2.2"}} :ns-default build} diff --git a/src/etaoin/api.clj b/src/etaoin/api.clj index 6d795483..dc9d79cc 100644 --- a/src/etaoin/api.clj +++ b/src/etaoin/api.clj @@ -160,6 +160,8 @@ java.text.SimpleDateFormat (java.util Base64 Date))) +(set! *warn-on-reflection* true) + ;; ;; WebDriver defaults ;; @@ -2813,7 +2815,7 @@ "Clojure returns a seq of chars for a string. This does not handle wide (unicode) characters. Here we return a seq of codepoint strings for string `s`." - [s] + [^String s] (->> s .codePoints .iterator @@ -3166,7 +3168,7 @@ (defn- b64-decode [s] (-> (Base64/getDecoder) - (.decode s))) + (.decode ^String s))) (defmulti ^:private b64-to-file "Dumps a Base64-encoded string into a file. diff --git a/src/etaoin/api2.clj b/src/etaoin/api2.clj index 11fa7c25..b1e7b924 100644 --- a/src/etaoin/api2.clj +++ b/src/etaoin/api2.clj @@ -3,6 +3,8 @@ (:require [etaoin.api :as e])) +(set! *warn-on-reflection* true) + (defmacro with-firefox "Executes `body` with a Firefox driver session bound to `bind`. diff --git a/src/etaoin/dev.clj b/src/etaoin/dev.clj index f53f1ec1..4a49e00b 100644 --- a/src/etaoin/dev.clj +++ b/src/etaoin/dev.clj @@ -5,6 +5,8 @@ [clojure.string :as str] [etaoin.api :as api])) +(set! *warn-on-reflection* true) + (defn- try-parse-int [line] (try (Integer/parseInt line) diff --git a/src/etaoin/ide/flow.clj b/src/etaoin/ide/flow.clj index 96b7757f..5ed9b607 100644 --- a/src/etaoin/ide/flow.clj +++ b/src/etaoin/ide/flow.clj @@ -8,6 +8,8 @@ [etaoin.ide.impl.api :refer [run-command-with-log str->var]] [etaoin.ide.impl.spec :as spec])) +(set! *warn-on-reflection* true) + (declare execute-commands) (defn- execute-branch diff --git a/src/etaoin/ide/impl/api.clj b/src/etaoin/ide/impl/api.clj index 28cc4788..6c7712bf 100644 --- a/src/etaoin/ide/impl/api.clj +++ b/src/etaoin/ide/impl/api.clj @@ -12,6 +12,8 @@ [etaoin.impl.util :refer [defmethods]] [etaoin.keys :as k])) +(set! *warn-on-reflection* true) + (defn absolute-path? [path] (-> path @@ -158,7 +160,7 @@ (str base-url "/" target))) (defn make-assert-msg - [command actual expected] + ^String [command actual expected] (format "\nAssert command:\"%s\"\nExpected: %s\nActual: %s" (name command) expected actual)) @@ -628,7 +630,7 @@ (not (str/blank? target)) (str (format " on '%s'" target)) - "always" + :always (str (format "Command: %s" (name command))))) (defn run-command-with-log diff --git a/src/etaoin/ide/impl/spec.clj b/src/etaoin/ide/impl/spec.clj index ddbf30ec..23185fea 100644 --- a/src/etaoin/ide/impl/spec.clj +++ b/src/etaoin/ide/impl/spec.clj @@ -5,6 +5,7 @@ (:require [clojure.spec.alpha :as s])) +(set! *warn-on-reflection* true) (def control-flow-commands #{:do :times :while :forEach :if :elseIf :else :end :repeatIf}) diff --git a/src/etaoin/ide/main.clj b/src/etaoin/ide/main.clj index 7c2a0d6d..5291036e 100644 --- a/src/etaoin/ide/main.clj +++ b/src/etaoin/ide/main.clj @@ -18,6 +18,8 @@ [etaoin.ide.flow :as flow] [etaoin.impl.util :as util])) +(set! *warn-on-reflection* true) + (def ^:private browsers-set #{:chrome :safari :firefox :edge :phantom}) diff --git a/src/etaoin/impl/client.cljc b/src/etaoin/impl/client.cljc index 8700f00d..31b6eb7a 100644 --- a/src/etaoin/impl/client.cljc +++ b/src/etaoin/impl/client.cljc @@ -9,6 +9,8 @@ :clj [clj-http.client :as client]) [slingshot.slingshot :refer [throw+]])) +(set! *warn-on-reflection* true) + ;; ;; defaults ;; diff --git a/src/etaoin/impl/driver.clj b/src/etaoin/impl/driver.clj index 421e0a43..85537dd7 100644 --- a/src/etaoin/impl/driver.clj +++ b/src/etaoin/impl/driver.clj @@ -39,6 +39,8 @@ [clojure.tools.logging :as log] [etaoin.impl.util :refer [deep-merge defmethods]])) +(set! *warn-on-reflection* true) + (defn dispatch-driver [driver & _] (:type driver)) diff --git a/src/etaoin/impl/proc.cljc b/src/etaoin/impl/proc.cljc index afe4844c..089508dd 100644 --- a/src/etaoin/impl/proc.cljc +++ b/src/etaoin/impl/proc.cljc @@ -6,6 +6,8 @@ [babashka.process.pprint]]) ;; to support exception rendering in REPL [clojure.string :as str])) +(set! *warn-on-reflection* true) + (def windows? (str/starts-with? (System/getProperty "os.name") "Windows")) (defn- get-null-file ^java.io.File @@ -61,4 +63,4 @@ For driver installation, check out the Etaoin user guide: %s" binary user-guide- (defn alive? "Check if `p` has died unexpectedly, use [[result]] to get result." [p] - (.isAlive (:proc p))) + (p/alive? p)) diff --git a/src/etaoin/impl/util.clj b/src/etaoin/impl/util.clj index c8bcf0d7..48c473a3 100644 --- a/src/etaoin/impl/util.clj +++ b/src/etaoin/impl/util.clj @@ -1,4 +1,5 @@ (ns ^:no-doc etaoin.impl.util + (:require [lambdaisland.uri :as uri]) (:import [java.io File IOException] [java.net InetSocketAddress ServerSocket Socket])) @@ -77,14 +78,10 @@ "Return `url` with any http credentials stripped, https://user:pass@hello.com -> https://hello.com. Use when logging urls to avoid spilling secrets." ^String [^String url] - (let [u (java.net.URL. url)] - (.toExternalForm - (java.net.URL. - (.getProtocol u) - (.getHost u) - (.getPort u) - (.getFile u) - (.getRef u))))) + (-> url + uri/parse + (dissoc :user :password) + uri/uri-str)) (defn assoc-some "Associates a key with a value in a map, if and only if the value is diff --git a/src/etaoin/impl/xpath.clj b/src/etaoin/impl/xpath.clj index 8a70989f..9a7ea987 100644 --- a/src/etaoin/impl/xpath.clj +++ b/src/etaoin/impl/xpath.clj @@ -2,6 +2,8 @@ "A special module to work with XPath language." (:require [clojure.string :as string])) +(set! *warn-on-reflection* true) + (def Q \") (defn node-by-text diff --git a/src/etaoin/keys.clj b/src/etaoin/keys.clj index d3936127..285676ec 100644 --- a/src/etaoin/keys.clj +++ b/src/etaoin/keys.clj @@ -3,6 +3,8 @@ Sourced from [WebDriver spec](https://www.w3.org/TR/webdriver/#keyboard-actions).") +(set! *warn-on-reflection* true) + (def unidentified \uE000) (def cancel \uE001) (def help \uE002) diff --git a/src/etaoin/query.clj b/src/etaoin/query.clj index 21b177fd..f918a774 100644 --- a/src/etaoin/query.clj +++ b/src/etaoin/query.clj @@ -8,6 +8,8 @@ [etaoin.impl.util :as util] [etaoin.impl.xpath :as xpath])) +(set! *warn-on-reflection* true) + ;; todo duplicates with api.clj (def locator-xpath "xpath") (def locator-css "css selector") diff --git a/test/etaoin/api_test.clj b/test/etaoin/api_test.clj index f8d0ead4..407330e4 100644 --- a/test/etaoin/api_test.clj +++ b/test/etaoin/api_test.clj @@ -344,8 +344,7 @@ (e/refresh) (e/wait-visible {:id :document-end}) (e/click {:id :wait-button}) - (e/wait-has-text :wait-span "-secret-")) - (is true "text found")) + (e/wait-has-text :wait-span "-secret-"))) (testing "wait for text timeout" (doto *driver* (e/refresh) @@ -388,8 +387,7 @@ (e/refresh) (e/wait-visible {:id :document-end}) (e/click {:id :wait-button}) - (e/wait-has-text-everywhere "-secret-")) - (is true "text found")) + (e/wait-has-text-everywhere "-secret-"))) (testing "wait for text timeout" (doto *driver* (e/refresh) @@ -425,7 +423,6 @@ :times 7})))))) (deftest test-wait-has-class - (is 1) (testing "wait for an element has class" (doto *driver* (e/scroll-query :wait-add-class-trigger) @@ -437,12 +434,10 @@ :message "No 'new-one' class found."})))) (deftest test-close-window - (is 1) (doto *driver* (e/close-window))) (deftest test-drag-n-drop - (is 1) (let [url (test-server-url "drag-n-drop/index.html") doc {:class :document} trash {:xpath "//div[contains(@class, 'trash')]"}] @@ -529,9 +524,7 @@ (e/click {:id :set-active-el}) (-> (e/get-element-attr :active :id) (= "active-el-input") - is))) - (e/when-safari *driver* - (is 1)))) + is))))) (deftest test-element-text (let [text (e/get-element-text *driver* {:id :element-text})] @@ -730,7 +723,6 @@ (e/click *driver* :non-existing-element)) (is false "should be caught") (catch Exception _e - (is true "caught") (let [files (file-seq (fs/file dir-tmp)) expected-file-count (if (e/supports-logs? *driver*) 3 2)] (is (= (-> files rest count) diff --git a/test/etaoin/ide_test.clj b/test/etaoin/ide_test.clj index 6ac6393e..297bb95b 100644 --- a/test/etaoin/ide_test.clj +++ b/test/etaoin/ide_test.clj @@ -2,7 +2,7 @@ (:require [clojure.edn :as edn] [clojure.java.io :as io] - [clojure.test :refer [deftest is testing use-fixtures]] + [clojure.test :refer [deftest testing use-fixtures]] [etaoin.api :as e] [etaoin.ide.flow :as ide] [etaoin.test-report :as test-report])) @@ -54,35 +54,28 @@ (deftest test-asserts (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-asserts"}) - (is 1)) + {:base-url *base-url* :test-name "test-asserts"})) (deftest test-click-type-select (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-click-type-select"}) - (is 1)) + {:base-url *base-url* :test-name "test-click-type-select"})) (deftest test-drag-n-drop (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-drag-n-drop"}) - (is 1)) + {:base-url *base-url* :test-name "test-drag-n-drop"})) (deftest test-select-window (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-select-window"}) - (is 1)) + {:base-url *base-url* :test-name "test-select-window"})) (deftest test-send-keys (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-send-keys"}) - (is 1)) + {:base-url *base-url* :test-name "test-send-keys"})) (deftest test-control-flow (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-control-flow"}) - (is 1)) + {:base-url *base-url* :test-name "test-control-flow"})) (deftest test-wait-for (ide/run-ide-script *driver* *test-file-path* - {:base-url *base-url* :test-name "test-wait-for"}) - (is 1)) + {:base-url *base-url* :test-name "test-wait-for"})) diff --git a/test/etaoin/unit/unit_test.clj b/test/etaoin/unit/unit_test.clj index 28f36262..19d303ca 100644 --- a/test/etaoin/unit/unit_test.clj +++ b/test/etaoin/unit/unit_test.clj @@ -98,8 +98,8 @@ (let [profile-path (str (fs/file chrome-dir "chrome-profile"))] (e/with-chrome {:profile profile-path :args ["--no-sandbox"]} driver (e/go driver "chrome://version") - (is profile-path - (e/get-element-text driver :profile_path)))))) + (is (= profile-path + (e/get-element-text driver :profile_path))))))) (deftest test-fail-run-driver (is (thrown-with-msg? From dbcdbe249435578c4d86573e215bb838947902a6 Mon Sep 17 00:00:00 2001 From: lread Date: Wed, 15 May 2024 12:58:32 -0400 Subject: [PATCH 2/2] add/ignore lint stuff - add some kondo lib config brought in by kondo - git ignore .eastwood dir --- .clj-kondo/taoensso/encore/config.edn | 1 + .clj-kondo/taoensso/encore/taoensso/encore.clj | 16 ++++++++++++++++ .gitignore | 1 + 3 files changed, 18 insertions(+) create mode 100644 .clj-kondo/taoensso/encore/config.edn create mode 100644 .clj-kondo/taoensso/encore/taoensso/encore.clj diff --git a/.clj-kondo/taoensso/encore/config.edn b/.clj-kondo/taoensso/encore/config.edn new file mode 100644 index 00000000..7b0ff3c2 --- /dev/null +++ b/.clj-kondo/taoensso/encore/config.edn @@ -0,0 +1 @@ +{:hooks {:analyze-call {taoensso.encore/defalias taoensso.encore/defalias}}} diff --git a/.clj-kondo/taoensso/encore/taoensso/encore.clj b/.clj-kondo/taoensso/encore/taoensso/encore.clj new file mode 100644 index 00000000..7f6d30ac --- /dev/null +++ b/.clj-kondo/taoensso/encore/taoensso/encore.clj @@ -0,0 +1,16 @@ +(ns taoensso.encore + (:require + [clj-kondo.hooks-api :as hooks])) + +(defn defalias [{:keys [node]}] + (let [[sym-raw src-raw] (rest (:children node)) + src (if src-raw src-raw sym-raw) + sym (if src-raw + sym-raw + (symbol (name (hooks/sexpr src))))] + {:node (with-meta + (hooks/list-node + [(hooks/token-node 'def) + (hooks/token-node (hooks/sexpr sym)) + (hooks/token-node (hooks/sexpr src))]) + (meta src))})) diff --git a/.gitignore b/.gitignore index 1b65612d..84ca9538 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ TAGS *.iml build.xml /.idea +/.eastwood # ignore cache under .clj-kondo and .lsp .cache