Skip to content
Snippets Groups Projects
Unverified Commit bc4acbd2 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Enable Kondo for tests (part 1) (#24736)

* Fix some small things

* Add Kondo to deps.edn to be able to debug custom hooks from REPL

* Fix macroexpansion hook for with-temp* without values

* Test config (WIP)

* More misc fixes

* Disable :inline-def for tests

* More misc fixes

* Fix $ids and mbql-query kondo hooks.

* Fix with-temporary-setting-values with namespaced symbols

* More misc fixes

* Fix the rest of the easy ones

* Fix hook for mt/dataset

* Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro

* Fix places calling mbql-query with a keyword table name

* Fix the last few errors in test/

* Fix errors in enterprise/test and shared/test

* Fix driver test errors

* Enable linters on CI

* Enable unresolved-namespace linter for tests

* Appease the namespace linter again

* Test fixes
parent e474b0aa
No related branches found
No related tags found
No related merge requests found
Showing
with 358 additions and 202 deletions
......@@ -35,6 +35,7 @@
:namespace-name-mismatch {:level :off}
:use {:level :warning}
:redundant-fn-wrapper {:level :warning}
:invalid-arity {:skip-args [metabase.mbql.util.match/match]}
;; TODO (cam): here are some more linters we should experiment with enabling -- some might be useful.
;; :docstring-no-summary {:level :warning}
;; :docstring-leading-trailing-whitespace {:level :warning}
......@@ -402,19 +403,18 @@
metabase.models.setting/defsetting hooks.metabase.models.setting/defsetting
metabase.models.setting.multi-setting/define-multi-setting hooks.metabase.models.setting/defsetting
metabase.test/$ids hooks.metabase.test.data/$ids
metabase.test/data-query hooks.metabase.test.data/$ids
metabase.test/dataset hooks.metabase.test.data/dataset
metabase.test/mbql-query hooks.metabase.test.data/$ids
metabase.test/query hooks.metabase.test.data/$ids
metabase.test/run-mbql-query hooks.metabase.test.data/$ids
metabase.test/mbql-query hooks.metabase.test.data/mbql-query
metabase.test/query hooks.metabase.test.data/mbql-query
metabase.test/run-mbql-query hooks.metabase.test.data/mbql-query
metabase.test/with-temp hooks.toucan.util.test/with-temp
metabase.test/with-temp* hooks.toucan.util.test/with-temp*
metabase-enterprise.serialization.test-util/with-temp-dpc hooks.toucan.util.test/with-temp*
metabase.test/with-temporary-setting-values hooks.metabase.test.util/with-temporary-setting-values
metabase.test.data/$ids hooks.metabase.test.data/$ids
metabase.test.data/dataset hooks.metabase.test.data/dataset
metabase.test.data/mbql-query hooks.metabase.test.data/$ids
metabase.test.data/run-mbql-query hooks.metabase.test.data/$ids
metabase.test.data/mbql-query hooks.metabase.test.data/mbql-query
metabase.test.data/run-mbql-query hooks.metabase.test.data/mbql-query
metabase.test.util/with-temporary-setting-values hooks.metabase.test.util/with-temporary-setting-values}
:macroexpand
......@@ -428,4 +428,35 @@
clojurewerkz.quartzite.schedule.simple/schedule macros.quartz/simple-schedule}}
:config-in-comment
{:linters {:unresolved-symbol {:level :off}}}}
{:linters {:unresolved-symbol {:level :off}}}
;;
;; TEST CONFIG
;;
:ns-groups
;; the following patterns are considered to be test namespaces:
;;
;; - Any namespace ending in `-test` or `-test.whatever`
;; - Any namespace ending in `test-util`
;; - Any namespace that starts with `metabase.test`
;;
;; this list isn't exhaustive because it misses some stuff like the test runner and HTTP client but it's easier to go
;; fix those namespaces than it is to make this regex super hairy.
[{:pattern "(?:.*-test(?:\\..*)?$)|(?:.*test-util$)|(?:^metabase\\.test.*)"
:name test-namespaces}]
:config-in-ns
{test-namespaces
{:linters
{:inline-def {:level :off}
:missing-docstring {:level :off}
:private-call {:level :off}
;; custom linters
:hooks.metabase.test.data/mbql-query-first-arg {:level :error}
;; FIXME
:unused-binding {:level :off}
:unresolved-symbol {:level :off}
:deprecated-var {:level :off}
}}}}
(ns hooks.metabase.test.data
(:require
[clj-kondo.hooks-api :as hooks]
[clojure.string :as string]
[clojure.string :as str]
[clojure.walk :as walk]))
(defn dataset [{:keys [node]}]
(let [[dataset & body] (rest (:children node))]
{:node (with-meta
(hooks/list-node
(list*
(hooks/token-node 'clojure.test/testing)
(hooks/string-node (str dataset))
body))
(meta dataset))}))
(defn $ids [{:keys [node]}]
(let [args (rest (:children node))
[table-name & body] args
unused-node (hooks/token-node '_)
vars (atom #{})
_ (walk/postwalk (fn [node]
(when (hooks/token-node? node)
(let [str-node (str (hooks/sexpr node))]
(when (or (string/starts-with? str-node "$")
(string/starts-with? str-node "!")
(string/starts-with? str-node "&")
(string/starts-with? str-node "*")
(string/starts-with? str-node "%"))
(swap! vars conj node))))
node)
body)
nil-bindings (vec (interpose (hooks/token-node nil) @vars))
unused-bindings (vec (interpose unused-node @vars))
final-bindings (concat
(when-not (= "nil" (str table-name))
[table-name (hooks/token-node nil)])
[unused-node table-name]
(if (seq nil-bindings)
(conj nil-bindings (hooks/token-node nil))
[])
(if (seq unused-bindings)
(conj (next unused-bindings)
(first unused-bindings)
unused-node)
[]))]
{:node (with-meta
(hooks/list-node
(with-meta
(defn- global-dataset-symbols
"Dataset definitions defined in [[metabase.test.data.dataset-definitions]]. This is only populated if clj-kondo has
cached analysis for that namespace -- so it may or may not be populated. If it is populated we can do a bit of extra
linting with that information."
[]
(not-empty (set (keys (:clj (hooks/ns-analysis 'metabase.test.data.dataset-definitions))))))
(defn- dataset-type
"`dataset` can be one of:
- a qualified symbol
- an unqualified symbol referring to a a var in [[metabase.test.data.dataset-definitions]]
- an unqualified symbol (referring to a let-bound value or a var in the current namespace
- some sort of non-symbol form like a function call
We can only determine if an unqualified symbol refers to something in the dataset definitions namespace if there are
cached results available from [[global-dataset-symbols]]."
[dataset]
(let [sexpr (hooks/sexpr dataset)
global-defs (global-dataset-symbols)]
(cond
(not (symbol? sexpr))
:non-symbol
(namespace sexpr)
:qualified
(empty? global-defs)
:unqualified/unknown
(contains? global-defs sexpr)
:unqualified/from-dataset-defs-namespace
;; either something defined in the current namespace or let-bound in the current scope.
:else
:unqualified/local-def)))
(defn dataset
[{{[_ dataset & body] :children} :node}]
(let [body (case (dataset-type dataset)
;; non-symbol, qualified symbols, and unqualified symbols from the current namespace/let-bound can all
;; get converted from something like
;;
;; (dataset whatever
;; ...)
;;
;; to
;;
;; (let [_ whatever]
;; ...)
(:non-symbol :qualified :unqualified/local-def)
(list* (hooks/token-node 'let)
(hooks/vector-node [(hooks/token-node '_) dataset])
body)
;; for ones that came from the dataset defs namespace or ones whose origin is unknown, just ignore them
;; and generate a `do` form:
;;
;; (do ...)
(:unqualified/from-dataset-defs-namespace :unqualified/unknown)
(list* (hooks/token-node 'do)
body))]
{:node (with-meta (hooks/list-node (with-meta body
(meta dataset)))
(meta dataset))}))
(defn- special-token-node?
"Whether this node is one of the special symbols like `$field`."
[node]
(when (hooks/token-node? node)
(let [symb (hooks/sexpr node)]
(and (symbol? symb)
(some (partial str/starts-with? symb)
#{"$" "!" "&" "*" "%"})
;; ignore args like `%` or `%1` inside function literals. $id forms have to be more than one character long,
;; and the first character won't be a number (hopefully -- maybe some DBs allow this -- but we don't use it)
(> (count (str symb)) 1)
(not (re-find #"^%\d+" (str symb)))))))
(defn- replace-$id-special-tokens
"Impl for [[$ids]] and [[mbql-query]]. Walk `form` and look for special tokens like `$field` and replace them with
strings so we don't get unresolved symbol errors. Preserves metadata."
[form]
;; [[walk/postwalk]] seems to preserve its meta so we don't need to do anything special
(walk/postwalk
(fn [node]
(if (special-token-node? node)
(-> (hooks/string-node (str (hooks/sexpr node)))
(with-meta (meta node)))
node))
form))
(defn $ids
[{{[_ & args] :children} :node}]
;; `$ids` accepts either
;;
;; ($ids form)
;;
;; or
;;
;; ($ids table & body)
;;
;; table is only relevant for expanding the special tokens so we can ignore it.
(let [body (if (= (count args) 1)
(first args)
(hooks/list-node
(list*
(hooks/token-node 'let)
(hooks/vector-node (vec final-bindings))
body)
(meta body)))
(meta body))}))
(hooks/token-node `do)
(rest args))))]
{:node (replace-$id-special-tokens body)}))
(defn mbql-query
[{{[_ & args] :children} :node}]
;; `mbql-query` accepts either
;;
;; (mbql-query table)
;;
;; or
;;
;; (mbql-query table query)
;;
;; and table may be `nil`.
;;
;; table is only relevant for expanding the special tokens so we can ignore it either way.
(let [[table query] (if (= (count args) 1)
[(first args)
(hooks/map-node [])]
args)]
(when-not ((some-fn symbol? nil?) (hooks/sexpr table))
(hooks/reg-finding! (assoc (meta table)
:message "First arg to mbql-query should be either a table name symbol or nil."
:type ::mbql-query-first-arg)))
(let [result (replace-$id-special-tokens query)
;; HACK I'm not sure WHY it works but I ran into https://github.com/clj-kondo/clj-kondo/issues/1773 when
;; trying to get this working -- for some magical reason wrapping the whole thing in a `do` form seems to fix
;; it. Once that bug is resolved we can go ahead and remove this line
result (with-meta (hooks/list-node (with-meta (list
(hooks/token-node 'do)
result)
(meta query)))
(meta query))]
{:node result})))
(ns hooks.metabase.test.util
(:require [clj-kondo.hooks-api :as hooks]))
(defn- namespaced-symbol-node? [node]
(when (hooks/token-node? node)
(let [symb (hooks/sexpr node)]
(and (symbol? symb)
(namespace symb)))))
(defn with-temporary-setting-values
"Rewrite a form like
(with-temporary-setting-values [x 1, y 2]
(with-temporary-setting-values [x 1, some.ns/y 2]
...)
as one like
(let [_ 1, _ 2]
(let [_ 1, _ some.ns/y, _ 2]
...)
for analysis purposes."
[{:keys [node]}]
(let [[bindings & body] (rest (:children node))
bindings (if (hooks/vector-node? bindings)
(hooks/vector-node (into []
(mapcat (fn [[_token-name v]]
[(hooks/token-node '_) v]))
(partition 2 (:children bindings))))
bindings)]
for analysis purposes. We only need to 'capture' namespaced Setting bindings with a `_` so Kondo considers their
namespace to be 'used' and to catch undefined var usage."
[{{[_ bindings & body] :children} :node}]
(let [bindings (if (hooks/vector-node? bindings)
(hooks/vector-node (into []
(mapcat (fn [[setting-name v]]
(concat
[(hooks/token-node '_) v]
;; if the setting name is namespace-qualified add a `_`
;; entry for it too.
(when (namespaced-symbol-node? setting-name)
[(hooks/token-node '_) setting-name]))))
(partition 2 (:children bindings))))
bindings)]
{:node (-> (list*
(hooks/token-node 'let)
bindings
......
......@@ -2,20 +2,23 @@
(:require [clj-kondo.hooks-api :as api]))
(defn- with-temp-inner [body bindings]
(let [pairs (partition 2 bindings)
db-refs (map first pairs)
let-stream (for [[_ binding+opts] pairs
part (:children binding+opts)]
part)]
(api/vector-node [(api/vector-node db-refs)
(api/list-node (list* (api/token-node `let)
(api/vector-node let-stream)
body))])))
(let [binding-infos (for [[model {[binding value] :children}] (partition 2 bindings)]
{:model model
:binding binding
:value (or value
(api/token-node 'nil))})]
(-> (api/vector-node
[(api/vector-node (map :model binding-infos))
(-> (api/list-node (list* (api/token-node `let)
(api/vector-node (mapcat (juxt :binding :value) binding-infos))
body))
(with-meta (meta body)))])
(with-meta (meta body)))))
(defn with-temp [{:keys [node]}]
(let [[_ db-ref binding+opts & body] (:children node)]
{:node (with-temp-inner body [db-ref binding+opts])}))
(defn with-temp* [{:keys [node]}]
(let [[_ bindings & body] (:children node)]
(let [[_ bindings & body] (:children node)]
{:node (with-temp-inner body (:children bindings))}))
......@@ -55,21 +55,22 @@
(eval . (put-clojure-indent 'u/prog1 1))
(eval . (put-clojure-indent 'u/select-keys-when 1))
(eval . (put-clojure-indent 'u/strict-extend 1))
(eval . (put-clojure-indent 'with-meta '(defn)))
;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
;; instead. Please don't add new entries unless they don't work with `put-clojure-indent'
(eval . (define-clojure-indent
(l/matcha '(1 (:defn)))
(l/matche '(1 (:defn)))
(p.types/def-abstract-type '(1 (:defn)))
(p.types/defprotocol+ '(1 (:defn)))
(p.types/defrecord+ '(2 nil nil (:defn)))
(p.types/deftype+ '(2 nil nil (:defn)))
(p/def-map-type '(2 nil nil (:defn)))
(p/defprotocol+ '(1 (:defn)))
(p/defrecord+ '(2 nil nil (:defn)))
(p/deftype+ '(2 nil nil (:defn)))
(tools.macro/macrolet '(1 ((:defn)) :form))))
(l/matcha '(1 (:defn)))
(l/matche '(1 (:defn)))
(p.types/def-abstract-type '(1 (:defn)))
(p.types/defprotocol+ '(1 (:defn)))
(p.types/defrecord+ '(2 nil nil (:defn)))
(p.types/deftype+ '(2 nil nil (:defn)))
(p/def-map-type '(2 nil nil (:defn)))
(p/defprotocol+ '(1 (:defn)))
(p/defrecord+ '(2 nil nil (:defn)))
(p/deftype+ '(2 nil nil (:defn)))
(tools.macro/macrolet '(1 ((:defn)) :form))))
(cider-clojure-cli-aliases . "dev:drivers:drivers-dev:ee:ee-dev:user")
(clojure-indent-style . always-align)
(cljr-favor-prefix-notation . nil)
......
......@@ -30,22 +30,39 @@ jobs:
--parallel
--lint
/work/src
/work/test
/work/enterprise/backend/src
/work/enterprise/backend/test
/work/shared/src
/work/modules/drivers/bigquery-cloud-sdk/src
/work/modules/drivers/druid/src
/work/modules/drivers/googleanalytics/src
/work/modules/drivers/mongo/src
/work/modules/drivers/oracle/src
/work/shared/test
/work/modules/drivers/sparksql/src
/work/modules/drivers/sparksql/test
/work/modules/drivers/vertica/src
/work/modules/drivers/vertica/test
/work/modules/drivers/presto-common/src
/work/modules/drivers/presto-jdbc/src
/work/modules/drivers/presto-common/test
/work/modules/drivers/presto/src
/work/modules/drivers/presto/test
/work/modules/drivers/mongo/src
/work/modules/drivers/mongo/test
/work/modules/drivers/oracle/src
/work/modules/drivers/oracle/test
/work/modules/drivers/sqlite/src
/work/modules/drivers/sqlite/test
/work/modules/drivers/bigquery-cloud-sdk/src
/work/modules/drivers/bigquery-cloud-sdk/test
/work/modules/drivers/redshift/src
/work/modules/drivers/redshift/test
/work/modules/drivers/snowflake/src
/work/modules/drivers/sparksql/src
/work/modules/drivers/sqlite/src
/work/modules/drivers/snowflake/test
/work/modules/drivers/googleanalytics/src
/work/modules/drivers/googleanalytics/test
/work/modules/drivers/druid/src
/work/modules/drivers/druid/test
/work/modules/drivers/sqlserver/src
/work/modules/drivers/vertica/src
/work/modules/drivers/sqlserver/test
/work/modules/drivers/presto-jdbc/src
/work/modules/drivers/presto-jdbc/test
be-linter-eastwood:
runs-on: ubuntu-20.04
......
......@@ -171,6 +171,7 @@
{:extra-deps
{clj-http-fake/clj-http-fake {:mvn/version "1.0.3"
:exclusions [slingshot/slingshot]}
clj-kondo/clj-kondo {:mvn/version "2022.08.03"} ; this is not for RUNNING kondo, but so we can hack on custom hooks code from the REPL.
cloverage/cloverage {:mvn/version "1.2.2"}
eftest/eftest {:mvn/version "0.5.9"}
jonase/eastwood {:mvn/version "1.2.2"}
......
(ns metabase-enterprise.advanced-permissions.api.application-test
(:require [clojure.test :refer :all]
[metabase.models :refer [PermissionsGroup]]
[metabase.models.permissions-group :as group]
[metabase.models.permissions-group :as perms-group]
[metabase.public-settings.premium-features-test :as premium-features-test]
[metabase.test :as mt]))
......@@ -22,11 +22,11 @@
(let [graph (mt/user-http-request :crowberto :get 200 "ee/advanced-permissions/application/graph")
groups (:groups graph)]
(is (int? (:revision graph)))
(is (partial= {(:id (group/admin))
(is (partial= {(:id (perms-group/admin))
{:monitoring "yes"
:setting "yes"
:subscription "yes"}
(:id (group/all-users))
(:id (perms-group/all-users))
{:monitoring "no"
:setting "no"
:subscription "yes"}}
......@@ -54,7 +54,7 @@
(assoc new-graph :revision (inc (:revision new-graph)))))))
(testing "successfully update application permissions"
(is (partial= {(:id (group/admin))
(is (partial= {(:id (perms-group/admin))
{:monitoring "yes"
:setting "yes"
:subscription "yes"}
......
(ns metabase-enterprise.advanced-permissions.api.subscription-test
"Permisisons tests for API that needs to be enforced by Application Permissions to create and edit alerts/subscriptions."
(:require [clojure.test :refer :all]
[metabase.api.alert :as api-alert]
[metabase.api.alert :as api.alert]
[metabase.api.alert-test :as alert-test]
[metabase.models :refer [Card Collection Pulse PulseCard PulseChannel PulseChannelRecipient]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
[metabase.models.permissions-group :as perms-group]
[metabase.models.pulse :as pulse]
[metabase.public-settings.premium-features-test :as premium-features-test]
[metabase.pulse-test :as pulse-test]
......@@ -18,10 +18,10 @@
Use it when we need to isolate a user's permissions during tests."
[& body]
`(try
(perms/revoke-application-permissions! (group/all-users) :subscription)
(perms/revoke-application-permissions! (perms-group/all-users) :subscription)
~@body
(finally
(perms/grant-application-permissions! (group/all-users) :subscription))))
(perms/grant-application-permissions! (perms-group/all-users) :subscription))))
(deftest pulse-permissions-test
(testing "/api/pulse/*"
......@@ -86,7 +86,7 @@
{:card card-id
:channel :email}]
(let [the-pulse (pulse/retrieve-pulse (:id the-pulse))
channel (api-alert/email-channel the-pulse)
channel (api.alert/email-channel the-pulse)
new-channel (assoc channel :recipients (conj (:recipients channel) (mt/fetch-user :lucky)))
new-pulse (assoc the-pulse :channels [new-channel])]
(testing (format "- add pulse's recipients with %s user" (mt/user-descriptor req-user))
......@@ -103,7 +103,7 @@
(db/select-one-id
PulseChannel :channel_type "email" :pulse_id (:id the-pulse))}]
(let [the-pulse (pulse/retrieve-pulse (:id the-pulse))
channel (api-alert/email-channel the-pulse)
channel (api.alert/email-channel the-pulse)
new-channel (update channel :recipients rest)
new-pulse (assoc the-pulse :channels [new-channel])]
(testing (format "- remove pulse's recipients with %s user" (mt/user-descriptor req-user))
......
......@@ -3,7 +3,7 @@
[metabase-enterprise.advanced-permissions.models.permissions.application-permissions :as g-perms]
[metabase.models :refer [ApplicationPermissionsRevision PermissionsGroup]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
[metabase.models.permissions-group :as perms-group]
[metabase.test :as mt]
[toucan.db :as db]))
......@@ -16,11 +16,11 @@
(testing "group should be in graph if one of application permission is enabled"
(let [graph (g-perms/graph)]
(is (= 0 (:revision graph)))
(is (partial= {(:id (group/admin))
(is (partial= {(:id (perms-group/admin))
{:monitoring :yes
:setting :yes
:subscription :yes}
(:id (group/all-users))
(:id (perms-group/all-users))
{:monitoring :no
:setting :no
:subscription :yes}}
......@@ -67,7 +67,7 @@
(testing "Failed when try to update permission for admin group"
(with-new-group-and-current-graph group-id current-graph
(let [new-graph (assoc-in current-graph [:groups (:id (group/admin)) :subscription] :no)]
(let [new-graph (assoc-in current-graph [:groups (:id (perms-group/admin)) :subscription] :no)]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You cannot create or revoke permissions for the 'Admin' group."
......
......@@ -3,8 +3,9 @@
[clojure.test :refer :all]
[metabase-enterprise.advanced-permissions.models.permissions :as ee.perms]
[metabase-enterprise.advanced-permissions.query-processor.middleware.permissions :as ee.qp.perms]
[metabase.api.dataset :as dataset] [metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
[metabase.api.dataset :as api.dataset]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.public-settings.premium-features-test :as premium-features-test]
[metabase.query-processor.context.default :as context.default]
[metabase.query-processor.streaming-test :as streaming-test]
......@@ -14,7 +15,7 @@
(defn- do-with-download-perms
[db-or-id graph f]
(let [all-users-group-id (u/the-id (group/all-users))
(let [all-users-group-id (u/the-id (perms-group/all-users))
db-id (u/the-id db-or-id)
current-download-perms-graph (get-in (perms/data-perms-graph)
[:groups all-users-group-id db-id :download])]
......@@ -47,13 +48,13 @@
(-> {:database (mt/id)
:type :query
:query {:source-table (mt/id table-name)}
:info {:context (dataset/export-format->context :csv)}})))
:info {:context (api.dataset/export-format->context :csv)}})))
(defn- native-download-query []
{:database (mt/id)
:type :native
:native {:query "select * from venues"}
:info {:context (dataset/export-format->context :csv)}})
:info {:context (api.dataset/export-format->context :csv)}})
(defn- download-limit
[query]
......
......@@ -5,11 +5,12 @@
This file makes it impossible to forget to add entity_id to new entities. It tests that every entity is either
explicitly excluded, or has the :entity_id property."
(:require
[clojure.test :refer :all]
metabase.db.data-migrations
metabase.models
metabase.models.revision-test
[toucan.models :refer [IModel properties]]))
[clojure.test :refer :all]
metabase.db.data-migrations
metabase.models
metabase.models.revision-test
[metabase.models.serialization.hash :as serdes.hash]
[toucan.models :refer [IModel]]))
(comment metabase.models/keep-me
metabase.db.data-migrations/keep-me
......@@ -78,4 +79,4 @@
(doseq [model (->> (extenders IModel)
(remove entities-not-exported))]
(testing (format "Model %s should implement IdentityHashable" (.getSimpleName model))
(is (extends? metabase.models.serialization.hash/IdentityHashable model)))))
(is (extends? serdes.hash/IdentityHashable model)))))
......@@ -60,8 +60,8 @@
"Update query for Card associated with an existing GTAP"
(fn [query]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query :venues)
:result_metadata (qp/query->expected-cols (mt/mbql-query :venues))}]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query venues)
:result_metadata (qp/query->expected-cols (mt/mbql-query venues))}]
GroupTableAccessPolicy [gtap {:table_id (mt/id :venues)
:group_id (u/the-id (perms-group/all-users))
:card_id (:id card)}]]
......@@ -84,7 +84,7 @@
(testing "Don't allow saving a Sandboxing query that changes the type of a column vs. the type in the Table it replaces (#13715)"
(doseq [[msg f] {"Create a new GTAP"
(fn [metadata]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query :venues)
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query venues)
:result_metadata metadata}]
GroupTableAccessPolicy [gtap {:table_id (mt/id :venues)
:group_id (u/the-id (perms-group/all-users))
......@@ -93,7 +93,7 @@
"Update an existing GTAP"
(fn [metadata]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query :venues)
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query venues)
:result_metadata metadata}]
GroupTableAccessPolicy [gtap {:table_id (mt/id :venues)
:group_id (u/the-id (perms-group/all-users))}]]
......@@ -102,8 +102,8 @@
"Update query for Card associated with an existing GTAP"
(fn [metadata]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query :venues)
:result_metadata (qp/query->expected-cols (mt/mbql-query :venues))}]
(mt/with-temp* [Card [card {:dataset_query (mt/mbql-query venues)
:result_metadata (qp/query->expected-cols (mt/mbql-query venues))}]
GroupTableAccessPolicy [gtap {:table_id (mt/id :venues)
:group_id (u/the-id (perms-group/all-users))
:card_id (:id card)}]]
......
......@@ -381,7 +381,7 @@
(doseq [[model entity] (:entities fingerprint)]
(testing (format "%s \"%s\"" (type model) (:name entity))
(is (or (-> entity :name nil?)
(if-let [loaded (db/select-one model :name (:name entity))]
(when-let [loaded (db/select-one model :name (:name entity))]
(assert-loaded-entity loaded fingerprint))
(and (-> entity :archived) ; archived card hasn't been dump-loaded
(= (:name entity) "My Arch Card"))
......
......@@ -79,9 +79,7 @@
(binding [mdb.connection/*application-db* (mdb.connection/application-db :h2 data-source)]
(testing (format "\nApp DB = %s" (pr-str connection-string))
(thunk))))]
(do-with-app-db
(fn []
(mdb/setup-db!)))
(do-with-app-db mdb/setup-db!)
(f do-with-app-db)))))
(defn do-with-source-and-dest-dbs [f]
......
......@@ -5,7 +5,7 @@
[metabase-enterprise.sso.integrations.sso-settings :as sso-settings]
[metabase.config :as config]
[metabase.http-client :as client]
[metabase.integrations.ldap :refer [ldap-enabled]]
[metabase.integrations.ldap :as ldap]
[metabase.models.permissions-group :refer [PermissionsGroup]]
[metabase.models.permissions-group-membership :refer [PermissionsGroupMembership]]
[metabase.models.user :refer [User]]
......@@ -27,7 +27,7 @@
(use-fixtures :once (fixtures/initialize :test-users))
(defn- disable-other-sso-types [thunk]
(mt/with-temporary-setting-values [ldap-enabled false
(mt/with-temporary-setting-values [ldap/ldap-enabled false
jwt-enabled false]
(thunk)))
......
......@@ -113,47 +113,46 @@
;;
(deftest test-external-table
(mt/test-driver :redshift
(testing "expects spectrum schema to exist"
(is (= [{:description nil
:table_id (mt/id :extsales)
:semantic_type nil
:name "buyerid"
:settings nil
:source :fields
:field_ref [:field (mt/id :extsales :buyerid) nil]
:nfc_path nil
:parent_id nil
:id (mt/id :extsales :buyerid)
:visibility_type :normal
:display_name "Buyerid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}
{:description nil
:table_id (mt/id :extsales)
:semantic_type nil
:name "salesid"
:settings nil
:source :fields
:field_ref [:field (mt/id :extsales :salesid) nil]
:nfc_path nil
:parent_id nil
:id (mt/id :extsales :salesid)
:visibility_type :normal
:display_name "Salesid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}]
;; in different Redshift instances, the fingerprint on these columns is different.
(map #(dissoc % :fingerprint)
(get-in (qp/process-query (mt/mbql-query
:extsales
{:limit 1
:fields [$buyerid $salesid]
:order-by [[:asc $buyerid]
[:asc $salesid]]
:filter [:= [:field (mt/id :extsales :buyerid) nil] 11498]}))
[:data :cols])))))))
(testing "expects spectrum schema to exist"
(is (= [{:description nil
:table_id (mt/id :extsales)
:semantic_type nil
:name "buyerid"
:settings nil
:source :fields
:field_ref [:field (mt/id :extsales :buyerid) nil]
:nfc_path nil
:parent_id nil
:id (mt/id :extsales :buyerid)
:visibility_type :normal
:display_name "Buyerid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}
{:description nil
:table_id (mt/id :extsales)
:semantic_type nil
:name "salesid"
:settings nil
:source :fields
:field_ref [:field (mt/id :extsales :salesid) nil]
:nfc_path nil
:parent_id nil
:id (mt/id :extsales :salesid)
:visibility_type :normal
:display_name "Salesid"
:base_type :type/Integer
:effective_type :type/Integer
:coercion_strategy nil}]
;; in different Redshift instances, the fingerprint on these columns is different.
(map #(dissoc % :fingerprint)
(get-in (qp/process-query (mt/mbql-query extsales
{:limit 1
:fields [$buyerid $salesid]
:order-by [[:asc $buyerid]
[:asc $salesid]]
:filter [:= [:field (mt/id :extsales :buyerid) nil] 11498]}))
[:data :cols])))))))
(deftest parameters-test
(mt/test-driver :redshift
......
......@@ -174,39 +174,39 @@
;; some upper layer will handle it.
(mt/with-db db
(testing "select datetime stored as unix epoch"
(is (= [["2021-08-25T04:18:24Z" ; TIMESTAMP
"2021-08-25T00:00:00Z" ; DATE
"2021-08-25T04:18:24Z"]] ; DATETIME
(is (= [["2021-08-25T04:18:24Z" ; TIMESTAMP
"2021-08-25T00:00:00Z" ; DATE
"2021-08-25T04:18:24Z"]] ; DATETIME
(qp.test/rows
(mt/run-mbql-query :datetime_table
{:fields [$col_timestamp $col_date $col_datetime]
:filter [:= $test_case "epoch"]})))))
(mt/run-mbql-query datetime_table
{:fields [$col_timestamp $col_date $col_datetime]
:filter [:= $test_case "epoch"]})))))
(testing "select datetime stored as string with milliseconds"
(is (= [["2021-08-25 04:18:24.111" ; TIMESTAMP (raw string)
"2021-08-25T04:18:24.111Z"]] ; DATETIME
(is (= [["2021-08-25 04:18:24.111" ; TIMESTAMP (raw string)
"2021-08-25T04:18:24.111Z"]] ; DATETIME
(qp.test/rows
(mt/run-mbql-query :datetime_table
{:fields [$col_timestamp $col_datetime]
:filter [:= $test_case "iso8601-ms"]})))))
(mt/run-mbql-query datetime_table
{:fields [$col_timestamp $col_datetime]
:filter [:= $test_case "iso8601-ms"]})))))
(testing "select datetime stored as string without milliseconds"
(is (= [["2021-08-25 04:18:24" ; TIMESTAMP (raw string)
"2021-08-25T04:18:24Z"]] ; DATETIME
(is (= [["2021-08-25 04:18:24" ; TIMESTAMP (raw string)
"2021-08-25T04:18:24Z"]] ; DATETIME
(qp.test/rows
(mt/run-mbql-query :datetime_table
{:fields [$col_timestamp $col_datetime]
:filter [:= $test_case "iso8601-no-ms"]})))))
(mt/run-mbql-query datetime_table
{:fields [$col_timestamp $col_datetime]
:filter [:= $test_case "iso8601-no-ms"]})))))
(testing "select date stored as string without time"
(is (= [["2021-08-25T00:00:00Z"]] ; DATE
(is (= [["2021-08-25T00:00:00Z"]] ; DATE
(qp.test/rows
(mt/run-mbql-query :datetime_table
{:fields [$col_date]
:filter [:= $test_case "iso8601-no-time"]})))))
(mt/run-mbql-query datetime_table
{:fields [$col_date]
:filter [:= $test_case "iso8601-no-time"]})))))
(testing "select NULL"
(is (= [[nil nil nil]]
(qp.test/rows
(mt/run-mbql-query :datetime_table
{:fields [$col_timestamp $col_date $col_datetime]
:filter [:= $test_case "null"]}))))))))))
(mt/run-mbql-query datetime_table
{:fields [$col_timestamp $col_date $col_datetime]
:filter [:= $test_case "null"]}))))))))))
(deftest duplicate-identifiers-test
(testing "Make sure duplicate identifiers (even with different cases) get unique aliases"
......
(ns metabase.mbql.js-test
(:require [metabase.mbql.js :as mbql.js]
[cljs.test :as t]))
(:require [cljs.test :as t]
[metabase.mbql.js :as mbql.js]))
(t/deftest normalize-test
(t/testing "normalize should preserve keyword namespaces"
......
......@@ -130,14 +130,14 @@
(mbql.match/match {:filter [:and
[:= [:field 1 nil] 4000]
[:= [:field 2 nil] 5000]]}
(&match :guard #(integer? %))))))
(&match :guard integer?)))))
(t/testing "can we use a predicate and bind the match at the same time?"
(t/is (= [2 4001 3 5001]
(mbql.match/match {:filter [:and
[:= [:field 1 nil] 4000]
[:= [:field 2 nil] 5000]]}
(i :guard #(integer? %))
(i :guard integer?)
(inc i))))))
(t/deftest ^:parallel match-map-test
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment