From 23725b4e9dcde164294b34966d3246ea3a909d08 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Fri, 16 Aug 2024 16:48:13 -0700
Subject: [PATCH] Misc Kondo Config cleanup (#46907)
* Kondo Config cleanup
* Restore warnings for with-log-messages-for-level until #28827 is merged
* Updated clojure.test hooks
* Fix kondo warning
* Time for me to learn to spell
---
.clj-kondo/config.edn | 163 ++++++++-
.clj-kondo/src/hooks/clojure/core.clj | 3 +-
.clj-kondo/src/hooks/clojure/test.clj | 173 ++-------
.clj-kondo/test/hooks/clojure/test_test.clj | 24 +-
.../advanced_config/file.clj | 3 +-
.../serialization/api_test.clj | 1 +
.../analyze/classifiers/name_test.clj | 2 +-
test/metabase/api/database_test.clj | 2 +-
test/metabase/cmd/copy_test.clj | 2 +-
test/metabase/db/custom_migrations_test.clj | 333 +++++++++--------
test/metabase/db/schema_migrations_test.clj | 1 +
.../driver/sql/parameters/substitute_test.clj | 342 +++++++++---------
.../drill_thru/underlying_records_test.cljc | 8 +-
13 files changed, 539 insertions(+), 518 deletions(-)
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index d9890ea380e..fad16ea34ed 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -20,14 +20,143 @@
:use {:level :warning}
:warn-on-reflection {:level :warning}
- :metabase/defsetting-must-specify-export {:level :warning}
- :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests {:level :warning}
- :metabase/mbql-query-first-arg {:level :warning}
- :metabase/missing-test-expr-requires-in-cljs {:level :warning}
- :metabase/require-shape-checker {:level :warning}
- :metabase/test-helpers-use-non-thread-safe-functions {:level :warning}
- :metabase/validate-deftest {:level :warning}
- :metabase/validate-logging {:level :warning}
+ :metabase/defsetting-must-specify-export {:level :warning}
+ :metabase/mbql-query-first-arg {:level :warning}
+ :metabase/missing-test-expr-requires-in-cljs {:level :warning}
+ :metabase/require-shape-checker {:level :warning}
+ :metabase/test-helpers-use-non-thread-safe-functions {:level :warning}
+ :metabase/validate-logging {:level :warning}
+
+ :metabase/validate-deftest
+ {:level :warning
+
+ ;; calling these functions will result in an error in tests marked `^:parallel`
+ ;;
+ ;; TODO -- these should all be made thread safe if possible or renamed so they end in `!`. Then we can remove them
+ ;; from this list because by default anything ending in `!` is considered to be thread-unsafe
+ :disallowed
+ #{clojure.core/alter-var-root
+ clojure.core/with-redefs
+ clojure.core/with-redefs-fn
+ metabase-enterprise.sandbox.test-util/with-user-attributes
+ metabase-enterprise.test/with-user-attributes
+ metabase.actions.test-util/with-actions
+ metabase.actions.test-util/with-actions-disabled
+ metabase.actions.test-util/with-actions-enabled
+ metabase.actions.test-util/with-actions-test-data
+ metabase.actions.test-util/with-actions-test-data-and-actions-enabled
+ metabase.actions.test-util/with-actions-test-data-tables
+ metabase.analytics.snowplow-test/with-fake-snowplow-collector
+ metabase.email-test/with-expected-messages
+ metabase.email-test/with-fake-inbox
+ metabase.test.data.users/with-group
+ metabase.test.data.users/with-group-for-user
+ metabase.test.persistence/with-persistence-enabled
+ metabase.test.util.log/with-log-level
+ metabase.test.util.misc/with-single-admin-user
+ metabase.test.util/with-all-users-permission
+ metabase.test.util/with-discarded-collections-perms-changes
+ metabase.test.util/with-env-keys-renamed-by
+ metabase.test.util/with-locale
+ metabase.test.util/with-non-admin-groups-no-root-collection-for-namespace-perms
+ metabase.test.util/with-non-admin-groups-no-root-collection-perms
+ metabase.test.util/with-temp-env-var-value
+ metabase.test.util/with-temp-vals-in-db
+ metabase.test.util/with-temporary-raw-setting-values
+ metabase.test.util/with-temporary-setting-values
+ metabase.test.util/with-user-in-groups
+ metabase.test/with-actions
+ metabase.test/with-actions-disabled
+ metabase.test/with-actions-enabled
+ metabase.test/with-actions-test-data
+ metabase.test/with-actions-test-data-and-actions-enabled
+ metabase.test/with-actions-test-data-tables
+ metabase.test/with-all-users-permission
+ metabase.test/with-discarded-collections-perms-changes
+ metabase.test/with-env-keys-renamed-by
+ metabase.test/with-expected-messages
+ metabase.test/with-fake-inbox
+ metabase.test/with-group
+ metabase.test/with-group-for-user
+ metabase.test/with-locale
+ metabase.test/with-log-level
+ metabase.test/with-non-admin-groups-no-root-collection-for-namespace-perms
+ metabase.test/with-non-admin-groups-no-root-collection-perms
+ metabase.test/with-persistence-enabled
+ metabase.test/with-single-admin-user
+ metabase.test/with-temp-env-var-value
+ metabase.test/with-temp-vals-in-db
+ metabase.test/with-temporary-raw-setting-values
+ metabase.test/with-temporary-setting-values
+ metabase.test/with-user-in-groups}
+
+ ;; these functions are allowed in `^:parallel` tests even tho they end in `!`
+ :allowed
+ #{clojure.core/assoc!
+ clojure.core/compare-and-set!
+ clojure.core/conj!
+ clojure.core/disj!
+ clojure.core/dissoc!
+ clojure.core/persistent!
+ clojure.core/pop!
+ clojure.core/reset!
+ clojure.core/reset-vals!
+ clojure.core/run!
+ clojure.core/swap!
+ clojure.core/swap-vals!
+ clojure.core/volatile!
+ clojure.core/vreset!
+ clojure.core/vswap!
+ clojure.core.async/<!
+ clojure.core.async/<!!
+ clojure.core.async/>!
+ clojure.core.async/>!!
+ clojure.core.async/alt!
+ clojure.core.async/alt!!
+ clojure.core.async/alts!
+ clojure.core.async/alts!!
+ clojure.core.async/close!
+ clojure.core.async/ioc-alts!
+ clojure.core.async/offer!
+ clojure.core.async/onto-chan!
+ clojure.core.async/onto-chan!!
+ clojure.core.async/poll!
+ clojure.core.async/put!
+ clojure.core.async/take!
+ clojure.core.async/to-chan!
+ clojure.core.async/to-chan!!
+ metabase.driver.sql-jdbc.execute/execute-prepared-statement!
+ metabase.pulse/send-pulse!
+ metabase.query-processor.store/store-database!
+ next.jdbc/execute!}}
+
+ :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests
+ {:level :warning
+ :max-length 180}
+
+ ;; this is not enabled here, it is enabled below only for specific namespaces.
+ :metabase/disallow-hardcoded-driver-names-in-tests
+ ;; don't include `:h2` since that will probably lead to a mountain of tests where we are specifically testing
+ ;; middleware or compilation behavior with just H2 since it's the default.
+ {:drivers #{ ;; core drivers
+ #_:h2
+ :postgres
+ :mysql
+ ;; module drivers
+ :athena
+ :bigquery-cloud-sdk
+ :druid
+ :druid-jdbc
+ :googleanalytics
+ :mongo
+ :oracle
+ :presto-jdbc
+ :redshift
+ :snowflake
+ :sparksql
+ :sqlite
+ :sqlserver
+ :vertica}}
;;
;; disabled linters
@@ -151,7 +280,7 @@
metabase.analytics #{metabase.analytics.prometheus
metabase.analytics.snowplow
metabase.analytics.stats
- metabase.analytics.sdk} ; TODO -- consolidate these into a real API namespace.
+ metabase.analytics.sdk} ; TODO -- consolidate these into a real API namespace.
metabase.analyze #{metabase.analyze}
metabase.api #{metabase.api.common
metabase.api.dataset
@@ -319,11 +448,11 @@
;; this is mostly intended for excluding test namespaces or those rare 'glue' namespaces that glue multiple modules
;; together, e.g. `metabase.lib.metadata.jvm`.
:ignored-namespace-patterns
- #{"-test$" ; anything ending in `-test`
- "test[-.]util" ; anything that contains `test.util` or `test-util`
- "test\\.impl" ; anything that contains `test.impl`
- "^metabase\\.test" ; anything starting with `metabase.test`
- "^metabase\\.http-client$"}} ; `metabase.http-client` which is a test-only namespace despite its name.
+ #{"-test$" ; anything ending in `-test`
+ "test[-.]util" ; anything that contains `test.util` or `test-util`
+ "test\\.impl" ; anything that contains `test.impl`
+ "^metabase\\.test" ; anything starting with `metabase.test`
+ "^metabase\\.http-client$"}} ; `metabase.http-client` which is a test-only namespace despite its name.
:consistent-alias
{:aliases
@@ -353,11 +482,7 @@
hiccup.util hiccup.util
honey.sql sql
honey.sql.helpers sql.helpers
- honeysql.core hsql
- honeysql.format hformat
- honeysql.helpers hh
- honeysql.types htypes
- java-time t
+ java-time.api t
macaw.core macaw
malli.core mc
malli.error me
diff --git a/.clj-kondo/src/hooks/clojure/core.clj b/.clj-kondo/src/hooks/clojure/core.clj
index 7a771e19ec7..d8fe34f5fbc 100644
--- a/.clj-kondo/src/hooks/clojure/core.clj
+++ b/.clj-kondo/src/hooks/clojure/core.clj
@@ -1,10 +1,11 @@
(ns hooks.clojure.core
(:require
[clj-kondo.hooks-api :as hooks]
- [clojure.set :as set]
[clojure.string :as str]
[hooks.common]))
+;;; TODO -- seems silly to maintain different blacklists and whitelists here than we use for the deftest `^:parallel`
+;;; checker... those lists live in the Clj Kondo config file
(def ^:private symbols-allowed-in-fns-not-ending-in-an-exclamation-point
'#{;; these toucan methods might actually set global values if it's used outside of a transaction,
;; but since mt/with-temp runs in a transaction, so we'll ignore them in this case.
diff --git a/.clj-kondo/src/hooks/clojure/test.clj b/.clj-kondo/src/hooks/clojure/test.clj
index 54e50edf3b3..8585dd8caf7 100644
--- a/.clj-kondo/src/hooks/clojure/test.clj
+++ b/.clj-kondo/src/hooks/clojure/test.clj
@@ -4,112 +4,7 @@
[clojure.string :as str]
[hooks.common]))
-(def ^:private disallowed-parallel-forms
- "Things you should not be allowed to use inside parallel tests. Besides these, anything ending in `!` not whitelisted
- in [[allowed-parallel-forms]] is not allowed."
- '#{clojure.core/alter-var-root
- clojure.core/with-redefs
- clojure.core/with-redefs-fn
- metabase-enterprise.sandbox.test-util/with-gtaps!
- metabase-enterprise.sandbox.test-util/with-gtaps-for-user!
- metabase-enterprise.sandbox.test-util/with-user-attributes
- metabase-enterprise.test/with-gtaps!
- metabase-enterprise.test/with-gtaps-for-user!
- metabase-enterprise.test/with-user-attributes
- metabase.actions.test-util/with-actions
- metabase.actions.test-util/with-actions-disabled
- metabase.actions.test-util/with-actions-enabled
- metabase.actions.test-util/with-actions-test-data
- metabase.actions.test-util/with-actions-test-data-and-actions-enabled
- metabase.actions.test-util/with-actions-test-data-tables
- metabase.analytics.snowplow-test/with-fake-snowplow-collector
- metabase.email-test/with-expected-messages
- metabase.email-test/with-fake-inbox
- metabase.test.data.users/with-group
- metabase.test.data.users/with-group-for-user
- metabase.test.persistence/with-persistence-enabled
- metabase.test.util.log/with-log-level
- metabase.test.util.misc/with-single-admin-user
- metabase.test.util/with-all-users-permission
- metabase.test.util/with-discarded-collections-perms-changes
- metabase.test.util/with-env-keys-renamed-by
- metabase.test.util/with-locale
- metabase.test.util/with-non-admin-groups-no-root-collection-for-namespace-perms
- metabase.test.util/with-non-admin-groups-no-root-collection-perms
- metabase.test.util/with-temp-env-var-value
- metabase.test.util/with-temp-vals-in-db
- metabase.test.util/with-temporary-raw-setting-values
- metabase.test.util/with-temporary-setting-values
- metabase.test.util/with-user-in-groups
- metabase.test/with-actions
- metabase.test/with-actions-disabled
- metabase.test/with-actions-enabled
- metabase.test/with-actions-test-data
- metabase.test/with-actions-test-data-and-actions-enabled
- metabase.test/with-actions-test-data-tables
- metabase.test/with-all-users-permission
- metabase.test/with-discarded-collections-perms-changes
- metabase.test/with-env-keys-renamed-by
- metabase.test/with-expected-messages
- metabase.test/with-fake-inbox
- metabase.test/with-group
- metabase.test/with-group-for-user
- metabase.test/with-locale
- metabase.test/with-log-level
- metabase.test/with-non-admin-groups-no-root-collection-for-namespace-perms
- metabase.test/with-non-admin-groups-no-root-collection-perms
- metabase.test/with-persistence-enabled
- metabase.test/with-single-admin-user
- metabase.test/with-temp-env-var-value
- metabase.test/with-temp-vals-in-db
- metabase.test/with-temporary-raw-setting-values
- metabase.test/with-temporary-setting-values
- metabase.test/with-user-in-groups})
-
-;;; TODO -- we should disallow `metabase.test/user-http-request` with any method other than `:get`
-
-(def ^:private allowed-parallel-forms
- "These fns are destructive, but are probably fine inside ^:parallel tests because it usually means you're doing
- something to an atom or something like that."
- '#{clojure.core/assoc!
- clojure.core/compare-and-set!
- clojure.core/conj!
- clojure.core/disj!
- clojure.core/dissoc!
- clojure.core/persistent!
- clojure.core/pop!
- clojure.core/reset!
- clojure.core/reset-vals!
- clojure.core/run!
- clojure.core/swap!
- clojure.core/swap-vals!
- clojure.core/volatile!
- clojure.core/vreset!
- clojure.core/vswap!
- clojure.core.async/<!
- clojure.core.async/<!!
- clojure.core.async/>!
- clojure.core.async/>!!
- clojure.core.async/alt!
- clojure.core.async/alt!!
- clojure.core.async/alts!
- clojure.core.async/alts!!
- clojure.core.async/close!
- clojure.core.async/ioc-alts!
- clojure.core.async/offer!
- clojure.core.async/onto-chan!
- clojure.core.async/onto-chan!!
- clojure.core.async/poll!
- clojure.core.async/put!
- clojure.core.async/take!
- clojure.core.async/to-chan!
- clojure.core.async/to-chan!!
- metabase.driver.sql-jdbc.execute/execute-prepared-statement!
- metabase.pulse/send-pulse!
- metabase.query-processor.store/store-database!
- next.jdbc/execute!})
-
-(defn- warn-about-disallowed-parallel-forms [form]
+(defn- warn-about-disallowed-parallel-forms [form config]
(letfn [(error! [form message]
(hooks/reg-finding! (assoc (meta form)
:message message
@@ -117,12 +12,12 @@
(f [form]
(when-let [qualified-symbol (hooks.common/node->qualified-symbol form)]
(cond
- (disallowed-parallel-forms qualified-symbol)
- (error! form (format "%s is not allowed inside a ^:parallel test or test fixture" qualified-symbol))
+ (contains? (:disallowed config) qualified-symbol)
+ (error! form (format "%s is not allowed inside a ^:parallel test or test fixture [:metabase/validate-deftest]" qualified-symbol))
- (and (not (allowed-parallel-forms qualified-symbol))
+ (and (not (contains? (:allowed config) qualified-symbol))
(str/ends-with? (name qualified-symbol) "!"))
- (error! form (format "destructive functions like %s are not allowed inside a ^:parallel test or test fixture. If this should be allowed, add it to the whitelist in .clj-kondo/hooks/clojure/test.clj"
+ (error! form (format "destructive functions like %s are not allowed inside a ^:parallel test or test fixture. If this should be allowed, add it to the whitelist in the Kondo config file [:metabase/validate-deftest]"
qualified-symbol)))))
(walk [form]
(f form)
@@ -133,7 +28,7 @@
(defn- deftest-check-parallel
"1. Check if test is marked ^:parallel / ^:synchronized correctly
2. Make sure disallowed forms are not used in ^:parallel tests"
- [{[_ test-name & body] :children, :as _node}]
+ [{[_ test-name & body] :children, :as _node} config]
(let [test-metadata (:meta test-name)
metadata-sexprs (map hooks/sexpr test-metadata)
combined-metadata (transduce
@@ -150,9 +45,9 @@
(hooks/reg-finding! (assoc (meta test-name)
:message "Test should not be marked both ^:parallel and ^:synchronized"
:type :metabase/validate-deftest)))
- ;; only when the custom `:metabase/deftest-not-marked-parallel` is enabled: complain if tests are not explicitly
- ;; marked `^:parallel` or `^:synchronized`. This is mostly to encourage people to mark everything `^:parallel` in
- ;; places like `metabase.lib` tests unless there is a really good reason not to.
+ ;; only when the custom `:metabase/deftest-not-marked-parallel-or-synchronized` is enabled: complain if tests are
+ ;; not explicitly marked `^:parallel` or `^:synchronized`. This is mostly to encourage people to mark everything
+ ;; `^:parallel` in places like `metabase.lib` tests unless there is a really good reason not to.
(when-not (or parallel? synchronized?)
(hooks/reg-finding!
(assoc (meta test-name)
@@ -160,46 +55,21 @@
:type :metabase/deftest-not-marked-parallel-or-synchronized)))
(when parallel?
(doseq [form body]
- (warn-about-disallowed-parallel-forms form)))))
-
-(def ^:private number-of-lines-for-a-test-to-be-considered-horrifically-long
- 200)
+ (warn-about-disallowed-parallel-forms form config)))))
(defn- deftest-check-not-horrifically-long
- [node]
+ [node {:keys [max-length], :as _config}]
(let [{:keys [row end-row]} (meta node)]
(when (and row end-row)
(let [num-lines (- end-row row)]
- (when (>= num-lines number-of-lines-for-a-test-to-be-considered-horrifically-long)
+ (when (and max-length
+ (>= num-lines max-length))
(hooks/reg-finding! (assoc (meta node)
:message (str (format "This test is horrifically long, it's %d lines! 😱 " num-lines)
"Do you really want to try to debug it if it fails? 💀 "
"Split it up into smaller tests! 🥰")
:type :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests)))))))
-;;; don't include `:h2` since that will probably lead to a mountain of tests where we are specifically testing
-;;; middleware or compilation behavior with just H2 since it's the default.
-(def driver-keywords
- #{;; core drivers
- #_:h2
- :postgres
- :mysql
- ;; module drivers
- :athena
- :bigquery-cloud-sdk
- :druid
- :druid-jdbc
- :googleanalytics
- :mongo
- :oracle
- :presto-jdbc
- :redshift
- :snowflake
- :sparksql
- :sqlite
- :sqlserver
- :vertica})
-
(defn- ignore? [node error-type]
(when-let [ignores (some-> node meta :clj-kondo/ignore hooks/sexpr)]
(when-let [ignores (cond
@@ -207,12 +77,13 @@
(keyword? ignores) #{ignores})]
(contains? ignores error-type))))
-(defn deftest-check-no-driver-keywords [node]
+(defn deftest-check-no-driver-keywords [node {:keys [drivers], :as _config}]
;; fail fast after we see the first error, where we see one hardcoded driver name there are likely several more and we
;; don't need multiple warnings for a single test.
(letfn [(f [node]
(when (and (hooks/keyword-node? node)
- (driver-keywords (hooks/sexpr node)))
+ (set? drivers)
+ (contains? drivers (hooks/sexpr node)))
(hooks/reg-finding! (assoc (meta node)
:message (format "Do not hardcode driver name %s in driver tests! [:metabase/disallow-hardcoded-driver-names-in-tests]"
(hooks/sexpr node))
@@ -235,14 +106,14 @@
(:children node))))]
(walk node)))
-(defn deftest [{:keys [node cljc lang]}]
+(defn deftest [{:keys [node cljc lang config]}]
;; run [[deftest-check-parallel]] only once... if this is a `.cljc` file only run it for the `:clj` analysis, no point
;; in running it twice.
(when (or (not cljc)
(= lang :clj))
- (deftest-check-parallel node))
- (deftest-check-not-horrifically-long node)
- (deftest-check-no-driver-keywords node)
+ (deftest-check-parallel node (get-in config [:linters :metabase/validate-deftest])))
+ (deftest-check-not-horrifically-long node (get-in config [:linters :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]))
+ (deftest-check-no-driver-keywords node (get-in config [:linters :metabase/disallow-hardcoded-driver-names-in-tests]))
{:node node})
;;; this is a hacky way to determine whether these namespaces are required in the `ns` form or not... basically `:ns`
@@ -284,6 +155,6 @@
(warn-about-missing-test-expr-requires-in-cljs node))
{:node node})
-(defn use-fixtures [{:keys [node]}]
- (warn-about-disallowed-parallel-forms node)
+(defn use-fixtures [{:keys [node config]}]
+ (warn-about-disallowed-parallel-forms node config)
{:node node})
diff --git a/.clj-kondo/test/hooks/clojure/test_test.clj b/.clj-kondo/test/hooks/clojure/test_test.clj
index b9fbf659014..19bf39b4621 100644
--- a/.clj-kondo/test/hooks/clojure/test_test.clj
+++ b/.clj-kondo/test/hooks/clojure/test_test.clj
@@ -2,6 +2,7 @@
(:require
[clj-kondo.hooks-api :as api]
[clj-kondo.impl.utils]
+ [clojure.edn :as edn]
[clojure.java.io :as io]
[clojure.test :refer :all]
[hooks.clojure.test]))
@@ -14,10 +15,14 @@
:ignores (atom nil)
:findings (atom [])
:namespaces (atom {})}]
- (hooks.clojure.test/deftest {:node (api/parse-string form)})
+ (hooks.clojure.test/deftest {:node (api/parse-string form)
+ :config {:linters
+ {:metabase/disallow-hardcoded-driver-names-in-tests
+ {:drivers
+ #{:athena}}}}})
(mapv :message @(:findings clj-kondo.impl.utils/*ctx*))))
-(deftest disallow-hardcoded-driver-names-in-tests-test
+(deftest ^:parallel disallow-hardcoded-driver-names-in-tests-test
(is (= []
(deftest-warnings
"(mt/test-drivers (mt/normal-drivers)
@@ -35,10 +40,13 @@
(when-not (= driver/*driver* :athena)
(do-something)))")))))
-(deftest check-driver-keywords-test
+(deftest ^:parallel check-driver-keywords-test
(testing "Make sure we keep hooks.clojure.test/driver-keywords up to date"
- (doseq [^java.io.File file (.listFiles (io/file "modules/drivers"))
- :when (.isDirectory file)
- :let [driver (keyword (.getName file))]]
- (is (contains? hooks.clojure.test/driver-keywords driver)
- (format "hooks.clojure.test/driver-keywords should contain %s, please add it" driver)))))
+ (let [driver-keywords (-> (slurp ".clj-kondo/config.edn")
+ edn/read-string
+ (get-in [:linters :metabase/disallow-hardcoded-driver-names-in-tests :drivers]))]
+ (doseq [^java.io.File file (.listFiles (io/file "modules/drivers"))
+ :when (.isDirectory file)
+ :let [driver (keyword (.getName file))]]
+ (is (contains? driver-keywords driver)
+ (format "hooks.clojure.test/driver-keywords should contain %s, please add it" driver))))))
diff --git a/enterprise/backend/src/metabase_enterprise/advanced_config/file.clj b/enterprise/backend/src/metabase_enterprise/advanced_config/file.clj
index cb8ccd2cac8..a6678d0e214 100644
--- a/enterprise/backend/src/metabase_enterprise/advanced_config/file.clj
+++ b/enterprise/backend/src/metabase_enterprise/advanced_config/file.clj
@@ -97,8 +97,7 @@
[clojure.walk :as walk]
[environ.core :as env]
[metabase-enterprise.advanced-config.file.databases]
- [metabase-enterprise.advanced-config.file.interface
- :as advanced-config.file.i]
+ [metabase-enterprise.advanced-config.file.interface :as advanced-config.file.i]
[metabase-enterprise.advanced-config.file.settings]
[metabase-enterprise.advanced-config.file.users]
[metabase.driver.common.parameters]
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
index 9f4add45a6a..de4c03ca2c9 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
@@ -136,6 +136,7 @@
(is (= known-files
(set (.list (io/file api.serialization/parent-dir)))))))))
+#_{:clj-kondo/ignore [:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]}
(deftest export-import-test
(testing "Serialization API e2e"
(let [known-files (set (.list (io/file api.serialization/parent-dir)))]
diff --git a/test/metabase/analyze/classifiers/name_test.clj b/test/metabase/analyze/classifiers/name_test.clj
index a8ba4adb665..bfff940186a 100644
--- a/test/metabase/analyze/classifiers/name_test.clj
+++ b/test/metabase/analyze/classifiers/name_test.clj
@@ -5,7 +5,7 @@
[metabase.models.interface :as mi]
[metabase.models.table :as table :refer [Table]]))
-(deftest ^:paralell semantic-type-for-name-and-base-type-test
+(deftest ^:parallel semantic-type-for-name-and-base-type-test
(doseq [[input expected] {["id" :type/Integer] :type/PK
;; other pattern matches based on type/regex (remember, base_type matters in matching!)
["rating" :type/Integer] :type/Score
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index d30e5845344..aac93db59fc 100644
--- a/test/metabase/api/database_test.clj
+++ b/test/metabase/api/database_test.clj
@@ -2094,7 +2094,7 @@
:unaggregated-query-row-limit (symbol "nil #_\"key is not present.\"")}
(settings)))))))))
-(deftest ^:paralell log-an-error-if-contains-undefined-setting-test
+(deftest ^:parallel log-an-error-if-contains-undefined-setting-test
(testing "should log an error message if database contains undefined settings"
(t2.with-temp/with-temp [Database {db-id :id} {:settings {:undefined-setting true}}]
(mt/with-log-messages-for-level [messages :error]
diff --git a/test/metabase/cmd/copy_test.clj b/test/metabase/cmd/copy_test.clj
index 0adde357c13..4bedc1a073b 100644
--- a/test/metabase/cmd/copy_test.clj
+++ b/test/metabase/cmd/copy_test.clj
@@ -47,7 +47,7 @@
(remove models-to-exclude))
(descendants :metabase/model)))
-(deftest ^:paralell all-models-accounted-for-test
+(deftest ^:parallel all-models-accounted-for-test
;; make sure the entire system is loaded before running this test, to make sure we account for all the models.
(doseq [ns-symb (ns.find/find-namespaces (classpath/system-classpath))
:when (and (str/starts-with? ns-symb "metabase")
diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj
index aeba178865b..f5d0946ce00 100644
--- a/test/metabase/db/custom_migrations_test.clj
+++ b/test/metabase/db/custom_migrations_test.clj
@@ -1093,189 +1093,202 @@
(encryption-test/with-secret-key "dont-tell-anyone-about-this"
(do-test true))))
-(deftest fix-click-through-test
- (let [migrate (fn [card dash]
- (:visualization_settings
- (#'custom-migrations/fix-click-through {:id 1
- :dashcard_visualization dash
- :card_visualization card})))]
- (testing "toplevel"
- (let [card {"some_setting:" {"foo" 123}
- "click_link_template" "http://example.com/{{col_name}}"
- "click" "link"}
- dash {"other_setting" {"bar" 123}}]
- (is (= {"other_setting" {"bar" 123}
+(defn- fix-click-thru [card dash]
+ (:visualization_settings
+ (#'custom-migrations/fix-click-through {:id 1
+ :dashcard_visualization dash
+ :card_visualization card})))
+
+(deftest ^:parallel fix-click-through-test
+ (testing "toplevel"
+ (let [card {"some_setting:" {"foo" 123}
+ "click_link_template" "http://example.com/{{col_name}}"
+ "click" "link"}
+ dash {"other_setting" {"bar" 123}}]
+ (is (= {"other_setting" {"bar" 123}
+ "click_behavior" {"type" "link"
+ "linkType" "url"
+ "linkTemplate" "http://example.com/{{col_name}}"}}
+ (fix-click-thru card dash))))))
+
+(deftest ^:parallel fix-click-through-test-2
+ (testing "top level disabled"
+ (let [card {"some_setting:" {"foo" 123}
+ "click_link_template" "http://example.com/{{col_name}}"
+ "click" "link"}
+ dash {"other_setting" {"bar" 123}
+ "click_link_template" "http://example.com/{{col_name}}"
+ "click" "menu"}]
+ ;; click: "menu" turned off the custom drill through so it's not migrated. Dropping click and click_link_template
+ ;; would be fine but isn't needed.
+ (is (nil? (fix-click-thru card dash))))))
+
+(deftest ^:parallel fix-click-through-test-3
+ (testing "column settings"
+ (let [card {"some_setting" {"foo" 123}
+ "column_settings"
+ {"[\"ref\",[\"field-id\",1]]"
+ {"view_as" "link"
+ "link_template" "http://example.com/{{id}}"
+ "link_text" "here is my id: {{id}}"}}}
+ dash {"other_setting" {"bar" 123}
+ "column_settings"
+ {"[\"ref\",[\"field-id\",1]]" {"fun_formatting" "foo"}
+ "[\"ref\",[\"field-id\",2]]" {"other_fun_formatting" 123}}}]
+ (is (= {"other_setting" {"bar" 123}
+ "column_settings"
+ {"[\"ref\",[\"field-id\",1]]"
+ {"fun_formatting" "foo"
+ "click_behavior" {"type" "link"
+ "linkType" "url"
+ "linkTemplate" "http://example.com/{{id}}"
+ "linkTextTemplate" "here is my id: {{id}}"}}
+ "[\"ref\",[\"field-id\",2]]"
+ {"other_fun_formatting" 123}}}
+ (fix-click-thru card dash))))))
+
+(deftest ^:parallel fix-click-through-test-4
+ (testing "manually updated new behavior"
+ (let [card {"some_setting" {"foo" 123}
+ "click_link_template" "http://example.com/{{col_name}}"
+ "click" "link"}
+ dash {"other_setting" {"bar" 123}
"click_behavior" {"type" "link"
"linkType" "url"
- "linkTemplate" "http://example.com/{{col_name}}"}}
- (migrate card dash)))))
-
- (testing "top level disabled"
- (let [card {"some_setting:" {"foo" 123}
- "click_link_template" "http://example.com/{{col_name}}"
- "click" "link"}
- dash {"other_setting" {"bar" 123}
- "click_link_template" "http://example.com/{{col_name}}"
- "click" "menu"}]
- ;;click: "menu" turned off the custom drill through so it's not migrated. Dropping click and click_link_template would be fine but isn't needed.
- (is (nil? (migrate card dash)))))
- (testing "column settings"
- (let [card {"some_setting" {"foo" 123}
- "column_settings"
- {"[\"ref\",[\"field-id\",1]]"
- {"view_as" "link"
- "link_template" "http://example.com/{{id}}"
- "link_text" "here is my id: {{id}}"}}}
- dash {"other_setting" {"bar" 123}
- "column_settings"
- {"[\"ref\",[\"field-id\",1]]" {"fun_formatting" "foo"}
- "[\"ref\",[\"field-id\",2]]" {"other_fun_formatting" 123}}}]
- (is (= {"other_setting" {"bar" 123}
+ "linkTemplate" "http://example.com/{{other_col_name}}"}}]
+ (is (nil? (fix-click-thru card dash))))))
+
+(deftest ^:parallel fix-click-through-test-5
+ (testing "Manually updated to new behavior on Column"
+ (let [card {"some_setting" {"foo" 123},
"column_settings"
{"[\"ref\",[\"field-id\",1]]"
- {"fun_formatting" "foo"
- "click_behavior" {"type" "link"
- "linkType" "url"
- "linkTemplate" "http://example.com/{{id}}"
- "linkTextTemplate" "here is my id: {{id}}"}}
+ {"view_as" "link"
+ "link_template" "http://example.com/{{id}}"
+ "other_special_formatting" "currency"}
"[\"ref\",[\"field-id\",2]]"
- {"other_fun_formatting" 123}}}
- (migrate card dash)))))
- (testing "manually updated new behavior"
- (let [card {"some_setting" {"foo" 123}
- "click_link_template" "http://example.com/{{col_name}}"
- "click" "link"}
- dash {"other_setting" {"bar" 123}
- "click_behavior" {"type" "link"
- "linkType" "url"
- "linkTemplate" "http://example.com/{{other_col_name}}"}}]
- (is (nil? (migrate card dash)))))
- (testing "Manually updated to new behavior on Column"
- (let [card {"some_setting" {"foo" 123},
- "column_settings"
- {"[\"ref\",[\"field-id\",1]]"
- {"view_as" "link"
- "link_template" "http://example.com/{{id}}"
- "other_special_formatting" "currency"}
- "[\"ref\",[\"field-id\",2]]"
- {"view_as" "link",
- "link_template" "http://example.com/{{something_else}}",
- "other_fun_formatting" 0}}}
- dash {"other_setting" {"bar" 123}
- "column_settings"
- {"[\"ref\",[\"field-id\",1]]"
- {"click_behavior"
- {"type" "link"
- "linkType" "url"
- "linkTemplate" "http://example.com/{{id}}"}}
- "[\"ref\",[\"field-id\",2]]"
- {"other_fun_formatting" 123}}}]
- (is (= {"other_setting" {"bar" 123}
+ {"view_as" "link",
+ "link_template" "http://example.com/{{something_else}}",
+ "other_fun_formatting" 0}}}
+ dash {"other_setting" {"bar" 123}
"column_settings"
{"[\"ref\",[\"field-id\",1]]"
{"click_behavior"
- {"type" "link",
- "linkType" "url",
+ {"type" "link"
+ "linkType" "url"
"linkTemplate" "http://example.com/{{id}}"}}
"[\"ref\",[\"field-id\",2]]"
- {"other_fun_formatting" 123,
+ {"other_fun_formatting" 123}}}]
+ (is (= {"other_setting" {"bar" 123}
+ "column_settings"
+ {"[\"ref\",[\"field-id\",1]]"
+ {"click_behavior"
+ {"type" "link",
+ "linkType" "url",
+ "linkTemplate" "http://example.com/{{id}}"}}
+ "[\"ref\",[\"field-id\",2]]"
+ {"other_fun_formatting" 123,
+ "click_behavior"
+ {"type" "link",
+ "linkType" "url",
+ "linkTemplate" "http://example.com/{{something_else}}"}}}}
+ (fix-click-thru card dash))))))
+
+(deftest ^:parallel fix-click-through-test-6
+ (testing "If there is migration eligible on dash but also new style on dash, new style wins"
+ (let [dash {"column_settings"
+ {"[\"ref\",[\"field-id\",4]]"
+ {"view_as" "link"
+ "link_template" "http://old" ;; this stuff could be migrated
+ "link_text" "old"
+ "column_title" "column title"
"click_behavior"
- {"type" "link",
- "linkType" "url",
- "linkTemplate" "http://example.com/{{something_else}}"}}}}
- (migrate card dash)))))
- (testing "If there is migration eligible on dash but also new style on dash, new style wins"
- (let [dash {"column_settings"
- {"[\"ref\",[\"field-id\",4]]"
- {"view_as" "link"
- "link_template" "http://old" ;; this stuff could be migrated
- "link_text" "old"
- "column_title" "column title"
- "click_behavior"
- {"type" "link",
- "linkType" "url", ;; but there is already a new style and it wins
- "linkTemplate" "http://new",
- "linkTextTemplate" "new"}}}}]
- ;; no change
- (is (nil? (migrate nil dash)))))
- (testing "flamber case"
- (let [card {"column_settings"
- {"[\"ref\",[\"field-id\",4]]"
- {"view_as" "link"
- "link_template" "http//localhost/?QCDT&{{CATEGORY}}"
- "link_text" "MyQCDT {{CATEGORY}}"
- "column_title" "QCDT Category"}
- "[\"ref\",[\"field-id\",6]]"
- {"view_as" "link"
- "column_title" "QCDT Rating"
- "link_text" "Rating {{RATING}}"
- "link_template" "http//localhost/?QCDT&{{RATING}}"
- "prefix" "prefix-"
- "suffix" "-suffix"}
- "[\"ref\",[\"field-id\",5]]"
- {"view_as" nil
- "link_text" "QCDT was disabled"
- "link_template" "http//localhost/?QCDT&{{TITLE}}"
- "column_title" "(QCDT disabled) Title"}}
- "table.pivot_column" "CATEGORY"
- "table.cell_column" "PRICE"}
- dash {"table.cell_column" "PRICE"
- "table.pivot_column" "CATEGORY"
- "column_settings"
- {"[\"ref\",[\"field-id\",5]]"
- {"view_as" nil
- "link_text" "QCDT was disabled"
- "link_template" "http//localhost/?QCDT&{{TITLE}}"
- "column_title" "(QCDT disabled) Title"}
- "[\"ref\",[\"field-id\",4]]"
- {"view_as" "link"
- "link_template" "http//localhost/?QCDT&{{CATEGORY}}"
- "link_text" "MyQCDT {{CATEGORY}}"
- "column_title" "QCDT Category"
- "click_behavior"
- {"type" "link"
- "linkType" "url"
- "linkTemplate" "http//localhost/?CB&{{CATEGORY}}"
- "linkTextTemplate" "MyCB {{CATEGORY}}"}}
- "[\"ref\",[\"field-id\",6]]"
- {"view_as" "link"
- "column_title" "QCDT Rating"
- "link_text" "Rating {{RATING}}"
- "link_template" "http//localhost/?QCDT&{{RATING}}"
- "prefix" "prefix-"
- "suffix" "-suffix"}}
- "card.title" "Table with QCDT - MANUALLY ADDED CB 37"}]
- (is (= {"card.title" "Table with QCDT - MANUALLY ADDED CB 37"
- "column_settings"
+ {"type" "link",
+ "linkType" "url", ;; but there is already a new style and it wins
+ "linkTemplate" "http://new",
+ "linkTextTemplate" "new"}}}}]
+ ;; no change
+ (is (nil? (fix-click-thru nil dash))))))
+
+(deftest ^:parallel fix-click-through-test-7
+ (testing "flamber case"
+ (let [card {"column_settings"
{"[\"ref\",[\"field-id\",4]]"
- {"column_title" "QCDT Category"
- "view_as" "link"
+ {"view_as" "link"
+ "link_template" "http//localhost/?QCDT&{{CATEGORY}}"
+ "link_text" "MyQCDT {{CATEGORY}}"
+ "column_title" "QCDT Category"}
+ "[\"ref\",[\"field-id\",6]]"
+ {"view_as" "link"
+ "column_title" "QCDT Rating"
+ "link_text" "Rating {{RATING}}"
+ "link_template" "http//localhost/?QCDT&{{RATING}}"
+ "prefix" "prefix-"
+ "suffix" "-suffix"}
+ "[\"ref\",[\"field-id\",5]]"
+ {"view_as" nil
+ "link_text" "QCDT was disabled"
+ "link_template" "http//localhost/?QCDT&{{TITLE}}"
+ "column_title" "(QCDT disabled) Title"}}
+ "table.pivot_column" "CATEGORY"
+ "table.cell_column" "PRICE"}
+ dash {"table.cell_column" "PRICE"
+ "table.pivot_column" "CATEGORY"
+ "column_settings"
+ {"[\"ref\",[\"field-id\",5]]"
+ {"view_as" nil
+ "link_text" "QCDT was disabled"
+ "link_template" "http//localhost/?QCDT&{{TITLE}}"
+ "column_title" "(QCDT disabled) Title"}
+ "[\"ref\",[\"field-id\",4]]"
+ {"view_as" "link"
"link_template" "http//localhost/?QCDT&{{CATEGORY}}"
"link_text" "MyQCDT {{CATEGORY}}"
+ "column_title" "QCDT Category"
"click_behavior"
{"type" "link"
"linkType" "url"
"linkTemplate" "http//localhost/?CB&{{CATEGORY}}"
"linkTextTemplate" "MyCB {{CATEGORY}}"}}
- "[\"ref\",[\"field-id\",5]]"
- {"link_text" "QCDT was disabled"
- "column_title" "(QCDT disabled) Title"
- "link_template" "http//localhost/?QCDT&{{TITLE}}"}
"[\"ref\",[\"field-id\",6]]"
- {"prefix" "prefix-"
- "suffix" "-suffix"
+ {"view_as" "link"
"column_title" "QCDT Rating"
- "view_as" "link"
"link_text" "Rating {{RATING}}"
"link_template" "http//localhost/?QCDT&{{RATING}}"
- "click_behavior"
- {"type" "link"
- "linkType" "url"
- "linkTemplate" "http//localhost/?QCDT&{{RATING}}"
- "linkTextTemplate" "Rating {{RATING}}"}}}
- "table.cell_column" "PRICE"
- "table.pivot_column" "CATEGORY"}
- (migrate card dash)))))))
+ "prefix" "prefix-"
+ "suffix" "-suffix"}}
+ "card.title" "Table with QCDT - MANUALLY ADDED CB 37"}]
+ (is (= {"card.title" "Table with QCDT - MANUALLY ADDED CB 37"
+ "column_settings"
+ {"[\"ref\",[\"field-id\",4]]"
+ {"column_title" "QCDT Category"
+ "view_as" "link"
+ "link_template" "http//localhost/?QCDT&{{CATEGORY}}"
+ "link_text" "MyQCDT {{CATEGORY}}"
+ "click_behavior"
+ {"type" "link"
+ "linkType" "url"
+ "linkTemplate" "http//localhost/?CB&{{CATEGORY}}"
+ "linkTextTemplate" "MyCB {{CATEGORY}}"}}
+ "[\"ref\",[\"field-id\",5]]"
+ {"link_text" "QCDT was disabled"
+ "column_title" "(QCDT disabled) Title"
+ "link_template" "http//localhost/?QCDT&{{TITLE}}"}
+ "[\"ref\",[\"field-id\",6]]"
+ {"prefix" "prefix-"
+ "suffix" "-suffix"
+ "column_title" "QCDT Rating"
+ "view_as" "link"
+ "link_text" "Rating {{RATING}}"
+ "link_template" "http//localhost/?QCDT&{{RATING}}"
+ "click_behavior"
+ {"type" "link"
+ "linkType" "url"
+ "linkTemplate" "http//localhost/?QCDT&{{RATING}}"
+ "linkTextTemplate" "Rating {{RATING}}"}}}
+ "table.cell_column" "PRICE"
+ "table.pivot_column" "CATEGORY"}
+ (fix-click-thru card dash))))))
(deftest fix-click-through-general-test
(testing "general case"
diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj
index 4c38acb013e..53385453d57 100644
--- a/test/metabase/db/schema_migrations_test.clj
+++ b/test/metabase/db/schema_migrations_test.clj
@@ -1741,6 +1741,7 @@
(t2/select-one-fn :perm_value (t2/table-name :model/DataPermissions)
:db_id db-id :table_id table-id-2 :group_id group-id :perm_type "perms/create-queries"))))))))
+#_{:clj-kondo/ignore [:metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]}
(deftest split-data-permissions-legacy-no-self-service-migration-test
(testing "view-data is set to `legacy-no-self-service` for groups that meet specific conditions"
(impl/test-migrations ["v50.2024-02-26T22:15:54" "v50.2024-02-26T22:15:55"] [migrate!]
diff --git a/test/metabase/driver/sql/parameters/substitute_test.clj b/test/metabase/driver/sql/parameters/substitute_test.clj
index 10902ee71e6..b4746c412c0 100644
--- a/test/metabase/driver/sql/parameters/substitute_test.clj
+++ b/test/metabase/driver/sql/parameters/substitute_test.clj
@@ -111,180 +111,182 @@
(is (= ["select * from orders" nil]
(substitute query {"created_at" (assoc (date-field-filter-value) :value params/no-value)}))))))))
+(def ^:private substitute-field-filter-test-2-test-cases
+ (partition-all
+ 2
+ [:string/contains {:field :name
+ :value ["foo"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"]
+ ["%foo%"]]}
+ :string/contains {:field :name
+ :value ["FOO"]
+ :options {:case-sensitive false}
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (LOWER(\"PUBLIC\".\"VENUES\".\"NAME\") LIKE ?)"]
+ ["%foo%"]]}
+ :string/does-not-contain {:field :name
+ :value ["foo"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " NOT (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
+ " )"]
+ ["%foo%"]]}
+ :string/does-not-contain {:field :name
+ :value ["FOO"]
+ :options {:case-sensitive false}
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " NOT (LOWER(\"PUBLIC\".\"VENUES\".\"NAME\") LIKE ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
+ " )"]
+ ["%foo%"]]}
+ :string/starts-with {:field :name
+ :value ["foo"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"]
+ ["foo%"]]}
+ :string/= {:field :name
+ :value ["foo"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"]
+ ["foo"]]}
+ :string/= {:field :name
+ :value ["foo" "bar" "baz"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
+ " )"]
+ ["foo" "bar" "baz"]]}
+ :string/!= {:field :name
+ :value ["foo" "bar"]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " ("
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" <> ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
+ " )"
+ " AND ("
+ " (\"PUBLIC\".\"VENUES\".\"NAME\" <> ?)"
+ " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
+ " )"
+ " )"]
+ ["foo" "bar"]]}
+ :number/= {:field :price
+ :value [1]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" = 1)"]
+ []]}
+ :number/= {:field :price
+ :value [1 2 3]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" = 1)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" = 2)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" = 3)"
+ " )"]
+ []]}
+ :number/!= {:field :price
+ :value [1]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 1)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
+ " )"]
+ []]}
+ :number/!= {:field :price
+ :value [1 2 3]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " ("
+ " ("
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 1)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
+ " )"
+ " AND ("
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 2)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
+ " )"
+ " AND ("
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 3)"
+ " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
+ " )"
+ " )"]
+ []]}
+ :number/>= {:field :price
+ :value [1]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " (\"PUBLIC\".\"VENUES\".\"PRICE\" >= 1)"]
+ []]}
+ :number/between {:field :price
+ :value [1 3]
+ :expected [["select"
+ " *"
+ "from"
+ " venues"
+ "where"
+ " \"PUBLIC\".\"VENUES\".\"PRICE\" BETWEEN 1 AND 3"]
+ []]}]))
+
(deftest ^:parallel substitute-field-filter-test-2
(testing "new operators"
(testing "string operators"
(let [query ["select * from venues where " (param "param")]]
- (doseq [[operator {:keys [field value expected options]}]
- (partition-all
- 2
- [:string/contains {:field :name
- :value ["foo"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"]
- ["%foo%"]]}
- :string/contains {:field :name
- :value ["FOO"]
- :options {:case-sensitive false}
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (LOWER(\"PUBLIC\".\"VENUES\".\"NAME\") LIKE ?)"]
- ["%foo%"]]}
- :string/does-not-contain {:field :name
- :value ["foo"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " NOT (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
- " )"]
- ["%foo%"]]}
- :string/does-not-contain {:field :name
- :value ["FOO"]
- :options {:case-sensitive false}
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " NOT (LOWER(\"PUBLIC\".\"VENUES\".\"NAME\") LIKE ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
- " )"]
- ["%foo%"]]}
- :string/starts-with {:field :name
- :value ["foo"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (\"PUBLIC\".\"VENUES\".\"NAME\" LIKE ?)"]
- ["foo%"]]}
- :string/= {:field :name
- :value ["foo"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"]
- ["foo"]]}
- :string/= {:field :name
- :value ["foo" "bar" "baz"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" = ?)"
- " )"]
- ["foo" "bar" "baz"]]}
- :string/!= {:field :name
- :value ["foo" "bar"]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " ("
- " (\"PUBLIC\".\"VENUES\".\"NAME\" <> ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
- " )"
- " AND ("
- " (\"PUBLIC\".\"VENUES\".\"NAME\" <> ?)"
- " OR (\"PUBLIC\".\"VENUES\".\"NAME\" IS NULL)"
- " )"
- " )"]
- ["foo" "bar"]]}
- :number/= {:field :price
- :value [1]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" = 1)"]
- []]}
- :number/= {:field :price
- :value [1 2 3]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" = 1)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" = 2)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" = 3)"
- " )"]
- []]}
- :number/!= {:field :price
- :value [1]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 1)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
- " )"]
- []]}
- :number/!= {:field :price
- :value [1 2 3]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " ("
- " ("
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 1)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
- " )"
- " AND ("
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 2)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
- " )"
- " AND ("
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" <> 3)"
- " OR (\"PUBLIC\".\"VENUES\".\"PRICE\" IS NULL)"
- " )"
- " )"]
- []]}
- :number/>= {:field :price
- :value [1]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " (\"PUBLIC\".\"VENUES\".\"PRICE\" >= 1)"]
- []]}
- :number/between {:field :price
- :value [1 3]
- :expected [["select"
- " *"
- "from"
- " venues"
- "where"
- " \"PUBLIC\".\"VENUES\".\"PRICE\" BETWEEN 1 AND 3"]
- []]}])]
+ (doseq [[operator {:keys [field value expected options]}] substitute-field-filter-test-2-test-cases]
(testing operator
(is (= expected
(-> (substitute query {"param" (params/map->FieldFilter
diff --git a/test/metabase/lib/drill_thru/underlying_records_test.cljc b/test/metabase/lib/drill_thru/underlying_records_test.cljc
index dda3b32f817..168db1ec904 100644
--- a/test/metabase/lib/drill_thru/underlying_records_test.cljc
+++ b/test/metabase/lib/drill_thru/underlying_records_test.cljc
@@ -13,7 +13,7 @@
[metabase.lib.test-util.metadata-providers.mock :as providers.mock]
[metabase.util :as u]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal])
- :clj ([java-time.api :as jt]
+ :clj ([java-time.api :as t]
[metabase.util.malli.fn :as mu.fn]))))
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
@@ -98,9 +98,9 @@
(-> (js/Date.UTC year (dec month))
(js/Date.)
(.toISOString)))
- :clj (let [last-month (-> (jt/zoned-date-time (jt/year) (jt/month))
- (jt/minus (jt/months 1)))]
- (jt/format :iso-offset-date-time last-month))))
+ :clj (let [last-month (-> (t/zoned-date-time (t/year) (t/month))
+ (t/minus (t/months 1)))]
+ (t/format :iso-offset-date-time last-month))))
(defn- underlying-state [query agg-index agg-value breakout-values exp-filters-fn]
(let [columns (lib/returned-columns query)
--
GitLab