Skip to content

Commit

Permalink
dev: add Eastwood linting (#573)
Browse files Browse the repository at this point in the history
* 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.

* add/ignore lint stuff

- add some kondo lib config brought in by kondo
- git ignore .eastwood dir
  • Loading branch information
lread authored May 15, 2024
1 parent 18124a3 commit 6453b30
Show file tree
Hide file tree
Showing 22 changed files with 80 additions and 43 deletions.
1 change: 1 addition & 0 deletions .clj-kondo/taoensso/encore/config.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{:hooks {:analyze-call {taoensso.encore/defalias taoensso.encore/defalias}}}
16 changes: 16 additions & 0 deletions .clj-kondo/taoensso/encore/taoensso/encore.clj
Original file line number Diff line number Diff line change
@@ -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))}))
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ TAGS
*.iml
build.xml
/.idea
/.eastwood

# ignore cache under .clj-kondo and .lsp
.cache
Expand Down
6 changes: 5 additions & 1 deletion bb.edn
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,12 @@
(filterv (fn [p] (re-find pattern (:command p))))
(doric/table [:pid :start-instant :is-alive :command :arguments])
println))}
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"
Expand Down
10 changes: 9 additions & 1 deletion deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}}
Expand Down Expand Up @@ -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}
Expand Down
6 changes: 4 additions & 2 deletions src/etaoin/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@
java.text.SimpleDateFormat
(java.util Base64 Date)))

(set! *warn-on-reflection* true)

;;
;; WebDriver defaults
;;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/api2.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/dev.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/ide/flow.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/etaoin/ide/impl/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
[etaoin.impl.util :refer [defmethods]]
[etaoin.keys :as k]))

(set! *warn-on-reflection* true)

(defn absolute-path?
[path]
(-> path
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/etaoin/ide/impl/spec.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/ide/main.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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})

Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/impl/client.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
:clj [clj-http.client :as client])
[slingshot.slingshot :refer [throw+]]))

(set! *warn-on-reflection* true)

;;
;; defaults
;;
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/impl/driver.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion src/etaoin/impl/proc.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
13 changes: 5 additions & 8 deletions src/etaoin/impl/util.clj
Original file line number Diff line number Diff line change
@@ -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]))
Expand Down Expand Up @@ -77,14 +78,10 @@
"Return `url` with any http credentials stripped, https://user:[email protected] -> 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
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/impl/xpath.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/keys.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/etaoin/query.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
14 changes: 3 additions & 11 deletions test/etaoin/api_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,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)
Expand Down Expand Up @@ -389,8 +388,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)
Expand Down Expand Up @@ -426,7 +424,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)
Expand All @@ -438,12 +435,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')]"}]
Expand Down Expand Up @@ -541,9 +536,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})]
Expand Down Expand Up @@ -742,7 +735,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)
Expand Down
23 changes: 8 additions & 15 deletions test/etaoin/ide_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand Down Expand Up @@ -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"}))
4 changes: 2 additions & 2 deletions test/etaoin/unit/unit_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@
(let [profile-path (str (fs/file chrome-dir "chrome-profile"))]
(e/with-chrome {:profile profile-path} 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?
Expand Down

0 comments on commit 6453b30

Please sign in to comment.