diff --git a/backend/mbql/test/metabase/mbql/util_test.clj b/backend/mbql/test/metabase/mbql/util_test.clj
index 0e44c432e02de86e21e9b7ec3df0e259ecb15ba7..928ad83b33e456ff74e0c1b7fe3065ae68ed0622 100644
--- a/backend/mbql/test/metabase/mbql/util_test.clj
+++ b/backend/mbql/test/metabase/mbql/util_test.clj
@@ -1,6 +1,5 @@
(ns metabase.mbql.util-test
(:require [clojure.test :refer :all]
- [expectations :refer [expect]]
[java-time :as t]
[metabase.mbql.util :as mbql.u]
metabase.types))
@@ -27,48 +26,52 @@
;;; | match |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; can we use `match` to find the instances of a clause?
-(expect
- [[:field-id 10]
- [:field-id 20]]
- (mbql.u/match {:query {:filter [:=
- [:field-id 10]
- [:field-id 20]]}}
- [:field-id & _]))
-
-;; is `match` nice enought to automatically wrap raw keywords in appropriate patterns for us?
-(expect
- [[:field-id 1]
- [:field-id 2]
- [:field-id 3]]
- (mbql.u/match {:fields [[:field-id 1] [:fk-> [:field-id 2] [:field-id 3]]]}
- :field-id))
-
-;; if we pass a set of keywords, will that generate an appropriate pattern to match multiple clauses as well?
-(expect
- [[:field-id 1]
- [:field-id 2]
- [:field-id 3]
- [:datetime-field [:field-id 4]]]
- (mbql.u/match {:fields [[:field-id 1]
- [:fk-> [:field-id 2] [:field-id 3]]
- [:datetime-field [:field-id 4]]]}
- #{:field-id :datetime-field}))
-
-;; `match` shouldn't include subclauses of matches
-(expect
- [[:field-id 1]
- [:fk-> [:field-id 2] [:field-id 3]]]
- (mbql.u/match [[:field-id 1] [:fk-> [:field-id 2] [:field-id 3]]]
- [(:or :field-id :fk->) & _]))
-
-(expect
- [[:field-id 10]
- [:field-id 20]]
- (mbql.u/match {:query {:filter [:=
- [:field-id 10]
- [:field-id 20]]}}
- [(:or :field-id :+ :-) & _]))
+(deftest basic-match-test
+ (testing "can we use `match` to find the instances of a clause?"
+ (is (= [[:field-id 10]
+ [:field-id 20]]
+ (mbql.u/match {:query {:filter [:=
+ [:field-id 10]
+ [:field-id 20]]}}
+ [:field-id & _])))))
+
+(deftest match-keywords-test
+ (testing "is `match` nice enought to automatically wrap raw keywords in appropriate patterns for us?"
+ (is (= [[:field-id 1]
+ [:field-id 2]
+ [:field-id 3]]
+ (mbql.u/match {:fields [[:field-id 1] [:fk-> [:field-id 2] [:field-id 3]]]}
+ :field-id)))
+
+ (is (= [[:field-id 1]
+ [:field-id 2]]
+ (mbql.u/match {:fields [[:field-id 1] [:datetime-field [:field-id 2] :day]]}
+ :field-id)))))
+
+(deftest match-set-of-keywords-tes
+ (testing "if we pass a set of keywords, will that generate an appropriate pattern to match multiple clauses as well?"
+ (is (= [[:field-id 1]
+ [:field-id 2]
+ [:field-id 3]
+ [:datetime-field [:field-id 4]]]
+ (mbql.u/match {:fields [[:field-id 1]
+ [:fk-> [:field-id 2] [:field-id 3]]
+ [:datetime-field [:field-id 4]]]}
+ #{:field-id :datetime-field})))))
+
+(deftest match-dont-include-subclauses-test
+ (testing "`match` shouldn't include subclauses of matches"
+ (is (= [[:field-id 1]
+ [:fk-> [:field-id 2] [:field-id 3]]]
+ (mbql.u/match [[:field-id 1] [:fk-> [:field-id 2] [:field-id 3]]]
+ [(:or :field-id :fk->) & _])))
+
+ (is (= [[:field-id 10]
+ [:field-id 20]]
+ (mbql.u/match {:query {:filter [:=
+ [:field-id 10]
+ [:field-id 20]]}}
+ [(:or :field-id :+ :-) & _])))))
;; can we use some of the cool features of pattern matching?
(def ^:private a-query
@@ -79,288 +82,277 @@
[:field-id 30]
[:field-id 40]]]})
-;; can we use the optional `result` parameter to find return something other than the whole clause?
-(expect
- [41]
- ;; return just the dest IDs of Fields in a fk-> clause
- (mbql.u/match a-query
- [:fk-> _ [:field-id dest-id]] (inc dest-id)))
-
-(expect
- [10 20]
- (mbql.u/match (:breakout a-query) [:field-id id] id))
-
-;; match should return `nil` if there are no matches so you don't need to call `seq`
-(expect
- nil
- (mbql.u/match {} [:datetime-field _ unit] unit))
-
-;; if pattern is just a raw keyword `match` should be kind enough to turn it into a pattern for you
-(expect
- [[:field-id 1]
- [:field-id 2]]
- (mbql.u/match {:fields [[:field-id 1] [:datetime-field [:field-id 2] :day]]}
- :field-id))
-
-;; can we `:guard` a pattern?
-(expect
- [[:field-id 2]]
- (let [a-field-id 2]
- (mbql.u/match {:fields [[:field-id 1] [:field-id 2]]}
- [:field-id (id :guard (partial = a-field-id))])))
-
-;; ok, if for some reason we can't use `:guard` in the pattern will `match` filter out nil results?
-(expect
- [2]
- (mbql.u/match {:fields [[:field-id 1] [:field-id 2]]}
- [:field-id id]
- (when (= id 2)
- id)))
-
-;; Ok, if we want to use predicates but still return the whole match, can we use the anaphoric `&match` symbol to
-;; return the whole thing?
+(deftest match-result-paramater-test
+ (testing "can we use the optional `result` parameter to find return something other than the whole clause?"
+ (is (= [41]
+ ;; return just the dest IDs of Fields in a fk-> clause
+ (mbql.u/match a-query
+ [:fk-> _ [:field-id dest-id]] (inc dest-id))))
+
+ (is (= [10 20]
+ (mbql.u/match (:breakout a-query) [:field-id id] id)))))
+
+(deftest match-return-nil-for-empty-sequences-test
+ (testing "match should return `nil` if there are no matches so you don't need to call `seq`"
+ (is (= nil
+ (mbql.u/match {} [:datetime-field _ unit] unit)))))
+
+(deftest match-guard-test
+ (testing "can we `:guard` a pattern?"
+ (is (= [[:field-id 2]]
+ (let [a-field-id 2]
+ (mbql.u/match {:fields [[:field-id 1] [:field-id 2]]}
+ [:field-id (id :guard (partial = a-field-id))])))))
+
+ (testing "ok, if for some reason we can't use `:guard` in the pattern will `match` filter out nil results?"
+ (is (= [2]
+ (mbql.u/match {:fields [[:field-id 1] [:field-id 2]]}
+ [:field-id id]
+ (when (= id 2)
+ id))))))
+
(def ^:private another-query
{:fields [[:field-id 1]
[:datetime-field [:field-id 2] :day]
[:datetime-field [:fk-> [:field-id 3] [:field-id 4]] :month]]})
-(expect
- [[:field-id 1]
- [:field-id 2]
- [:field-id 3]
- [:field-id 4]]
- (let [some-pred? (constantly true)]
- (mbql.u/match another-query
- :field-id
- (when some-pred?
- &match))))
-
-;; can we use the anaphoric `&parents` symbol to examine the parents of the collection? let's see if we can match
-;; `:field-id` clauses that are inside `:datetime-field` clauses, regardless of whether something else wraps them
-(expect
- [[:field-id 2]
- [:field-id 3]
- [:field-id 4]]
- (mbql.u/match another-query
- :field-id
- (when (contains? (set &parents) :datetime-field)
- &match)))
-
-;; can we match using a CLASS?
-(expect
- [#inst "2018-10-08T00:00:00.000-00:00"]
- (mbql.u/match [[:field-id 1]
- [:field-id 2]
- #inst "2018-10-08"
- 4000]
- java.util.Date))
-
-;; can we match using a PREDICATE?
-(expect
- [4000 5000]
- ;; find the integer args to `:=` clauses that are not inside `:field-id` clauses
- (mbql.u/match {:filter [:and
- [:= [:field-id 1] 4000]
- [:= [:field-id 2] 5000]]}
- integer?
- (when (= := (last &parents))
- &match)))
-
-;; how can we use predicates not named by a symbol?
-(expect
- [1 4000 2 5000]
- (mbql.u/match {:filter [:and
- [:= [:field-id 1] 4000]
- [:= [:field-id 2] 5000]]}
- (&match :guard #(integer? %))))
-
-;; can we use a predicate and bind the match at the same time?
-(expect
- [2 4001 3 5001]
- (mbql.u/match {:filter [:and
- [:= [:field-id 1] 4000]
- [:= [:field-id 2] 5000]]}
- (i :guard #(integer? %))
- (inc i)))
-
-;; can we match against a map?
-(expect
- ["card__1847"]
- (let [x {:source-table "card__1847"}]
- (mbql.u/match x
- (m :guard (every-pred map? (comp string? :source-table)))
- (:source-table m))))
-
-;; how about a sequence of maps?
-(expect
- ["card__1847"]
- (let [x [{:source-table "card__1847"}]]
- (mbql.u/match x
- (m :guard (every-pred map? (comp string? :source-table)))
- (:source-table m))))
-
-;; can we use `recur` inside a pattern?
-(expect
- [[0 :month]]
- (mbql.u/match {:filter [:time-interval [:field-id 1] :current :month]}
- [:time-interval field :current unit] (recur [:time-interval field 0 unit])
- [:time-interval _ n unit] [n unit]))
-
-;; can we short-circut a match to prevent recursive matching?
-(expect
- [10]
- (mbql.u/match [[:field-id 10]
- [:datetime-field [:field-id 20] :day]]
- [:field-id id] id
- [_ [:field-id & _] & _] nil))
-
-;; can we use a list with a :guard clause?
-(expect
- [10 20]
- (mbql.u/match {:query {:filter [:=
- [:field-id 10]
- [:field-id 20]]}}
- (id :guard int?) id))
+(deftest match-&match-test
+ (testing (str "Ok, if we want to use predicates but still return the whole match, can we use the anaphoric `&match` "
+ "symbol to return the whole thing?")
+ (is (= [[:field-id 1]
+ [:field-id 2]
+ [:field-id 3]
+ [:field-id 4]]
+ (let [some-pred? (constantly true)]
+ (mbql.u/match another-query
+ :field-id
+ (when some-pred?
+ &match)))))))
+
+(deftest match-&parents-test
+ (testing (str "can we use the anaphoric `&parents` symbol to examine the parents of the collection? let's see if we "
+ "can match `:field-id` clauses that are inside `:datetime-field` clauses, regardless of whether "
+ "something else wraps them")
+ (is (= [[:field-id 2]
+ [:field-id 3]
+ [:field-id 4]]
+ (mbql.u/match another-query
+ :field-id
+ (when (contains? (set &parents) :datetime-field)
+ &match))))))
+
+(deftest match-by-class-test
+ (testing "can we match using a CLASS?"
+ (is (= [#inst "2018-10-08T00:00:00.000-00:00"]
+ (mbql.u/match [[:field-id 1]
+ [:field-id 2]
+ #inst "2018-10-08"
+ 4000]
+ java.util.Date)))))
+
+(deftest match-by-predicate-test
+ (testing "can we match using a PREDICATE?"
+ (is (= [4000 5000]
+ ;; find the integer args to `:=` clauses that are not inside `:field-id` clauses
+ (mbql.u/match {:filter [:and
+ [:= [:field-id 1] 4000]
+ [:= [:field-id 2] 5000]]}
+ integer?
+ (when (= := (last &parents))
+ &match)))))
+
+ (testing "how can we use predicates not named by a symbol?"
+ (is (= [1 4000 2 5000]
+ (mbql.u/match {:filter [:and
+ [:= [:field-id 1] 4000]
+ [:= [:field-id 2] 5000]]}
+ (&match :guard #(integer? %))))))
+
+ (testing "can we use a predicate and bind the match at the same time?"
+ (is (= [2 4001 3 5001]
+ (mbql.u/match {:filter [:and
+ [:= [:field-id 1] 4000]
+ [:= [:field-id 2] 5000]]}
+ (i :guard #(integer? %))
+ (inc i))))))
+
+(deftest match-map-test
+ (testing "can we match against a map?"
+ (is (= ["card__1847"]
+ (let [x {:source-table "card__1847"}]
+ (mbql.u/match x
+ (m :guard (every-pred map? (comp string? :source-table)))
+ (:source-table m)))))))
+
+(deftest match-sequence-of-maps-test
+ (testing "how about a sequence of maps?"
+ (is (= ["card__1847"]
+ (let [x [{:source-table "card__1847"}]]
+ (mbql.u/match x
+ (m :guard (every-pred map? (comp string? :source-table)))
+ (:source-table m)))))))
+
+(deftest match-recur-inside-pattern-test
+ (testing "can we use `recur` inside a pattern?"
+ (is (= [[0 :month]]
+ (mbql.u/match {:filter [:time-interval [:field-id 1] :current :month]}
+ [:time-interval field :current unit] (recur [:time-interval field 0 unit])
+ [:time-interval _ n unit] [n unit])))))
+
+(deftest match-short-circut-test
+ (testing "can we short-circut a match to prevent recursive matching?"
+ (is (= [10]
+ (mbql.u/match [[:field-id 10]
+ [:datetime-field [:field-id 20] :day]]
+ [:field-id id] id
+ [_ [:field-id & _] & _] nil)))))
+
+(deftest match-list-with-guard-clause-test
+ (testing "can we use a list with a :guard clause?"
+ (is (= [10 20]
+ (mbql.u/match {:query {:filter [:=
+ [:field-id 10]
+ [:field-id 20]]}}
+ (id :guard int?) id)))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | replace |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; can we use `replace` to replace a specific clause?
-(expect
- {:breakout [[:datetime-field [:field-id 10] :day]
- [:datetime-field [:field-id 20] :day]
- [:field-literal "Wow"]]
- :fields [[:fk->
- [:datetime-field [:field-id 30] :day]
- [:datetime-field [:field-id 40] :day]]]}
- (mbql.u/replace a-query [:field-id id]
- [:datetime-field [:field-id id] :day]))
-
-;; can we wrap the pattern in a map to restrict what gets replaced?
-(expect
- {:breakout [[:datetime-field [:field-id 10] :day]
- [:datetime-field [:field-id 20] :day]
- [:field-literal "Wow"]]
- :fields [[:fk-> [:field-id 30] [:field-id 40]]]}
- (mbql.u/replace-in a-query [:breakout] [:field-id id]
- [:datetime-field [:field-id id] :day]))
-
-;; can we use multiple patterns at the same time?!
-(expect
- {:breakout [[:field-id 10] [:field-id 20] {:name "Wow"}], :fields [30]}
- (mbql.u/replace a-query
- [:fk-> [:field-id field-id] _] field-id
- [:field-literal field-name] {:name field-name}))
-
-;; can we use `replace` to replace the ID of the dest Field in fk-> clauses?
-(expect
- {:breakout [[:field-id 10]
- [:field-id 20]
- [:field-literal "Wow"]]
- :fields [[:fk-> [:field-id 30] [:field-id 100]]]}
- (mbql.u/replace a-query [:fk-> source [:field-id 40]]
- [:fk-> source [:field-id 100]]))
-
-;; can we use `replace` to fix `fk->` clauses where both args are unwrapped IDs?
-(expect
- {:query {:fields [[:fk-> [:field-id 1] [:field-id 2]]
- [:fk-> [:field-id 3] [:field-id 4]]]}}
- (mbql.u/replace-in
- {:query {:fields [[:fk-> 1 2]
- [:fk-> [:field-id 3] [:field-id 4]]]}}
- [:query :fields]
- [:fk-> (source :guard integer?) (dest :guard integer?)]
- [:fk-> [:field-id source] [:field-id dest]]))
-
-;; does `replace` accept a raw keyword as the pattern the way `match` does?
-(expect
- {:fields ["WOW"
- [:datetime-field "WOW" :day]
- [:datetime-field [:fk-> "WOW" "WOW"] :month]]}
- (mbql.u/replace another-query :field-id "WOW"))
-
-;; does `replace` accept a set of keywords the way `match` does?
-(expect
- {:fields ["WOW" "WOW" "WOW"]}
- (mbql.u/replace another-query #{:datetime-field :field-id} "WOW"))
-
-;; can we use the anaphor `&match` to look at the entire match?
-(expect
- {:fields [[:field-id 1]
- [:magical-field
- [:datetime-field [:field-id 2] :day]]
- [:magical-field
- [:datetime-field [:fk-> [:field-id 3] [:field-id 4]] :month]]]}
- (mbql.u/replace another-query :datetime-field [:magical-field &match]))
-
-;; can we use the anaphor `&parents` to look at the parents of the match?
-(expect
- {:fields
- [[:field-id 1]
- [:datetime-field "WOW" :day]
- [:datetime-field [:fk-> "WOW" "WOW"] :month]]}
- ;; replace field ID clauses that are inside a datetime-field clause
- (mbql.u/replace another-query :field-id
- (if (contains? (set &parents) :datetime-field)
- "WOW"
- &match)))
-
-;; can we replace using a CLASS?
-(expect
- [[:field-id 1]
- [:field-id 2]
- [:timestamp #inst "2018-10-08T00:00:00.000-00:00"]
- 4000]
- (mbql.u/replace [[:field-id 1]
- [:field-id 2]
- #inst "2018-10-08"
- 4000]
- java.util.Date
- [:timestamp &match]))
-
-;; can we replace using a PREDICATE?
-(expect
- {:filter [:and [:= [:field-id nil] 4000.0] [:= [:field-id nil] 5000.0]]}
- ;; find the integer args to `:=` clauses that are not inside `:field-id` clauses and make them FLOATS
- (mbql.u/replace {:filter [:and
- [:= [:field-id 1] 4000]
- [:= [:field-id 2] 5000]]}
- integer?
- (when (= := (last &parents))
- (float &match))))
-
-;; can we do fancy stuff like remove all the filters that use datetime fields from a query?
-;;
-;; (NOTE: this example doesn't take into account the fact that [:binning-strategy ...] can wrap a `:datetime-field`,
-;; so it's only appropriate for drivers that don't support binning (e.g. GA). Also the driver QP will need to be
-;; written to handle the nils in a filter clause appropriately.)
-(expect
- [:and nil [:= [:field-id 100] 20]]
- (mbql.u/replace [:and
- [:=
- [:datetime-field [:field-literal "ga:date"] :day]
- [:absolute-datetime #inst "2016-11-08T00:00:00.000-00:00" :day]]
- [:= [:field-id 100] 20]]
- [_ [:datetime-field & _] & _] nil))
-
-;; can we use short-circuting patterns to do something tricky like only replace `:field-id` clauses that aren't
-;; wrapped by other clauses?
-(expect
- [[:datetime-field [:field-id 10] :day]
- [:datetime-field [:field-id 20] :month]
- [:field-id 30]]
- (let [id-is-datetime-field? #{10}]
- (mbql.u/replace [[:field-id 10]
- [:datetime-field [:field-id 20] :month]
- [:field-id 30]]
- ;; don't replace anything that's already wrapping a `field-id`
- [_ [:field-id & _] & _]
- &match
-
- [:field-id (_ :guard id-is-datetime-field?)]
- [:datetime-field &match :day])))
+(deftest basic-replace-test
+ (testing "can we use `replace` to replace a specific clause?"
+ (is (= {:breakout [[:datetime-field [:field-id 10] :day]
+ [:datetime-field [:field-id 20] :day]
+ [:field-literal "Wow"]]
+ :fields [[:fk->
+ [:datetime-field [:field-id 30] :day]
+ [:datetime-field [:field-id 40] :day]]]}
+ (mbql.u/replace a-query [:field-id id]
+ [:datetime-field [:field-id id] :day])))))
+
+(deftest basic-replace-in-test
+ (testing "can we wrap the pattern in a map to restrict what gets replaced?"
+ (is (= {:breakout [[:datetime-field [:field-id 10] :day]
+ [:datetime-field [:field-id 20] :day]
+ [:field-literal "Wow"]]
+ :fields [[:fk-> [:field-id 30] [:field-id 40]]]}
+ (mbql.u/replace-in a-query [:breakout] [:field-id id]
+ [:datetime-field [:field-id id] :day])))))
+
+(deftest replace-multiple-patterns-test
+ (testing "can we use multiple patterns at the same time?!"
+ (is (= {:breakout [[:field-id 10] [:field-id 20] {:name "Wow"}], :fields [30]}
+ (mbql.u/replace a-query
+ [:fk-> [:field-id field-id] _] field-id
+ [:field-literal field-name] {:name field-name})))))
+
+(deftest replace-field-ids-test
+ (testing "can we use `replace` to replace the ID of the dest Field in fk-> clauses?"
+ (is (= {:breakout [[:field-id 10]
+ [:field-id 20]
+ [:field-literal "Wow"]]
+ :fields [[:fk-> [:field-id 30] [:field-id 100]]]}
+ (mbql.u/replace a-query [:fk-> source [:field-id 40]]
+ [:fk-> source [:field-id 100]])))))
+
+(deftest replace-fix-bad-mbql-test
+ (testing "can we use `replace` to fix `fk->` clauses where both args are unwrapped IDs?"
+ (is (= {:query {:fields [[:fk-> [:field-id 1] [:field-id 2]]
+ [:fk-> [:field-id 3] [:field-id 4]]]}}
+ (mbql.u/replace-in
+ {:query {:fields [[:fk-> 1 2]
+ [:fk-> [:field-id 3] [:field-id 4]]]}}
+ [:query :fields]
+ [:fk-> (source :guard integer?) (dest :guard integer?)]
+ [:fk-> [:field-id source] [:field-id dest]])))))
+
+(deftest replace-raw-keyword-patterns-test
+ (testing "does `replace` accept a raw keyword as the pattern the way `match` does?"
+ (is (= {:fields ["WOW"
+ [:datetime-field "WOW" :day]
+ [:datetime-field [:fk-> "WOW" "WOW"] :month]]}
+ (mbql.u/replace another-query :field-id "WOW")))))
+
+(deftest replace-set-of-keywords-test
+ (testing "does `replace` accept a set of keywords the way `match` does?"
+ (is (= {:fields ["WOW" "WOW" "WOW"]}
+ (mbql.u/replace another-query #{:datetime-field :field-id} "WOW")))))
+
+(deftest replace-&match-test
+ (testing "can we use the anaphor `&match` to look at the entire match?"
+ (is (= {:fields [[:field-id 1]
+ [:magical-field
+ [:datetime-field [:field-id 2] :day]]
+ [:magical-field
+ [:datetime-field [:fk-> [:field-id 3] [:field-id 4]] :month]]]}
+ (mbql.u/replace another-query :datetime-field [:magical-field &match])))))
+
+(deftest replace-&parents-test
+ (testing "can we use the anaphor `&parents` to look at the parents of the match?"
+ (is (= {:fields
+ [[:field-id 1]
+ [:datetime-field "WOW" :day]
+ [:datetime-field [:fk-> "WOW" "WOW"] :month]]}
+ ;; replace field ID clauses that are inside a datetime-field clause
+ (mbql.u/replace another-query :field-id
+ (if (contains? (set &parents) :datetime-field)
+ "WOW"
+ &match))))))
+
+(deftest replace-by-class-test
+ (testing "can we replace using a CLASS?"
+ (is (= [[:field-id 1]
+ [:field-id 2]
+ [:timestamp #inst "2018-10-08T00:00:00.000-00:00"]
+ 4000]
+ (mbql.u/replace [[:field-id 1]
+ [:field-id 2]
+ #inst "2018-10-08"
+ 4000]
+ java.util.Date
+ [:timestamp &match])))))
+
+(deftest replace-by-predicate-test
+ (testing "can we replace using a PREDICATE?"
+ (is (= {:filter [:and [:= [:field-id nil] 4000.0] [:= [:field-id nil] 5000.0]]}
+ ;; find the integer args to `:=` clauses that are not inside `:field-id` clauses and make them FLOATS
+ (mbql.u/replace {:filter [:and
+ [:= [:field-id 1] 4000]
+ [:= [:field-id 2] 5000]]}
+ integer?
+ (when (= := (last &parents))
+ (float &match)))))))
+
+(deftest complex-replace-test
+ (testing "can we do fancy stuff like remove all the filters that use datetime fields from a query?"
+ ;; (NOTE: this example doesn't take into account the fact that [:binning-strategy ...] can wrap a `:datetime-field`,
+ ;; so it's only appropriate for drivers that don't support binning (e.g. GA). Also the driver QP will need to be
+ ;; written to handle the nils in a filter clause appropriately.)
+ (is (= [:and nil [:= [:field-id 100] 20]]
+ (mbql.u/replace [:and
+ [:=
+ [:datetime-field [:field-literal "ga:date"] :day]
+ [:absolute-datetime #inst "2016-11-08T00:00:00.000-00:00" :day]]
+ [:= [:field-id 100] 20]]
+ [_ [:datetime-field & _] & _] nil)))))
+
+(deftest replace-short-circut-test
+ (testing (str "can we use short-circuting patterns to do something tricky like only replace `:field-id` clauses that "
+ "aren't wrapped by other clauses?")
+ (is (= [[:datetime-field [:field-id 10] :day]
+ [:datetime-field [:field-id 20] :month]
+ [:field-id 30]]
+ (let [id-is-datetime-field? #{10}]
+ (mbql.u/replace [[:field-id 10]
+ [:datetime-field [:field-id 20] :month]
+ [:field-id 30]]
+ ;; don't replace anything that's already wrapping a `field-id`
+ [_ [:field-id & _] & _]
+ &match
+
+ [:field-id (_ :guard id-is-datetime-field?)]
+ [:datetime-field &match :day]))))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -472,41 +464,38 @@
(mbql.u/simplify-compound-filter [:and nil [:= [:field-id 1] nil]])))))
-;; can we add an order-by clause to a query?
-(expect
- {:source-table 1, :order-by [[:asc [:field-id 10]]]}
- (mbql.u/add-order-by-clause {:source-table 1} [:asc [:field-id 10]]))
-
-(expect
- {:source-table 1
- :order-by [[:asc [:field-id 10]]
- [:asc [:field-id 20]]]}
- (mbql.u/add-order-by-clause {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- [:asc [:field-id 20]]))
-
-;; duplicate clauses should get ignored
-(expect
- {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- (mbql.u/add-order-by-clause {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- [:asc [:field-id 10]]))
-
-;; as should clauses that reference the same Field
-(expect
- {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- (mbql.u/add-order-by-clause {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- [:desc [:field-id 10]]))
-
-(expect
- {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- (mbql.u/add-order-by-clause {:source-table 1
- :order-by [[:asc [:field-id 10]]]}
- [:asc [:datetime-field [:field-id 10] :day]]))
+(deftest add-order-by-clause-test
+ (testing "can we add an order-by clause to a query?"
+ (is (= {:source-table 1, :order-by [[:asc [:field-id 10]]]}
+ (mbql.u/add-order-by-clause {:source-table 1} [:asc [:field-id 10]])))
+
+ (is (= {:source-table 1
+ :order-by [[:asc [:field-id 10]]
+ [:asc [:field-id 20]]]}
+ (mbql.u/add-order-by-clause {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ [:asc [:field-id 20]]))))
+
+ (testing "duplicate clauses should get ignored"
+ (is (= {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ (mbql.u/add-order-by-clause {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ [:asc [:field-id 10]]))))
+
+ (testing "as should clauses that reference the same Field"
+ (is (= {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ (mbql.u/add-order-by-clause {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ [:desc [:field-id 10]])))
+
+ (testing "fields with different amounts of wrapping (plain field vs datetime-field)"
+ (is (= {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ (mbql.u/add-order-by-clause {:source-table 1
+ :order-by [[:asc [:field-id 10]]]}
+ [:asc [:datetime-field [:field-id 10] :day]]))))))
(deftest combine-filter-clauses-test
(is (= [:and [:= [:field-id 1] 100] [:= [:field-id 2] 200]]
@@ -763,151 +752,134 @@
:aggregation [[:avg [:field-id 1]]
[:max [:field-id 1]]]}})
-(expect
- [:avg [:field-id 1]]
- (mbql.u/aggregation-at-index query-with-some-nesting 0))
-
-(expect
- [:max [:field-id 1]]
- (mbql.u/aggregation-at-index query-with-some-nesting 1))
-
-(expect
- [:avg [:field-id 1]]
- (mbql.u/aggregation-at-index query-with-some-nesting 0 0))
-
-(expect
- [:stddev [:field-id 1]]
- (mbql.u/aggregation-at-index query-with-some-nesting 0 1))
-
-(expect
- [:min [:field-id 1]]
- (mbql.u/aggregation-at-index query-with-some-nesting 1 1))
+(deftest aggregation-at-index-test
+ (doseq [[input expected] {[0] [:avg [:field-id 1]]
+ [1] [:max [:field-id 1]]
+ [0 0] [:avg [:field-id 1]]
+ [0 1] [:stddev [:field-id 1]]
+ [1 1] [:min [:field-id 1]]}]
+ (testing (pr-str (cons 'aggregation-at-index input))
+ (is (= expected
+ (apply mbql.u/aggregation-at-index query-with-some-nesting input))))))
;;; --------------------------------- Unique names & transforming ags to have names ----------------------------------
-;; can we generate unique names?
-(expect
- ["count" "sum" "count_2" "count_3"]
- (mbql.u/uniquify-names ["count" "sum" "count" "count"]))
-
-(expect
- [[:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:count] {:name "count_2"}]
- [:aggregation-options [:count] {:name "count_3"}]]
- (mbql.u/uniquify-named-aggregations
- [[:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:count] {:name "count"}]]))
-
-;; what if we try to trick it by using a name it would have generated?
-(expect
- ["count" "count_2" "count_2_2"]
- (mbql.u/uniquify-names ["count" "count" "count_2"]))
-
-(expect
- [[:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:count] {:name "count_2"}]
- [:aggregation-options [:count] {:name "count_2_2"}]]
- (mbql.u/uniquify-named-aggregations
- [[:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:count] {:name "count"}]
- [:aggregation-options [:count] {:name "count_2"}]]))
-
-;; for wacky DBMSes like SQLServer that return blank column names sometimes let's make sure we handle those without
-;; exploding
-(expect
- ["" "_2"]
- (mbql.u/uniquify-names ["" ""]))
-
-;; can we wrap all of our aggregation clauses in `:named` clauses?
-(defn- simple-ag->name [[ag-name]]
- (name ag-name))
-
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:count [:field-id 1]] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
- (mbql.u/pre-alias-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:count [:field-id 1]]
- [:sum [:field-id 1]]
- [:avg [:field-id 1]]
- [:sum [:field-id 1]]
- [:min [:field-id 1]]]))
-
-;; we shouldn't change the name of ones that are already named
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:count [:field-id 1]] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
- (mbql.u/pre-alias-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:count [:field-id 1]]
- [:sum [:field-id 1]]
- [:avg [:field-id 1]]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:min [:field-id 1]]]))
-
-;; ok, can we do the same thing as the tests above but make those names *unique* at the same time?
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"} ]
- [:aggregation-options [:count [:field-id 1]] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:aggregation-options [:avg [:field-id 1]] {:name "avg"} ]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_3"}]
- [:aggregation-options [:min [:field-id 1]] {:name "min"} ]]
- (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:count [:field-id 1]]
- [:sum [:field-id 1]]
- [:avg [:field-id 1]]
- [:sum [:field-id 1]]
- [:min [:field-id 1]]]))
-
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:count [:field-id 1]] {:name "count"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2_2"}]
- [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
- (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:count [:field-id 1]]
- [:sum [:field-id 1]]
- [:avg [:field-id 1]]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:min [:field-id 1]]]))
-
-;; if `:aggregation-options` only specifies `:display-name` it should still a new `:name`.
-;; `pre-alias-and-uniquify-aggregations` shouldn't stomp over display name
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:aggregation-options [:sum [:field-id 1]] {:display-name "Sum of Field 1", :name "sum_3"}]]
- (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:sum [:field-id 1]]
- [:aggregation-options [:sum [:field-id 1]] {:display-name "Sum of Field 1"}]]))
-
-;; if both are specified, `display-name` should still be propogated
-(expect
- [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2_2", :display-name "Sum of Field 1"}]]
- (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
- [[:sum [:field-id 1]]
- [:sum [:field-id 1]]
- [:aggregation-options [:sum [:field-id 1]] {:name "sum_2", :display-name "Sum of Field 1"}]]))
+(deftest uniquify-names
+ (testing "can we generate unique names?"
+ (is (= ["count" "sum" "count_2" "count_3"]
+ (mbql.u/uniquify-names ["count" "sum" "count" "count"]))))
+
+ (testing "what if we try to trick it by using a name it would have generated?"
+ (is (= ["count" "count_2" "count_2_2"]
+ (mbql.u/uniquify-names ["count" "count" "count_2"]))))
+
+ (testing (str "for wacky DBMSes like SQL Server that return blank column names sometimes let's make sure we handle "
+ "those without exploding")
+ (is (= ["" "_2"]
+ (mbql.u/uniquify-names ["" ""])))))
+
+(deftest uniquify-named-aggregations-test
+ (is (= [[:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:count] {:name "count_2"}]
+ [:aggregation-options [:count] {:name "count_3"}]]
+ (mbql.u/uniquify-named-aggregations
+ [[:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:count] {:name "count"}]])))
+
+ (testing "what if we try to trick it by using a name it would have generated?"
+ (is (= [[:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:count] {:name "count_2"}]
+ [:aggregation-options [:count] {:name "count_2_2"}]]
+ (mbql.u/uniquify-named-aggregations
+ [[:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:count] {:name "count"}]
+ [:aggregation-options [:count] {:name "count_2"}]])))))
+
+(deftest pre-alias-aggregations-test
+ (letfn [(simple-ag->name [[ag-name]]
+ (name ag-name))]
+ (testing "can we wrap all of our aggregation clauses in `:named` clauses?"
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:count [:field-id 1]] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
+ (mbql.u/pre-alias-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:count [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:avg [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:min [:field-id 1]]]))))
+
+ (testing "we shouldn't change the name of ones that are already named"
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:count [:field-id 1]] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
+ (mbql.u/pre-alias-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:count [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:avg [:field-id 1]]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:min [:field-id 1]]]))))
+
+
+ (testing "ok, can we do the same thing as the tests above but make those names *unique* at the same time?"
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"} ]
+ [:aggregation-options [:count [:field-id 1]] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:aggregation-options [:avg [:field-id 1]] {:name "avg"} ]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_3"}]
+ [:aggregation-options [:min [:field-id 1]] {:name "min"} ]]
+ (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:count [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:avg [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:min [:field-id 1]]])))
+
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:count [:field-id 1]] {:name "count"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:aggregation-options [:avg [:field-id 1]] {:name "avg"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2_2"}]
+ [:aggregation-options [:min [:field-id 1]] {:name "min"}]]
+ (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:count [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:avg [:field-id 1]]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:min [:field-id 1]]]))))
+
+ (testing (str "if `:aggregation-options` only specifies `:display-name` it should still a new `:name`. "
+ "`pre-alias-and-uniquify-aggregations` shouldn't stomp over display name")
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:aggregation-options [:sum [:field-id 1]] {:display-name "Sum of Field 1", :name "sum_3"}]]
+ (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:aggregation-options [:sum [:field-id 1]] {:display-name "Sum of Field 1"}]])))
+
+ (testing "if both are specified, `display-name` should still be propagated"
+ (is (= [[:aggregation-options [:sum [:field-id 1]] {:name "sum"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2"}]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2_2", :display-name "Sum of Field 1"}]]
+ (mbql.u/pre-alias-and-uniquify-aggregations simple-ag->name
+ [[:sum [:field-id 1]]
+ [:sum [:field-id 1]]
+ [:aggregation-options [:sum [:field-id 1]] {:name "sum_2", :display-name "Sum of Field 1"}]])))))))
(deftest unique-name-generator-test
(testing "Can we get a simple unique name generator"
@@ -923,100 +895,69 @@
;;; --------------------------------------------- query->max-rows-limit ----------------------------------------------
-;; should return `:limit` if set
-(expect
- 10
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1, :limit 10}}))
-
-;; should return `:page` items if set
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1, :page {:page 1, :items 5}}}))
-
-;; if `:max-results` is set return that
-(expect
- 15
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1}, :constraints {:max-results 15}}))
-
-;; if `:max-results-bare-rows` is set AND query has no aggregations, return that
-(expect
- 10
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1}, :constraints {:max-results 5, :max-results-bare-rows 10}}))
-
-(expect
- 10
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :native
- :native {:query "SELECT * FROM my_table"}
- :constraints {:max-results 5, :max-results-bare-rows 10}}))
-
-;; if `:max-results-bare-rows` is set but query has aggregations, return `:max-results` instead
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1, :aggregation [[:count]]}
- :constraints {:max-results 5, :max-results-bare-rows 10}}))
-
-;; if both `:limit` and `:page` are set (not sure makes sense), return the smaller of the two
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1, :limit 10, :page {:page 1, :items 5}}}))
-
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1, :type :query, :query {:source-table 1, :limit 5, :page {:page 1, :items 10}}}))
-
-;; if both `:limit` and `:constraints` are set, prefer the smaller of the two
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1, :limit 5}
- :constraints {:max-results 10}}))
-
-(expect
- 10
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1, :limit 15}
- :constraints {:max-results 10}}))
-
-;; since this query doesn't have an aggregation we should be using `max-results-bare-rows`
-(expect
- 5
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1, :limit 10}
- :constraints {:max-results 15, :max-results-bare-rows 5}}))
-
-;; add an aggregation, and `:max-results` is used instead; since `:limit` is lower, return that
-(expect
- 10
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1, :limit 10, :aggregation [[:count]]}
- :constraints {:max-results 15, :max-results-bare-rows 5}}))
-
-;; if nothing is set return `nil`
-(expect
- nil
- (mbql.u/query->max-rows-limit
- {:database 1
- :type :query
- :query {:source-table 1}}))
+(deftest query->max-rows-limit-test
+ (doseq [[group query->expected]
+ {"should return `:limit` if set"
+ {{:database 1, :type :query, :query {:source-table 1, :limit 10}} 10}
+
+ "should return `:page` items if set"
+ {{:database 1, :type :query, :query {:source-table 1, :page {:page 1, :items 5}}} 5}
+
+ "if `:max-results` is set return that"
+ {{:database 1, :type :query, :query {:source-table 1}, :constraints {:max-results 15}} 15}
+
+ "if `:max-results-bare-rows` is set AND query has no aggregations, return that"
+ {{:database 1
+ :type :query
+ :query {:source-table 1}
+ :constraints {:max-results 5, :max-results-bare-rows 10}} 10
+ {:database 1
+ :type :native
+ :native {:query "SELECT * FROM my_table"}
+ :constraints {:max-results 5, :max-results-bare-rows 10}} 10}
+
+ "if `:max-results-bare-rows` is set but query has aggregations, return `:max-results` instead"
+ {{:database 1
+ :type :query
+ :query {:source-table 1, :aggregation [[:count]]}
+ :constraints {:max-results 5, :max-results-bare-rows 10}} 5}
+
+ "if both `:limit` and `:page` are set (not sure makes sense), return the smaller of the two"
+ {{:database 1, :type :query, :query {:source-table 1, :limit 10, :page {:page 1, :items 5}}} 5
+ {:database 1, :type :query, :query {:source-table 1, :limit 5, :page {:page 1, :items 10}}} 5}
+
+ "if both `:limit` and `:constraints` are set, prefer the smaller of the two"
+ {{:database 1
+ :type :query
+ :query {:source-table 1, :limit 5}
+ :constraints {:max-results 10}} 5
+
+ {:database 1
+ :type :query
+ :query {:source-table 1, :limit 15}
+ :constraints {:max-results 10}} 10}
+
+ "since this query doesn't have an aggregation we should be using `max-results-bare-rows`"
+ {{:database 1
+ :type :query
+ :query {:source-table 1, :limit 10}
+ :constraints {:max-results 15, :max-results-bare-rows 5}} 5}
+
+ "add an aggregation, and `:max-results` is used instead; since `:limit` is lower, return that"
+ {{:database 1
+ :type :query
+ :query {:source-table 1, :limit 10, :aggregation [[:count]]}
+ :constraints {:max-results 15, :max-results-bare-rows 5}} 10}
+
+ "if nothing is set return `nil`"
+ {{:database 1
+ :type :query
+ :query {:source-table 1}} nil}} ]
+ (testing group
+ (doseq [[query expected] query->expected]
+ (testing (pr-str (list 'query->max-rows-limit query))
+ (is (= expected
+ (mbql.u/query->max-rows-limit query))))))))
(deftest joined-field-test
(is (= [:joined-field "a" [:field-id 10]]
@@ -1035,35 +976,30 @@
(is (thrown? Exception
(mbql.u/->joined-field "a" [:datetime-field [:joined-field "a" [:field-id 1]] :month])))))
-(expect
- (mbql.u/datetime-arithmetics? [:+ [:field-id 13] [:interval -1 :month]]))
-
-(expect
- (mbql.u/datetime-arithmetics? [:datetime-field [:joined-field "a" [:field-id 1]] :month]))
-
-(expect
- false
- (mbql.u/datetime-arithmetics? [:+ [:field-id 13] 3]))
-
-(expect
- (mbql.u/expression-with-name {:expressions {:two [:+ 1 1]}
- :source-table 1}
- "two"))
-
-;; Make sure `expression-with-name` knows how to reach into the parent query if need be
-(expect
- (mbql.u/expression-with-name {:source-query {:expressions {:two [:+ 1 1]}
- :source-table 1}}
- "two"))
-
-(expect
- 1
- (mbql.u/field-clause->id-or-literal [:field-id 1]))
-
-(expect
- "foo"
- (mbql.u/field-clause->id-or-literal [:field-literal "foo" :type/Integer]))
-
-(expect
- "foo"
- (mbql.u/field-clause->id-or-literal [:expression "foo"]))
+(deftest datetime-arithmetics?-test
+ (is (mbql.u/datetime-arithmetics?
+ [:+ [:field-id 13] [:interval -1 :month]]))
+ (is (mbql.u/datetime-arithmetics?
+ [:datetime-field [:joined-field "a" [:field-id 1]] :month]))
+ (is (not (mbql.u/datetime-arithmetics?
+ [:+ [:field-id 13] 3]))))
+
+(deftest expression-with-name-test
+ (is (= [:+ 1 1]
+ (mbql.u/expression-with-name {:expressions {:two [:+ 1 1]}
+ :source-table 1}
+ "two")))
+
+ (testing "Make sure `expression-with-name` knows how to reach into the parent query if need be"
+ (is (= [:+ 1 1]
+ (mbql.u/expression-with-name {:source-query {:expressions {:two [:+ 1 1]}
+ :source-table 1}}
+ "two")))))
+
+(deftest field-clause->id-or-literal-test
+ (doseq [[input expected] {[:field-id 1] 1
+ [:field-literal "foo" :type/Integer] "foo"
+ [:expression "foo"] "foo"}]
+ (testing (pr-str (list 'field-clause->id-or-literal input))
+ (is (= expected
+ (mbql.u/field-clause->id-or-literal input))))))
diff --git a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
index f8c1d4a83ec300060a8390562c68900714f733d8..e75f2a2156d41c16ed13db143b056dfe9110acfb 100644
--- a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
+++ b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
@@ -2,7 +2,6 @@
"Tests for specific behavior of the Oracle driver."
(:require [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
- [expectations :refer [expect]]
[honeysql.core :as hsql]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
@@ -16,7 +15,6 @@
[metabase.query-processor.test-util :as qp.test-util]
[metabase.test :as mt]
[metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets :refer [expect-with-driver]]
[metabase.test.data.oracle :as oracle.tx]
[metabase.test.data.sql :as sql.tx]
[metabase.test.data.sql.ddl :as ddl]
@@ -54,18 +52,17 @@
(sql-jdbc.conn/connection-details->spec :oracle details))
message)))
-;; no SID and not Service Name should throw an exception
-(expect
- AssertionError
- (sql-jdbc.conn/connection-details->spec :oracle {:host "localhost"
- :port 1521}))
-
-(expect
- "You must specify the SID and/or the Service Name."
- (try (sql-jdbc.conn/connection-details->spec :oracle {:host "localhost"
- :port 1521})
- (catch Throwable e
- (driver/humanize-connection-error-message :oracle (.getMessage e)))))
+(deftest require-sid-or-service-name-test
+ (testing "no SID and no Service Name should throw an exception"
+ (is (thrown?
+ AssertionError
+ (sql-jdbc.conn/connection-details->spec :oracle {:host "localhost"
+ :port 1521})))
+ (is (= "You must specify the SID and/or the Service Name."
+ (try (sql-jdbc.conn/connection-details->spec :oracle {:host "localhost"
+ :port 1521})
+ (catch Throwable e
+ (driver/humanize-connection-error-message :oracle (.getMessage e))))))))
(deftest test-ssh-connection
(testing "Gets an error when it can't connect to oracle via ssh tunnel"
@@ -96,9 +93,10 @@
(throw e))
(some-> (.getCause e) recur))))))))))
-(expect-with-driver :oracle
- "UTC"
- (tu/db-timezone-id))
+(deftest timezone-id-test
+ (mt/test-driver :oracle
+ (is (= "UTC"
+ (tu/db-timezone-id)))))
(deftest insert-rows-ddl-test
(is (= [[(str "INSERT ALL"
@@ -127,31 +125,32 @@
`(do-with-temp-user ~username (fn [~username-binding] ~@body)))
-;; Make sure Oracle CLOBs are returned as text (#9026)
-(expect-with-driver :oracle
- [[1M "Hello"]
- [2M nil]]
- (let [details (:details (data/db))
- spec (sql-jdbc.conn/connection-details->spec :oracle details)
- execute! (fn [format-string & args]
- (jdbc/execute! spec (apply format format-string args)))
- pk-type (sql.tx/pk-sql-type :oracle)]
- (with-temp-user [username]
- (execute! "CREATE TABLE \"%s\".\"messages\" (\"id\" %s, \"message\" CLOB)" username pk-type)
- (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (1, 'Hello')" username)
- (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (2, NULL)" username)
- (tt/with-temp* [Table [table {:schema username, :name "messages", :db_id (data/id)}]
- Field [id-field {:table_id (u/get-id table), :name "id", :base_type "type/Integer"}]
- Field [_ {:table_id (u/get-id table), :name "message", :base_type "type/Text"}]]
- (qp.test/rows
- (qp/process-query
- {:database (data/id)
- :type :query
- :query {:source-table (u/get-id table)
- :order-by [[:asc [:field-id (u/get-id id-field)]]]}}))))))
+(deftest return-clobs-as-text-test
+ (mt/test-driver :oracle
+ (testing "Make sure Oracle CLOBs are returned as text (#9026)"
+ (let [details (:details (data/db))
+ spec (sql-jdbc.conn/connection-details->spec :oracle details)
+ execute! (fn [format-string & args]
+ (jdbc/execute! spec (apply format format-string args)))
+ pk-type (sql.tx/pk-sql-type :oracle)]
+ (with-temp-user [username]
+ (execute! "CREATE TABLE \"%s\".\"messages\" (\"id\" %s, \"message\" CLOB)" username pk-type)
+ (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (1, 'Hello')" username)
+ (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (2, NULL)" username)
+ (tt/with-temp* [Table [table {:schema username, :name "messages", :db_id (data/id)}]
+ Field [id-field {:table_id (u/get-id table), :name "id", :base_type "type/Integer"}]
+ Field [_ {:table_id (u/get-id table), :name "message", :base_type "type/Text"}]]
+ (is (= [[1M "Hello"]
+ [2M nil]]
+ (qp.test/rows
+ (qp/process-query
+ {:database (data/id)
+ :type :query
+ :query {:source-table (u/get-id table)
+ :order-by [[:asc [:field-id (u/get-id id-field)]]]}}))))))))))
(deftest handle-slashes-test
- (datasets/test-driver :oracle
+ (mt/test-driver :oracle
(let [details (:details (data/db))
spec (sql-jdbc.conn/connection-details->spec :oracle details)
execute! (fn [format-string & args]
@@ -170,7 +169,7 @@
;; let's make sure we're actually attempting to generate the correctl HoneySQL for joins and source queries so we
;; don't sit around scratching our heads wondering why the queries themselves aren't working
(deftest honeysql-test
- (datasets/test-driver :oracle
+ (mt/test-driver :oracle
(is (= {:select [:*]
:from [{:select
[[(hx/identifier :field oracle.tx/session-schema "test_data_venues" "id")
diff --git a/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj b/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
index 50473c48e3da85dde1f70fd6aa72dda80d65acc0..e9cdbea0c05a799b8c73ae2192cdf94223bf373d 100644
--- a/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
+++ b/modules/drivers/sqlserver/test/metabase/driver/sqlserver_test.clj
@@ -22,7 +22,6 @@
;;; -------------------------------------------------- VARCHAR(MAX) --------------------------------------------------
-;; Make sure something long doesn't come back as some weird type like `ClobImpl`
(def ^:private a-gene
"Really long string representing a gene like \"GGAGCACCTCCACAAGTGCAGGCTATCCTGTCGAGTAAGGCCT...\""
(apply str (repeatedly 1000 (partial rand-nth [\A \G \C \T]))))
@@ -32,12 +31,14 @@
[{:field-name "gene", :base-type {:native "VARCHAR(MAX)"}}]
[[a-gene]]]])
-(datasets/expect-with-driver :sqlserver
- [[1 a-gene]]
- (-> (data/dataset metabase.driver.sqlserver-test/genetic-data (data/run-mbql-query genetic-data))
- :data
- :rows
- obj->json->obj)) ; convert to JSON + back so the Clob gets stringified
+(deftest clobs-should-come-back-as-text-test
+ (mt/test-driver :sqlserver
+ (testing "Make sure something long doesn't come back as some weird type like `ClobImpl`"
+ (is (= [[1 a-gene]]
+ (-> (data/dataset metabase.driver.sqlserver-test/genetic-data (data/run-mbql-query genetic-data))
+ :data
+ :rows
+ obj->json->obj)))))) ; convert to JSON + back so the Clob gets stringified
(deftest connection-spec-test
(testing "Test that additional connection string options work (#5296)"
@@ -63,84 +64,88 @@
;; `<version>`
(update :applicationName #(str/replace % #"\s.*$" " <version>")))))))
-(datasets/expect-with-driver :sqlserver
- "UTC"
- (tu/db-timezone-id))
+(deftest timezone-id-test
+ (mt/test-driver :sqlserver
+ (is (= "UTC"
+ (tu/db-timezone-id)))))
-;; SQL Server doesn't let you use ORDER BY in nested SELECTs unless you also specify a TOP (their equivalent of
-;; LIMIT). Make sure we add a max-results LIMIT to the nested query
-(datasets/expect-with-driver :sqlserver
- {:query (str
- "SELECT TOP 1048576 \"source\".\"name\" AS \"name\" "
- "FROM ("
- "SELECT TOP 1048576 "
- "\"dbo\".\"venues\".\"name\" AS \"name\" "
- "FROM \"dbo\".\"venues\" "
- "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
- " ) \"source\" ") ; not sure why this generates an extra space before the closing paren, but it does
- :params nil}
- (qp/query->native
- (data/mbql-query venues
- {:source-query {:source-table $$venues
- :fields [$name]
- :order-by [[:asc $id]]}})))
+(deftest add-max-results-limit-test
+ (mt/test-driver :sqlserver
+ (testing (str "SQL Server doesn't let you use ORDER BY in nested SELECTs unless you also specify a TOP (their "
+ "equivalent of LIMIT). Make sure we add a max-results LIMIT to the nested query")
+ (is (= {:query (str "SELECT TOP 1048576 \"source\".\"name\" AS \"name\" "
+ "FROM ("
+ "SELECT TOP 1048576 "
+ "\"dbo\".\"venues\".\"name\" AS \"name\" "
+ "FROM \"dbo\".\"venues\" "
+ "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
+ " ) \"source\" ") ; not sure why this generates an extra space before the closing paren, but it does
+ :params nil}
+ (qp/query->native
+ (mt/mbql-query venues
+ {:source-query {:source-table $$venues
+ :fields [$name]
+ :order-by [[:asc $id]]}})))))))
-;; make sure when adding TOP clauses to make ORDER BY work we don't stomp over any explicit TOP clauses that may have
-;; been set in the query
-(datasets/expect-with-driver :sqlserver
- {:query (str "SELECT TOP 10 \"source\".\"name\" AS \"name\" "
- "FROM ("
- "SELECT TOP 20 "
- "\"dbo\".\"venues\".\"name\" AS \"name\" "
- "FROM \"dbo\".\"venues\" "
- "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
- " ) \"source\" ")
- :params nil}
- (qp/query->native
- (data/mbql-query venues
- {:source-query {:source-table $$venues
- :fields [$name]
- :order-by [[:asc $id]]
- :limit 20}
- :limit 10})))
+(deftest preserve-existing-top-clauses
+ (mt/test-driver :sqlserver
+ (testing (str "make sure when adding TOP clauses to make ORDER BY work we don't stomp over any explicit TOP "
+ "clauses that may have been set in the query")
+ (is (= {:query (str "SELECT TOP 10 \"source\".\"name\" AS \"name\" "
+ "FROM ("
+ "SELECT TOP 20 "
+ "\"dbo\".\"venues\".\"name\" AS \"name\" "
+ "FROM \"dbo\".\"venues\" "
+ "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
+ " ) \"source\" ")
+ :params nil}
+ (qp/query->native
+ (mt/mbql-query venues
+ {:source-query {:source-table $$venues
+ :fields [$name]
+ :order-by [[:asc $id]]
+ :limit 20}
+ :limit 10})))))))
-;; We don't need to add TOP clauses for top-level order by. Normally we always add one anyway because of the
-;; max-results stuff, but make sure our impl doesn't add one when it's not in the source MBQL
-(datasets/expect-with-driver :sqlserver
- {:query (str "SELECT \"source\".\"name\" AS \"name\" "
- "FROM ("
- "SELECT TOP 1048576 "
- "\"dbo\".\"venues\".\"name\" AS \"name\" "
- "FROM \"dbo\".\"venues\" "
- "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
- " ) \"source\" "
- "ORDER BY \"source\".\"id\" ASC")
- :params nil}
- ;; in order to actually see how things would work without the implicit max-results limit added we'll preprocess
- ;; the query, strip off the `:limit` that got added, and then feed it back to the QP where we left off
- (let [preprocessed (-> (data/mbql-query venues
- {:source-query {:source-table $$venues
- :fields [$name]
- :order-by [[:asc $id]]}
- :order-by [[:asc $id]]})
- qp/query->preprocessed
- (m/dissoc-in [:query :limit]))]
- (qp.test-util/with-everything-store
- (driver/mbql->native :sqlserver preprocessed))))
+(deftest dont-add-top-clauses-for-top-level-test
+ (mt/test-driver :sqlserver
+ (testing (str "We don't need to add TOP clauses for top-level order by. Normally we always add one anyway because "
+ "of the max-results stuff, but make sure our impl doesn't add one when it's not in the source MBQL"))
+ ;; in order to actually see how things would work without the implicit max-results limit added we'll preprocess
+ ;; the query, strip off the `:limit` that got added, and then feed it back to the QP where we left off
+ (let [preprocessed (-> (mt/mbql-query venues
+ {:source-query {:source-table $$venues
+ :fields [$name]
+ :order-by [[:asc $id]]}
+ :order-by [[:asc $id]]})
+ qp/query->preprocessed
+ (m/dissoc-in [:query :limit]))]
+ (qp.test-util/with-everything-store
+ (is (= {:query (str "SELECT \"source\".\"name\" AS \"name\" "
+ "FROM ("
+ "SELECT TOP 1048576 "
+ "\"dbo\".\"venues\".\"name\" AS \"name\" "
+ "FROM \"dbo\".\"venues\" "
+ "ORDER BY \"dbo\".\"venues\".\"id\" ASC"
+ " ) \"source\" "
+ "ORDER BY \"source\".\"id\" ASC")
+ :params nil}
+ (driver/mbql->native :sqlserver preprocessed)))))))
-;; ok, generating all that SQL above is nice, but let's make sure our queries actually work!
-(datasets/expect-with-driver :sqlserver
- [["Red Medicine"]
- ["Stout Burgers & Beers"]
- ["The Apple Pan"]]
- (qp.test/rows
- (qp/process-query
- (data/mbql-query venues
- {:source-query {:source-table $$venues
- :fields [$name]
- :order-by [[:asc $id]]
- :limit 5}
- :limit 3}))))
+(deftest max-results-should-actually-work-test
+ (mt/test-driver :sqlserver
+ (testing "ok, generating all that SQL above is nice, but let's make sure our queries actually work!"
+ (is (= [["Red Medicine"]
+ ["Stout Burgers & Beers"]
+ ["The Apple Pan"]]
+ (qp.test/rows
+ (qp/process-query
+ (mt/mbql-query venues
+ {:source-query {:source-table $$venues
+ :fields [$name]
+ :order-by [[:asc $id]]
+ :limit 5}
+ :limit 3}))))))))
(deftest locale-bucketing-test
(datasets/test-driver :sqlserver
diff --git a/test/expectations.clj b/test/expectations.clj
index 773543d84e94aee1d3205233adb9248f9749945b..52adcebb0f861f7ad180ee6dd57a68752b9a07ff 100644
--- a/test/expectations.clj
+++ b/test/expectations.clj
@@ -136,9 +136,8 @@
(when-not (env/env :drivers)
(t/testing "Don't write any new tests using expect!"
(let [ee? (u/ignore-exceptions (require 'metabase-enterprise.core) true)]
- ;; TODO - update the numbers for EE
- (t/is (<= total-expect-forms (if ee? 871 842)))
- (t/is (<= total-namespaces-using-expect (if ee? 84 80))))))))
+ (t/is (<= total-expect-forms (if ee? 549 524)))
+ (t/is (<= total-namespaces-using-expect (if ee? 59 56))))))))
(defmacro ^:deprecated expect
"Simple macro that simulates converts an Expectations-style `expect` form into a `clojure.test` `deftest` form."
diff --git a/test/metabase/api/transform_test.clj b/test/metabase/api/transform_test.clj
index 3e373286f54e31c4839c0cc6837bc20707319ef9..63462b5bf721d57eb34696c26cbae46b2080c5ee 100644
--- a/test/metabase/api/transform_test.clj
+++ b/test/metabase/api/transform_test.clj
@@ -1,46 +1,42 @@
(ns metabase.api.transform-test
(:require [clojure.test :refer :all]
- [expectations :refer :all]
[metabase.models.card :refer [Card]]
[metabase.models.collection :refer [Collection]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.query-processor :as qp]
- [metabase.test.data :as data]
- [metabase.test.data.users :as test-users]
+ [metabase.test :as mt]
[metabase.test.domain-entities :refer :all]
[metabase.test.fixtures :as fixtures]
- [metabase.test.transforms :refer :all]
- [metabase.test.util :as tu]))
+ [metabase.test.transforms :refer :all]))
(use-fixtures :once (fixtures/initialize :db))
-(defn- test-endpoint
- []
- (format "transform/%s/%s/%s" (data/id) "PUBLIC" "Test transform"))
+(defn- test-endpoint []
+ (format "transform/%s/%s/%s" (mt/id) "PUBLIC" "Test transform"))
-;; Run the transform and make sure it produces the correct result
-(expect
- [[1 "Red Medicine" 4 10.0646 -165.374 3 1.5 4 3 2 1]
- [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2 2.0 11 2 1 1]
- [3 "The Apple Pan" 11 34.0406 -118.428 2 2.0 11 2 1 1]]
- (test-users/with-test-user :rasta
- (with-test-transform-specs
- (with-test-domain-entity-specs
- (tu/with-model-cleanup [Card Collection]
- (-> ((test-users/user->client :rasta) :get 200 (test-endpoint))
- first
- :dataset_query
- qp/process-query
- :data
- :rows))))))
+(deftest transform-test
+ (testing "GET /api/transform/:db-id/:schema/:transform-name"
+ (testing "Run the transform and make sure it produces the correct result"
+ (mt/with-test-user :rasta
+ (with-test-transform-specs
+ (with-test-domain-entity-specs
+ (mt/with-model-cleanup [Card Collection]
+ (is (= [[1 "Red Medicine" 4 10.0646 -165.374 3 1.5 4 3 2 1]
+ [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2 2.0 11 2 1 1]
+ [3 "The Apple Pan" 11 34.0406 -118.428 2 2.0 11 2 1 1]]
+ (-> (mt/user-http-request :rasta :get 200 (test-endpoint))
+ first
+ :dataset_query
+ qp/process-query
+ mt/rows))))))))))
(deftest permissions-test
(testing "GET /api/transform/:db-id/:schema/:transform-name"
(testing "Do we correctly check for permissions?"
(try
- (perms/revoke-permissions! (perms-group/all-users) (data/id))
+ (perms/revoke-permissions! (perms-group/all-users) (mt/id))
(is (= "You don't have permissions to do that."
- ((test-users/user->client :rasta) :get 403 (test-endpoint))))
+ (mt/user-http-request :rasta :get 403 (test-endpoint))))
(finally
- (perms/grant-permissions! (perms-group/all-users) (perms/object-path (data/id))))))))
+ (perms/grant-permissions! (perms-group/all-users) (perms/object-path (mt/id))))))))
diff --git a/test/metabase/models/field_test.clj b/test/metabase/models/field_test.clj
index a2981731b288c28c72bf1f86749927da339bf164..86e6e005f88f6d56630b7f7739c1b0a7251e2c06 100644
--- a/test/metabase/models/field_test.clj
+++ b/test/metabase/models/field_test.clj
@@ -1,13 +1,15 @@
(ns metabase.models.field-test
"Tests for specific behavior related to the Field model."
- (:require [expectations :refer :all]
+ (:require [clojure.test :refer :all]
[metabase.sync.analyze.classifiers.name :as name]))
-
-;;; infer-field-special-type
-(expect :type/PK (#'name/special-type-for-name-and-base-type "id" :type/Integer))
-;; other pattern matches based on type/regex (remember, base_type matters in matching!)
-(expect :type/Score (#'name/special-type-for-name-and-base-type "rating" :type/Integer))
-(expect nil (#'name/special-type-for-name-and-base-type "rating" :type/Boolean))
-(expect :type/Country (#'name/special-type-for-name-and-base-type "country" :type/Text))
-(expect nil (#'name/special-type-for-name-and-base-type "country" :type/Integer))
+(deftest special-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
+ ["rating" :type/Boolean] nil
+ ["country" :type/Text] :type/Country
+ ["country" :type/Integer] nil}]
+ (testing (pr-str (cons 'special-type-for-name-and-base-type input))
+ (is (= expected
+ (apply #'name/special-type-for-name-and-base-type input))))))
diff --git a/test/metabase/models/permissions_group_membership_test.clj b/test/metabase/models/permissions_group_membership_test.clj
index 12c5e2bab6e99baa9b92fa844885193accb31fee..c3b0aa45418879396b7f4b5b8cea64340d4872f7 100644
--- a/test/metabase/models/permissions_group_membership_test.clj
+++ b/test/metabase/models/permissions_group_membership_test.clj
@@ -1,26 +1,25 @@
(ns metabase.models.permissions-group-membership-test
(:require [clojure.test :refer :all]
- [expectations :refer [expect]]
[metabase.models.permissions-group :as group]
[metabase.models.permissions-group-membership :as pgm :refer [PermissionsGroupMembership]]
[metabase.models.user :refer [User]]
+ [metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
- [toucan.db :as db]
- [toucan.util.test :as tt]))
+ [toucan.db :as db]))
(use-fixtures :once (fixtures/initialize :test-users))
-;; when you create a PermissionsGroupMembership for a User in the admin group, it should set their `is_superuser` flag
-(expect
- true
- (tt/with-temp User [user]
- (db/insert! PermissionsGroupMembership {:user_id (u/get-id user), :group_id (u/get-id (group/admin))})
- (db/select-one-field :is_superuser User :id (u/get-id user))))
+(deftest set-is-superuser-test
+ (testing "when you create a PermissionsGroupMembership for a User in the admin group, it should set their `is_superuser` flag"
+ (mt/with-temp User [user]
+ (db/insert! PermissionsGroupMembership {:user_id (u/the-id user), :group_id (u/the-id (group/admin))})
+ (is (= true
+ (db/select-one-field :is_superuser User :id (u/the-id user)))))))
-;; when you delete a PermissionsGroupMembership for a User in the admin group, it should set their `is_superuser` flag
-(expect
- false
- (tt/with-temp User [user {:is_superuser true}]
- (db/delete! PermissionsGroupMembership :user_id (u/get-id user), :group_id (u/get-id (group/admin)))
- (db/select-one-field :is_superuser User :id (u/get-id user))))
+(deftest remove-is-superuser-test
+ (testing "when you delete a PermissionsGroupMembership for a User in the admin group, it should set their `is_superuser` flag"
+ (mt/with-temp User [user {:is_superuser true}]
+ (db/delete! PermissionsGroupMembership :user_id (u/the-id user), :group_id (u/the-id (group/admin)))
+ (is (= false
+ (db/select-one-field :is_superuser User :id (u/the-id user)))))))
diff --git a/test/metabase/query_processor/middleware/expand_macros_test.clj b/test/metabase/query_processor/middleware/expand_macros_test.clj
index 048d69f81aca67ded72da23500009b32fd4adb50..71084abc128001cd188b251020325f9bfa7f59e6 100644
--- a/test/metabase/query_processor/middleware/expand_macros_test.clj
+++ b/test/metabase/query_processor/middleware/expand_macros_test.clj
@@ -1,5 +1,5 @@
(ns metabase.query-processor.middleware.expand-macros-test
- (:require [expectations :refer [expect]]
+ (:require [clojure.test :refer :all]
[metabase.models.database :refer [Database]]
[metabase.models.metric :refer [Metric]]
[metabase.models.segment :refer [Segment]]
@@ -7,252 +7,241 @@
[metabase.query-processor-test :as qp.test]
[metabase.query-processor.middleware.expand-macros :as expand-macros]
[metabase.test :as mt]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]
- [metabase.util :as u]
- [toucan.util.test :as tt]))
+ [metabase.util :as u]))
(defn- mbql-query [inner-query]
{:database 1, :type :query, :query (merge {:source-table 1}
inner-query)})
-;; no Segment or Metric should yield exact same query
-(expect
- (mbql-query
- {:filter [:> [:field-id 4] 1]
- :breakout [[:field-id 17]]})
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:filter [:> [:field-id 4] 1]
- :breakout [[:field-id 17]]})))
-
-;; just segments
-(expect
- (mbql-query
- {:filter [:and
- [:= [:field-id 5] "abc"]
- [:or
- [:is-null [:field-id 7]]
- [:> [:field-id 4] 1]]]
- :breakout [[:field-id 17]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Segment [{segment-1-id :id} {:table_id table-id
- :definition {:filter [:and [:= [:field-id 5] "abc"]]}}]
- Segment [{segment-2-id :id} {:table_id table-id
- :definition {:filter [:and [:is-null [:field-id 7]]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:filter [:and
- [:segment segment-1-id]
- [:or
- [:segment segment-2-id]
- [:> [:field-id 4] 1]]]
- :breakout [[:field-id 17]]}))))
-
-;; just a metric (w/out nested segments)
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options [:count] {:display-name "Toucans in the rainforest"}]]
- :filter [:and
- [:> [:field-id 4] 1]
- [:= [:field-id 5] "abc"]]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Metric [{metric-1-id :id} {:name "Toucans in the rainforest"
- :table_id table-id
- :definition {:aggregation [[:count]]
- :filter [:and [:= [:field-id 5] "abc"]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:aggregation [[:metric metric-1-id]]
- :filter [:> [:field-id 4] 1]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]}))))
-
-;; check that when the original filter is empty we simply use our metric filter definition instead
-(expect
- (mbql-query
- {:source-table 1000
- :aggregation [[:aggregation-options [:count] {:display-name "ABC Fields"}]]
- :filter [:= [:field-id 5] "abc"]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Metric [{metric-1-id :id} {:name "ABC Fields"
- :table_id table-id
- :definition {:aggregation [[:count]]
- :filter [:and [:= [:field-id 5] "abc"]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:source-table 1000
- :aggregation [[:metric metric-1-id]]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]}))))
-
-;; metric w/ no filter definition
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options [:count] {:display-name "My Metric"}]]
- :filter [:= [:field-id 5] "abc"]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Metric [{metric-1-id :id} {:name "My Metric"
- :table_id table-id
- :definition {:aggregation [[:count]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:aggregation [[:metric metric-1-id]]
- :filter [:= [:field-id 5] "abc"]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]}))))
-
-;; a metric w/ nested segments
-(expect
- (mbql-query
- {:source-table 1000
- :aggregation [[:aggregation-options [:sum [:field-id 18]] {:display-name "My Metric"}]]
- :filter [:and
- [:> [:field-id 4] 1]
- [:is-null [:field-id 7]]
- [:= [:field-id 5] "abc"]
- [:between [:field-id 9] 0 25]]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Segment [{segment-1-id :id} {:table_id table-id
- :definition {:filter [:and [:between [:field-id 9] 0 25]]}}]
- Segment [{segment-2-id :id} {:table_id table-id
- :definition {:filter [:and [:is-null [:field-id 7]]]}}]
- Metric [{metric-1-id :id} {:name "My Metric"
- :table_id table-id
- :definition {:aggregation [[:sum [:field-id 18]]]
- :filter [:and
- [:= [:field-id 5] "abc"]
- [:segment segment-1-id]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:source-table 1000
- :aggregation [[:metric metric-1-id]]
- :filter [:and
- [:> [:field-id 4] 1]
- [:segment segment-2-id]]
- :breakout [[:field-id 17]]
- :order-by [[:asc [:field-id 1]]]}))))
-
-;; Check that a metric w/ multiple aggregation syntax (nested vector) still works correctly
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[2 118]
- [3 39]
- [4 24]]
- (tt/with-temp Metric [metric {:table_id (data/id :venues)
- :definition {:aggregation [[:sum [:field-id (data/id :venues :price)]]]
- :filter [:> [:field-id (data/id :venues :price)] 1]}}]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:metric (u/get-id metric)]]
- :breakout [$price]})))))
-
-;; make sure that we don't try to expand GA "metrics" (#6104)
-(expect
- (mbql-query {:aggregation [[:metric "ga:users"]]})
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:metric "ga:users"]]})))
-
-(expect
- (mbql-query {:aggregation [[:metric "gaid:users"]]})
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:metric "gaid:users"]]})))
-
-;; make sure expansion works with multiple GA "metrics" (#7399)
-(expect
- (mbql-query {:aggregation [[:metric "ga:users"]
- [:metric "ga:1dayUsers"]]})
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:metric "ga:users"]
- [:metric "ga:1dayUsers"]]})))
-
-;; make sure we don't try to expand GA "segments"
-(expect
- (mbql-query {:filter [:segment "gaid:-11"]})
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:filter [:segment "gaid:-11"]})))
-
-;; make sure we can name a :metric (ick)
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options [:sum [:field-id 20]] {:display-name "Named Metric"}]]
- :breakout [[:field-id 10]]})
- (tt/with-temp Metric [metric {:definition {:aggregation [[:sum [:field-id 20]]]}}]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:aggregation-options [:metric (u/get-id metric)] {:display-name "Named Metric"}]]
- :breakout [[:field-id 10]]}))))
-
-;; if the `:metric` is wrapped in aggregation options that do *not* give it a display name, `:display-name` should be
-;; added to the options
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options
- [:sum [:field-id 20]]
- {:name "auto_generated_name", :display-name "Toucans in the rainforest"}]]
- :breakout [[:field-id 10]]})
- (tt/with-temp Metric [metric {:definition {:aggregation [[:sum [:field-id 20]]]}}]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:aggregation-options [:metric (u/get-id metric)] {:name "auto_generated_name"}]]
- :breakout [[:field-id 10]]}))))
-
-;; a Metric whose :aggregation is already named should not get wrapped in an `:aggregation-options` clause
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options [:sum [:field-id 20]] {:display-name "My Cool Aggregation"}]]
- :breakout [[:field-id 10]]})
- (tt/with-temp Metric [metric {:definition {:aggregation [[:aggregation-options
- [:sum [:field-id 20]]
- {:display-name "My Cool Aggregation"}]]}}]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:metric (u/get-id metric)]]
- :breakout [[:field-id 10]]}))))
-
-;; ...but if it's wrapped in `:aggregation-options`, but w/o given a display name, we should merge the options
-(expect
- (mbql-query
- {:aggregation [[:aggregation-options
- [:sum [:field-id 20]]
- {:name "auto_generated_name", :display-name "Toucans in the rainforest"}]]
- :breakout [[:field-id 10]]})
- (tt/with-temp Metric [metric {:definition {:aggregation [[:aggregation-options
- [:sum [:field-id 20]]
- {:name "auto_generated_name"}]]}}]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query {:aggregation [[:metric (u/get-id metric)]]
- :breakout [[:field-id 10]]}))))
-
-
-;; segments in :share clauses
-(expect
- (mbql-query
- {:aggregation [[:share [:and
- [:= [:field-id 5] "abc"]
- [:or
- [:is-null [:field-id 7]]
- [:> [:field-id 4] 1]]]]]})
- (tt/with-temp* [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Segment [{segment-1-id :id} {:table_id table-id
- :definition {:filter [:and [:= [:field-id 5] "abc"]]}}]
- Segment [{segment-2-id :id} {:table_id table-id
- :definition {:filter [:and [:is-null [:field-id 7]]]}}]]
- (#'expand-macros/expand-metrics-and-segments
- (mbql-query
- {:aggregation [[:share [:and
- [:segment segment-1-id]
- [:or
- [:segment segment-2-id]
- [:> [:field-id 4] 1]]]]]}))))
+(deftest basic-expansion-test
+ (testing "no Segment or Metric should yield exact same query"
+ (is (= (mbql-query
+ {:filter [:> [:field-id 4] 1]
+ :breakout [[:field-id 17]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:filter [:> [:field-id 4] 1]
+ :breakout [[:field-id 17]]}))))))
+
+(deftest segments-test
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Segment [{segment-1-id :id} {:table_id table-id
+ :definition {:filter [:and [:= [:field-id 5] "abc"]]}}]
+ Segment [{segment-2-id :id} {:table_id table-id
+ :definition {:filter [:and [:is-null [:field-id 7]]]}}]]
+ (is (= (mbql-query
+ {:filter [:and
+ [:= [:field-id 5] "abc"]
+ [:or
+ [:is-null [:field-id 7]]
+ [:> [:field-id 4] 1]]]
+ :breakout [[:field-id 17]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:filter [:and
+ [:segment segment-1-id]
+ [:or
+ [:segment segment-2-id]
+ [:> [:field-id 4] 1]]]
+ :breakout [[:field-id 17]]}))))))
+
+(deftest metric-test
+ (testing "just a metric (w/out nested segments)"
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Metric [{metric-1-id :id} {:name "Toucans in the rainforest"
+ :table_id table-id
+ :definition {:aggregation [[:count]]
+ :filter [:and [:= [:field-id 5] "abc"]]}}]]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options [:count] {:display-name "Toucans in the rainforest"}]]
+ :filter [:and
+ [:> [:field-id 4] 1]
+ [:= [:field-id 5] "abc"]]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:aggregation [[:metric metric-1-id]]
+ :filter [:> [:field-id 4] 1]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})))))))
+
+(deftest use-metric-filter-definition-test
+ (testing "check that when the original filter is empty we simply use our metric filter definition instead"
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Metric [{metric-1-id :id} {:name "ABC Fields"
+ :table_id table-id
+ :definition {:aggregation [[:count]]
+ :filter [:and [:= [:field-id 5] "abc"]]}}]]
+ (is (= (mbql-query
+ {:source-table 1000
+ :aggregation [[:aggregation-options [:count] {:display-name "ABC Fields"}]]
+ :filter [:= [:field-id 5] "abc"]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:source-table 1000
+ :aggregation [[:metric metric-1-id]]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})))))))
+
+(deftest metric-with-no-filter-test
+ (testing "metric w/ no filter definition"
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Metric [{metric-1-id :id} {:name "My Metric"
+ :table_id table-id
+ :definition {:aggregation [[:count]]}}]]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options [:count] {:display-name "My Metric"}]]
+ :filter [:= [:field-id 5] "abc"]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:aggregation [[:metric metric-1-id]]
+ :filter [:= [:field-id 5] "abc"]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})))))))
+
+(deftest metric-with-nested-segments-test
+ (testing "metric w/ nested segments"
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Segment [{segment-1-id :id} {:table_id table-id
+ :definition {:filter [:and [:between [:field-id 9] 0 25]]}}]
+ Segment [{segment-2-id :id} {:table_id table-id
+ :definition {:filter [:and [:is-null [:field-id 7]]]}}]
+ Metric [{metric-1-id :id} {:name "My Metric"
+ :table_id table-id
+ :definition {:aggregation [[:sum [:field-id 18]]]
+ :filter [:and
+ [:= [:field-id 5] "abc"]
+ [:segment segment-1-id]]}}]]
+ (is (= (mbql-query
+ {:source-table 1000
+ :aggregation [[:aggregation-options [:sum [:field-id 18]] {:display-name "My Metric"}]]
+ :filter [:and
+ [:> [:field-id 4] 1]
+ [:is-null [:field-id 7]]
+ [:= [:field-id 5] "abc"]
+ [:between [:field-id 9] 0 25]]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:source-table 1000
+ :aggregation [[:metric metric-1-id]]
+ :filter [:and
+ [:> [:field-id 4] 1]
+ [:segment segment-2-id]]
+ :breakout [[:field-id 17]]
+ :order-by [[:asc [:field-id 1]]]})))))))
+
+(deftest metric-with-multiple-aggregation-syntax-test
+ (testing "Check that a metric w/ multiple aggregation syntax (nested vector) still works correctly"
+ ;; so-called "multiple aggregation syntax" is the norm now -- query normalization will do this automatically
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (mt/with-temp Metric [metric {:table_id (mt/id :venues)
+ :definition {:aggregation [[:sum [:field-id (mt/id :venues :price)]]]
+ :filter [:> [:field-id (mt/id :venues :price)] 1]}}]
+ (is (= [[2 118]
+ [3 39]
+ [4 24]]
+ (qp.test/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:metric (u/the-id metric)]]
+ :breakout [$price]}))))))))
+
+(deftest dont-expand-ga-metrics-test
+ (testing "make sure that we don't try to expand GA \"metrics\" (#6104)"
+ (doseq [metric ["ga:users" "gaid:users"]]
+ (is (= (mbql-query {:aggregation [[:metric metric]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:metric metric]]}))))))
+
+ (testing "make sure expansion works with multiple GA \"metrics\" (#7399)"
+ (is (= (mbql-query {:aggregation [[:metric "ga:users"]
+ [:metric "ga:1dayUsers"]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:metric "ga:users"]
+ [:metric "ga:1dayUsers"]]}))))))
+
+(deftest dont-expand-ga-segments-test
+ (testing "make sure we don't try to expand GA 'segments'"
+ (is (= (mbql-query {:filter [:segment "gaid:-11"]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:filter [:segment "gaid:-11"]}))))))
+
+(deftest named-metrics-test
+ (testing "make sure we can name a :metric"
+ (mt/with-temp Metric [metric {:definition {:aggregation [[:sum [:field-id 20]]]}}]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options [:sum [:field-id 20]] {:display-name "Named Metric"}]]
+ :breakout [[:field-id 10]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:aggregation-options [:metric (u/the-id metric)] {:display-name "Named Metric"}]]
+ :breakout [[:field-id 10]]})))))))
+
+(deftest include-display-name-test
+ (testing (str "if the `:metric` is wrapped in aggregation options that do *not* give it a display name, "
+ "`:display-name` should be added to the options")
+ (mt/with-temp Metric [metric {:definition {:aggregation [[:sum [:field-id 20]]]}}]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options
+ [:sum [:field-id 20]]
+ {:name "auto_generated_name", :display-name "Toucans in the rainforest"}]]
+ :breakout [[:field-id 10]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:aggregation-options [:metric (u/the-id metric)] {:name "auto_generated_name"}]]
+ :breakout [[:field-id 10]]}))))))
+
+ (testing "a Metric whose :aggregation is already named should not get wrapped in an `:aggregation-options` clause"
+ (mt/with-temp Metric [metric {:definition {:aggregation [[:aggregation-options
+ [:sum [:field-id 20]]
+ {:display-name "My Cool Aggregation"}]]}}]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options [:sum [:field-id 20]] {:display-name "My Cool Aggregation"}]]
+ :breakout [[:field-id 10]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:metric (u/the-id metric)]]
+ :breakout [[:field-id 10]]}))))))
+
+ (testing "...but if it's wrapped in `:aggregation-options`, but w/o given a display name, we should merge the options"
+ (mt/with-temp Metric [metric {:definition {:aggregation [[:aggregation-options
+ [:sum [:field-id 20]]
+ {:name "auto_generated_name"}]]}}]
+ (is (= (mbql-query
+ {:aggregation [[:aggregation-options
+ [:sum [:field-id 20]]
+ {:name "auto_generated_name", :display-name "Toucans in the rainforest"}]]
+ :breakout [[:field-id 10]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query {:aggregation [[:metric (u/the-id metric)]]
+ :breakout [[:field-id 10]]})))))))
+
+(deftest segments-in-share-clauses-test
+ (testing "segments in :share clauses"
+ (mt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Segment [{segment-1-id :id} {:table_id table-id
+ :definition {:filter [:and [:= [:field-id 5] "abc"]]}}]
+ Segment [{segment-2-id :id} {:table_id table-id
+ :definition {:filter [:and [:is-null [:field-id 7]]]}}]]
+ (is (= (mbql-query
+ {:aggregation [[:share [:and
+ [:= [:field-id 5] "abc"]
+ [:or
+ [:is-null [:field-id 7]]
+ [:> [:field-id 4] 1]]]]]})
+ (#'expand-macros/expand-metrics-and-segments
+ (mbql-query
+ {:aggregation [[:share [:and
+ [:segment segment-1-id]
+ [:or
+ [:segment segment-2-id]
+ [:> [:field-id 4] 1]]]]]})))))))
diff --git a/test/metabase/query_processor_test/count_where_test.clj b/test/metabase/query_processor_test/count_where_test.clj
index a908eb2d6476b3bcdd8e6b13c40d505d024aba1e..1f4334a0229a282a9ffd7a094c609f8bf3ba0afd 100644
--- a/test/metabase/query_processor_test/count_where_test.clj
+++ b/test/metabase/query_processor_test/count_where_test.clj
@@ -1,88 +1,97 @@
(ns metabase.query-processor-test.count-where-test
- (:require [metabase.models.metric :refer [Metric]]
+ (:require [clojure.test :refer :all]
+ [metabase.models.metric :refer [Metric]]
[metabase.models.segment :refer [Segment]]
- [metabase.query-processor-test :refer :all]
- [metabase.test :as mt]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]
- [metabase.test.util :as tu]
- [toucan.util.test :as tt]))
+ [metabase.test :as mt]))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 94
- (->> {:aggregation [[:count-where [:< [:field-id (data/id :venues :price)] 4]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long))
+(deftest basic-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= 94
+ (->> {:aggregation [[:count-where [:< [:field-id (mt/id :venues :price)] 4]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long)))
+ (testing "normalization"
+ (is (= 94
+ (->> {:aggregation [["count-where" ["<" ["field-id" (mt/id :venues :price)] 4]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long))))))
-;; Test normalization
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 94
- (->> {:aggregation [["count-where" ["<" ["field-id" (data/id :venues :price)] 4]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long))
+(deftest compound-condition-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= 17
+ (->> {:aggregation [[:count-where
+ [:and
+ [:< [:field-id (mt/id :venues :price)] 4]
+ [:or
+ [:starts-with [:field-id (mt/id :venues :name)] "M"]
+ [:ends-with [:field-id (mt/id :venues :name)] "t"]]]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 17
- (->> {:aggregation [[:count-where [:and [:< [:field-id (data/id :venues :price)] 4]
- [:or [:starts-with [:field-id (data/id :venues :name)] "M"]
- [:ends-with [:field-id (data/id :venues :name)] "t"]]]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long))
+(deftest filter-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= nil
+ (->> {:aggregation [[:count-where [:< [:field-id (mt/id :venues :price)] 4]]]
+ :filter [:> [:field-id (mt/id :venues :price)] Long/MAX_VALUE]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- nil
- (->> {:aggregation [[:count-where [:< [:field-id (data/id :venues :price)] 4]]]
- :filter [:> [:field-id (data/id :venues :price)] Long/MAX_VALUE]}
- (data/run-mbql-query venues)
- rows
- ffirst))
+(deftest breakout-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= [[2 0]
+ [3 0]
+ [4 1]
+ [5 1]]
+ (->> {:aggregation [[:count-where [:< [:field-id (mt/id :venues :price)] 2]]]
+ :breakout [[:field-id (mt/id :venues :category_id)]]
+ :limit 4}
+ (mt/run-mbql-query venues)
+ (mt/round-all-decimals 2)
+ mt/rows
+ (map (fn [[k v]]
+ [(long k) (long v)])))))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- [[2 0]
- [3 0]
- [4 1]
- [5 1]]
- (->> {:aggregation [[:count-where [:< [:field-id (data/id :venues :price)] 2]]]
- :breakout [[:field-id (data/id :venues :category_id)]]
- :limit 4}
- (data/run-mbql-query venues)
- (tu/round-all-decimals 2)
- rows
- (map (fn [[k v]]
- [(long k) (long v)]))))
+(deftest count-where-inside-expression-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations :expressions)
+ (is (= 48
+ (->> {:aggregation [[:+
+ [:/
+ [:count-where [:< [:field-id (mt/id :venues :price)] 4]]
+ 2]
+ 1]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations :expressions)
- 48
- (->> {:aggregation [[:+ [:/ [:count-where [:< [:field-id (data/id :venues :price)] 4]] 2] 1]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long))
+(deftest segment-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (mt/with-temp Segment [{segment-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :filter [:< [:field-id (mt/id :venues :price)] 4]}}]
+ (is (= 94
+ (->> {:aggregation [[:count-where [:segment segment-id]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long))))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 94
- (tt/with-temp* [Segment [{segment-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :filter [:< [:field-id (data/id :venues :price)] 4]}}]]
- (->> {:aggregation [[:count-where [:segment segment-id]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long)))
-
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 94
- (tt/with-temp* [Metric [{metric-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :aggregation [:count-where [:< [:field-id (data/id :venues :price)] 4]]}}]]
- (->> {:aggregation [[:metric metric-id]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- long)))
+(deftest metric-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (mt/with-temp Metric [{metric-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :aggregation [:count-where
+ [:< [:field-id (mt/id :venues :price)] 4]]}}]
+ (is (= 94
+ (->> {:aggregation [[:metric metric-id]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ long))))))
diff --git a/test/metabase/query_processor_test/expression_aggregations_test.clj b/test/metabase/query_processor_test/expression_aggregations_test.clj
index 81740c8416fbc6c9a1dec69bbbd3883c40549d18..58603fc1c9c39b8d853b2b7c7995565f3e48ffb0 100644
--- a/test/metabase/query_processor_test/expression_aggregations_test.clj
+++ b/test/metabase/query_processor_test/expression_aggregations_test.clj
@@ -1,281 +1,276 @@
(ns metabase.query-processor-test.expression-aggregations-test
"Tests for expression aggregations and for named aggregations."
- (:require [metabase.driver :as driver]
+ (:require [clojure.test :refer :all]
+ [metabase.driver :as driver]
[metabase.models.metric :refer [Metric]]
[metabase.query-processor-test :as qp.test]
[metabase.test :as mt]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]
- [metabase.util :as u]
- [toucan.util.test :as tt]))
+ [metabase.util :as u]))
-;; sum, *
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 1211]
- [2 5710]
- [3 1845]
- [4 1476]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:sum [:* $id $price]]]
- :breakout [$price]}))))
+(deftest sum-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "sum, *"
+ (is (= [[1 1211]
+ [2 5710]
+ [3 1845]
+ [4 1476]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:sum [:* $id $price]]]
+ :breakout [$price]})))))))
-;; min, +
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 10]
- [2 4]
- [3 4]
- [4 20]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:min [:+ $id $price]]]
- :breakout [$price]}))))
+(deftest min-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "min, +"
+ (is (= [[1 10]
+ [2 4]
+ [3 4]
+ [4 20]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:min [:+ $id $price]]]
+ :breakout [$price]})))))))
-;; max, /
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 94]
- [2 50]
- [3 26]
- [4 20]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:max [:/ $id $price]]]
- :breakout [$price]}))))
+(deftest max-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "max, /"
+ (is (= [[1 94]
+ [2 50]
+ [3 26]
+ [4 20]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:max [:/ $id $price]]]
+ :breakout [$price]})))))))
-;; avg, -
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- (if (= driver/*driver* :h2)
- [[1 55]
- [2 97]
- [3 142]
- [4 246]]
- [[1 55]
- [2 96]
- [3 141]
- [4 246]])
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:avg [:* $id $price]]]
- :breakout [$price]}))))
+(deftest avg-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "avg, -"
+ (is (= (if (= driver/*driver* :h2)
+ [[1 55]
+ [2 97]
+ [3 142]
+ [4 246]]
+ [[1 55]
+ [2 96]
+ [3 141]
+ [4 246]])
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:avg [:* $id $price]]]
+ :breakout [$price]})))))))
-;; post-aggregation math w/ 2 args: count + sum
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 44]
- [2 177]
- [3 52]
- [4 30]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+
- [:count $id]
- [:sum $price]]]
- :breakout [$price]}))))
+(deftest post-aggregation-math-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "post-aggregation math"
+ (testing "w/ 2 args: count + sum"
+ (is (= [[1 44]
+ [2 177]
+ [3 52]
+ [4 30]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+
+ [:count $id]
+ [:sum $price]]]
+ :breakout [$price]})))))
-;; post-aggregation math w/ 3 args: count + sum + count
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 66]
- [2 236]
- [3 65]
- [4 36]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+ [:count $id] [:sum $price] [:count $price]]]
- :breakout [$price]}))))
+ (testing "w/ 3 args: count + sum + count"
+ (is (= [[1 66]
+ [2 236]
+ [3 65]
+ [4 36]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+ [:count $id] [:sum $price] [:count $price]]]
+ :breakout [$price]})))))
-;; post-aggregation math w/ a constant: count * 10
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 220]
- [2 590]
- [3 130]
- [4 60]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:* [:count $id] 10]]
- :breakout [$price]}))))
+ (testing "w/ a constant: count * 10"
+ (is (= [[1 220]
+ [2 590]
+ [3 130]
+ [4 60]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:* [:count $id] 10]]
+ :breakout [$price]})))))
-;; nested post-aggregation math: count + (count * sum)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 506]
- [2 7021]
- [3 520]
- [4 150]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+
- [:count $id]
- [:* [:count $id] [:sum $price]]]]
- :breakout [$price]}))))
+ (testing "w/ avg: count + avg"
+ (is (= (if (= driver/*driver* :h2)
+ [[1 77]
+ [2 107]
+ [3 60]
+ [4 68]]
+ [[1 77]
+ [2 107]
+ [3 60]
+ [4 67]])
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+ [:count $id] [:avg $id]]]
+ :breakout [$price]}))))))))
-;; post-aggregation math w/ avg: count + avg
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- (if (= driver/*driver* :h2)
- [[1 77]
- [2 107]
- [3 60]
- [4 68]]
- [[1 77]
- [2 107]
- [3 60]
- [4 67]])
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+ [:count $id] [:avg $id]]]
- :breakout [$price]}))))
+(deftest nested-post-aggregation-mat-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "nested post-aggregation math: count + (count * sum)"
+ (is (= [[1 506]
+ [2 7021]
+ [3 520]
+ [4 150]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+
+ [:count $id]
+ [:* [:count $id] [:sum $price]]]]
+ :breakout [$price]})))))))
-;; post aggregation math + math inside aggregations: max(venue_price) + min(venue_price - id)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 -92]
- [2 -96]
- [3 -74]
- [4 -73]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+ [:max $price] [:min [:- $price $id]]]]
- :breakout [$price]}))))
+(deftest math-inside-aggregations-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "post aggregation math + math inside aggregations: max(venue_price) + min(venue_price - id)"
+ (is (= [[1 -92]
+ [2 -96]
+ [3 -74]
+ [4 -73]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+ [:max $price] [:min [:- $price $id]]]]
+ :breakout [$price]})))))))
-;; aggregation w/o field
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 23]
- [2 60]
- [3 14]
- [4 7]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:+ 1 [:count]]]
- :breakout [$price]}))))
+(deftest aggregation-without-field-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "aggregation w/o field"
+ (is (= [[1 23]
+ [2 60]
+ [3 14]
+ [4 7]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:+ 1 [:count]]]
+ :breakout [$price]})))))))
-;; Sorting by an un-named aggregate expression
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 2] [2 2] [12 2] [4 4] [7 4] [10 4] [11 4] [8 8]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query users
- {:aggregation [[:* [:count] 2]]
- :breakout [[:datetime-field $last_login :month-of-year]]
- :order-by [[:asc [:aggregation 0]]]}))))
+(deftest sort-by-unnamed-aggregate-expression-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "Sorting by an un-named aggregate expression"
+ (is (= [[1 2] [2 2] [12 2] [4 4] [7 4] [10 4] [11 4] [8 8]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query users
+ {:aggregation [[:* [:count] 2]]
+ :breakout [[:datetime-field $last_login :month-of-year]]
+ :order-by [[:asc [:aggregation 0]]]})))))))
-;; aggregation with math inside the aggregation :scream_cat:
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1 44]
- [2 177]
- [3 52]
- [4 30]]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:sum [:+ $price 1]]]
- :breakout [$price]}))))
+(deftest math-inside-the-aggregation-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "aggregation with math inside the aggregation :scream_cat:"
+ (is (= [[1 44]
+ [2 177]
+ [3 52]
+ [4 30]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [[:sum [:+ $price 1]]]
+ :breakout [$price]})))))))
-;; check that we can name an expression aggregation w/ aggregation at top-level
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- {:rows [[1 44]
- [2 177]
- [3 52]
- [4 30]]
- :columns [(data/format-name "price")
- "sum_of_price"]}
- (qp.test/format-rows-by [int int]
- (qp.test/rows+column-names
- (data/run-mbql-query venues
- {:aggregation [[:aggregation-options [:sum [:+ $price 1]] {:name "sum_of_price"}]]
- :breakout [$price]}))))
+(deftest named-expression-aggregation-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "check that we can name an expression aggregation w/ aggregation at top-level"
+ (is (= {:rows [[1 44]
+ [2 177]
+ [3 52]
+ [4 30]]
+ :columns [(mt/format-name "price")
+ "sum_of_price"]}
+ (mt/format-rows-by [int int]
+ (mt/rows+column-names
+ (mt/run-mbql-query venues
+ {:aggregation [[:aggregation-options [:sum [:+ $price 1]] {:name "sum_of_price"}]]
+ :breakout [$price]}))))))
-;; check that we can name an expression aggregation w/ expression at top-level
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- {:rows [[1 -19]
- [2 77]
- [3 -2]
- [4 -17]]
- :columns [(data/format-name "price")
- "sum_41"]}
- (qp.test/format-rows-by [int int]
- (qp.test/rows+column-names
- (data/run-mbql-query venues
- {:aggregation [[:aggregation-options [:- [:sum $price] 41] {:name "sum_41"}]]
- :breakout [$price]}))))
+ (testing "check that we can name an expression aggregation w/ expression at top-level"
+ (is (= {:rows [[1 -19]
+ [2 77]
+ [3 -2]
+ [4 -17]]
+ :columns [(mt/format-name "price")
+ "sum_41"]}
+ (mt/format-rows-by [int int]
+ (mt/rows+column-names
+ (mt/run-mbql-query venues
+ {:aggregation [[:aggregation-options [:- [:sum $price] 41] {:name "sum_41"}]]
+ :breakout [$price]}))))))))
-;; check that we can handle Metrics inside expression aggregation clauses
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[2 119]
- [3 40]
- [4 25]]
- (tt/with-temp Metric [metric {:table_id (data/id :venues)
- :definition {:aggregation [:sum [:field-id (data/id :venues :price)]]
- :filter [:> [:field-id (data/id :venues :price)] 1]}}]
- (qp.test/format-rows-by [int int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [:+ [:metric (u/get-id metric)] 1]
- :breakout [[:field-id $price]]})))))
+(deftest metrics-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "check that we can handle Metrics inside expression aggregation clauses"
+ (mt/with-temp Metric [metric {:table_id (mt/id :venues)
+ :definition {:aggregation [:sum [:field-id (mt/id :venues :price)]]
+ :filter [:> [:field-id (mt/id :venues :price)] 1]}}]
+ (is (= [[2 119]
+ [3 40]
+ [4 25]]
+ (mt/formatted-rows [int int]
+ (mt/run-mbql-query venues
+ {:aggregation [:+ [:metric (u/the-id metric)] 1]
+ :breakout [[:field-id $price]]}))))))
-;; check that we can handle Metrics inside an `:aggregation-options` clause
-;; TODO - Pretty sure this test doesn't belong here (?)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- {:rows [[2 118]
- [3 39]
- [4 24]]
- :columns [(data/format-name "price")
- "auto_generated_name"]}
- (tt/with-temp Metric [metric {:table_id (data/id :venues)
- :definition {:aggregation [:sum [:field-id (data/id :venues :price)]]
- :filter [:> [:field-id (data/id :venues :price)] 1]}}]
- (qp.test/format-rows-by [int int]
- (qp.test/rows+column-names
- (data/run-mbql-query venues
- {:aggregation [[:aggregation-options [:metric (u/get-id metric)] {:name "auto_generated_name"}]]
- :breakout [[:field-id $price]]})))))
+ (testing "check that we can handle Metrics inside an `:aggregation-options` clause"
+ (mt/with-temp Metric [metric {:table_id (mt/id :venues)
+ :definition {:aggregation [:sum [:field-id (mt/id :venues :price)]]
+ :filter [:> [:field-id (mt/id :venues :price)] 1]}}]
+ (is (= {:rows [[2 118]
+ [3 39]
+ [4 24]]
+ :columns [(mt/format-name "price")
+ "auto_generated_name"]}
+ (mt/format-rows-by [int int]
+ (mt/rows+column-names
+ (mt/run-mbql-query venues
+ {:aggregation [[:aggregation-options [:metric (u/the-id metric)] {:name "auto_generated_name"}]]
+ :breakout [[:field-id $price]]})))))))
-;; check that Metrics with a nested aggregation still work inside an `:aggregation-options` clause
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- {:rows [[2 118]
- [3 39]
- [4 24]]
- :columns [(data/format-name "price")
- "auto_generated_name"]}
- (tt/with-temp Metric [metric (data/$ids venues
- {:table_id $$venues
- :definition {:aggregation [[:sum $price]]
- :filter [:> $price 1]}})]
- (qp.test/format-rows-by [int int]
- (qp.test/rows+column-names
- (data/run-mbql-query venues
- {:aggregation [[:aggregation-options [:metric (u/get-id metric)] {:name "auto_generated_name"}]]
- :breakout [[:field-id $price]]})))))
+ (testing "check that Metrics with a nested aggregation still work inside an `:aggregation-options` clause"
+ (mt/with-temp Metric [metric (mt/$ids venues
+ {:table_id $$venues
+ :definition {:aggregation [[:sum $price]]
+ :filter [:> $price 1]}})]
+ (is (= {:rows [[2 118]
+ [3 39]
+ [4 24]]
+ :columns [(mt/format-name "price")
+ "auto_generated_name"]}
+ (mt/format-rows-by [int int]
+ (mt/rows+column-names
+ (mt/run-mbql-query venues
+ {:aggregation [[:aggregation-options [:metric (u/the-id metric)] {:name "auto_generated_name"}]]
+ :breakout [[:field-id $price]]})))))))))
-;; check that named aggregations come back with the correct column metadata (#4002)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- (assoc (qp.test/aggregate-col :count)
- :name "auto_generated_name"
- :display_name "Count of Things")
- (-> (data/run-mbql-query venues
- {:aggregation [[:aggregation-options ["COUNT"] {:name "auto_generated_name", :display-name "Count of Things"}]]})
- qp.test/cols
- first))
+(deftest named-aggregations-metadata-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "check that named aggregations come back with the correct column metadata (#4002)"
+ (is (= (assoc (qp.test/aggregate-col :count)
+ :name "auto_generated_name"
+ :display_name "Count of Things")
+ (-> (mt/run-mbql-query venues
+ {:aggregation [[:aggregation-options ["COUNT"]
+ {:name "auto_generated_name"
+ :display-name "Count of Things"}]]})
+ mt/cols
+ first))))))
-;; check that we can use cumlative count in expression aggregations
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[1000]]
- (qp.test/format-rows-by [int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [["*" ["cum_count"] 10]]}))))
+(deftest cumulative-count-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "check that we can use cumlative count in expression aggregations"
+ (is (= [[1000]]
+ (mt/format-rows-by [int]
+ (mt/rows
+ (mt/run-mbql-query venues
+ {:aggregation [["*" ["cum_count"] 10]]}))))))))
-;; can we use named expressions inside expression aggregations?
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :expression-aggregations)
- [[406]]
- (qp.test/format-rows-by [int]
- (qp.test/rows
- (data/run-mbql-query venues
- {:aggregation [[:sum [:expression "double-price"]]]
- :expressions {"double-price" [:* $price 2]}}))))
+(deftest named-expressions-inside-expression-aggregations-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (testing "can we use named expressions inside expression aggregations?"
+ (is (= [[406]]
+ (mt/format-rows-by [int]
+ (mt/rows
+ (mt/run-mbql-query venues
+ {:aggregation [[:sum [:expression "double-price"]]]
+ :expressions {"double-price" [:* $price 2]}}))))))))
diff --git a/test/metabase/query_processor_test/implicit_joins_test.clj b/test/metabase/query_processor_test/implicit_joins_test.clj
index a5edbc997b767d1f68ace58202e6bf040194217a..b7e8bd57821f0a4f2e371f25aefdf2eeae18a04f 100644
--- a/test/metabase/query_processor_test/implicit_joins_test.clj
+++ b/test/metabase/query_processor_test/implicit_joins_test.clj
@@ -1,151 +1,139 @@
(ns metabase.query-processor-test.implicit-joins-test
- "Test for JOIN behavior."
+ "Tests for joins that are created automatically when an `:fk->` column is present."
(:require [clojure.test :refer :all]
- [expectations :refer [expect]]
[metabase.driver :as driver]
- [metabase.query-processor-test :as qp.test]
- [metabase.test :as mt]
- [metabase.test.data.datasets :as datasets]))
+ [metabase.test :as mt]))
-;; The top 10 cities by number of Tupac sightings
-;; Test that we can breakout on an FK field (Note how the FK Field is returned in the results)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :foreign-keys)
- [["Arlington" 16]
- ["Albany" 15]
- ["Portland" 14]
- ["Louisville" 13]
- ["Philadelphia" 13]
- ["Anchorage" 12]
- ["Lincoln" 12]
- ["Houston" 11]
- ["Irvine" 11]
- ["Lakeland" 11]]
- (->> (mt/dataset tupac-sightings
- (mt/run-mbql-query sightings
- {:aggregation [[:count]]
- :breakout [$city_id->cities.name]
- :order-by [[:desc [:aggregation 0]]]
- :limit 10}))
- qp.test/rows
- (qp.test/format-rows-by [str int])))
-
-
-;; Number of Tupac sightings in the Expa office
-;; (he was spotted here 60 times)
-;; Test that we can filter on an FK field
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :foreign-keys)
- [[60]]
- (->> (mt/dataset tupac-sightings
- (mt/run-mbql-query sightings
- {:aggregation [[:count]]
- :filter [:= $category_id->categories.id 8]}))
- qp.test/rows
- (qp.test/format-rows-by [int])))
-
-
-;; THE 10 MOST RECENT TUPAC SIGHTINGS (!)
-;; (What he was doing when we saw him, sighting ID)
-;; Check that we can include an FK field in the :fields clause
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :foreign-keys)
- [[772 "In the Park"]
- [894 "Working at a Pet Store"]
- [684 "At the Airport"]
- [199 "At a Restaurant"]
- [33 "Working as a Limo Driver"]
- [902 "At Starbucks"]
- [927 "On TV"]
- [996 "At a Restaurant"]
- [897 "Wearing a Biggie Shirt"]
- [499 "In the Expa Office"]]
- (->> (mt/dataset tupac-sightings
- (mt/run-mbql-query sightings
- {:fields [$id $category_id->categories.name]
- :order-by [[:desc $timestamp]]
- :limit 10}))
- qp.test/rows
- (qp.test/format-rows-by [int str])))
+(deftest breakout-on-fk-field-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys)
+ (testing "Test that we can breakout on an FK field (Note how the FK Field is returned in the results)"
+ (mt/dataset tupac-sightings
+ ;; The top 10 cities by number of Tupac sightings
+ (is (= [["Arlington" 16]
+ ["Albany" 15]
+ ["Portland" 14]
+ ["Louisville" 13]
+ ["Philadelphia" 13]
+ ["Anchorage" 12]
+ ["Lincoln" 12]
+ ["Houston" 11]
+ ["Irvine" 11]
+ ["Lakeland" 11]]
+ (mt/formatted-rows [str int]
+ (mt/run-mbql-query sightings
+ {:aggregation [[:count]]
+ :breakout [$city_id->cities.name]
+ :order-by [[:desc [:aggregation 0]]]
+ :limit 10}))))))))
+(deftest filter-by-fk-field-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys)
+ (testing "Test that we can filter on an FK field"
+ (mt/dataset tupac-sightings
+ ;; Number of Tupac sightings in the Expa office (he was spotted here 60 times)
+ (is (= [[60]]
+ (mt/formatted-rows [int]
+ (mt/run-mbql-query sightings
+ {:aggregation [[:count]]
+ :filter [:= $category_id->categories.id 8]}))))))))
-;; 1. Check that we can order by Foreign Keys
-;; (this query targets sightings and orders by cities.name and categories.name)
-;; 2. Check that we can join MULTIPLE tables in a single query
-;; (this query joins both cities and categories)
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :foreign-keys)
- ;; ID, CITY_ID, CATEGORY_ID
- ;; Cities are already alphabetized in the source data which is why CITY_ID is sorted
- [[6 1 12]
- [355 1 11]
- [596 1 11]
- [379 1 13]
- [413 1 5]
- [426 1 1]
- [67 2 11]
- [524 2 11]
- [77 2 13]
- [202 2 13]]
- (->> (mt/dataset tupac-sightings
- (mt/run-mbql-query sightings
- {:order-by [[:asc $city_id->cities.name]
- [:desc $category_id->categories.name]
- [:asc $id]]
- :limit 10}))
- ;; drop timestamps.
- qp.test/rows
- (map butlast)
- (qp.test/format-rows-by [int int int])))
+(deftest fk-field-in-fields-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys)
+ (testing "Check that we can include an FK field in `:fields`"
+ (mt/dataset tupac-sightings
+ ;; THE 10 MOST RECENT TUPAC SIGHTINGS (!) (What he was doing when we saw him, sighting ID)
+ (is (= [[772 "In the Park"]
+ [894 "Working at a Pet Store"]
+ [684 "At the Airport"]
+ [199 "At a Restaurant"]
+ [33 "Working as a Limo Driver"]
+ [902 "At Starbucks"]
+ [927 "On TV"]
+ [996 "At a Restaurant"]
+ [897 "Wearing a Biggie Shirt"]
+ [499 "In the Expa Office"]]
+ (mt/formatted-rows [int str]
+ (mt/run-mbql-query sightings
+ {:fields [$id $category_id->categories.name]
+ :order-by [[:desc $timestamp]]
+ :limit 10}))))))))
+(deftest join-multiple-tables-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys)
+ (testing (str "1. Check that we can order by Foreign Keys (this query targets sightings and orders by cities.name "
+ "and categories.name)"
+ "\n"
+ "2. Check that we can join MULTIPLE tables in a single query (this query joins both cities and "
+ "categories)")
+ (mt/dataset tupac-sightings
+ ;; ID, CITY_ID, CATEGORY_ID
+ ;; Cities are already alphabetized in the source data which is why CITY_ID is sorted
+ (is (= [[6 1 12]
+ [355 1 11]
+ [596 1 11]
+ [379 1 13]
+ [413 1 5]
+ [426 1 1]
+ [67 2 11]
+ [524 2 11]
+ [77 2 13]
+ [202 2 13]]
+ (->> (mt/run-mbql-query sightings
+ {:order-by [[:asc $city_id->cities.name]
+ [:desc $category_id->categories.name]
+ [:asc $id]]
+ :limit 10})
+ ;; drop timestamps.
+ mt/rows
+ (map butlast)
+ (mt/format-rows-by [int int int]))))))))
-;; Check that trying to use a Foreign Key fails for Mongo and other DBs
(deftest feature-check-test
(mt/test-drivers (mt/normal-drivers-without-feature :foreign-keys)
- (is
- (thrown-with-msg?
- clojure.lang.ExceptionInfo
- (re-pattern (format "%s driver does not support foreign keys" driver/*driver*))
- (mt/dataset tupac-sightings
- (mt/run-mbql-query sightings
- {:order-by [[:asc $city_id->cities.name]
- [:desc $category_id->categories.name]
- [:asc $id]]
- :limit 10}))))))
-
-;; Implicit joins should come back with `:fk->` field refs
-(expect
- (mt/$ids venues $category_id->categories.name)
- (-> (mt/cols
- (mt/run-mbql-query :venues
- {:fields [$category_id->categories.name]
- :order-by [[:asc $id]]
- :limit 1}))
- first
- :field_ref))
-
+ (testing "Check that trying to use a Foreign Key fails for Mongo and other DBs"
+ (is
+ (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ (re-pattern (format "%s driver does not support foreign keys" driver/*driver*))
+ (mt/dataset tupac-sightings
+ (mt/run-mbql-query sightings
+ {:order-by [[:asc $city_id->cities.name]
+ [:desc $category_id->categories.name]
+ [:asc $id]]
+ :limit 10})))))))
-;;; +----------------------------------------------------------------------------------------------------------------+
-;;; | MULTIPLE JOINS |
-;;; +----------------------------------------------------------------------------------------------------------------+
+(deftest field-refs-test
+ (testing "Implicit joins should come back with `:fk->` field refs"
+ (is (= (mt/$ids venues $category_id->categories.name)
+ (-> (mt/cols
+ (mt/run-mbql-query :venues
+ {:fields [$category_id->categories.name]
+ :order-by [[:asc $id]]
+ :limit 1}))
+ first
+ :field_ref)))))
-;;; CAN WE JOIN AGAINST THE SAME TABLE TWICE (MULTIPLE FKS TO A SINGLE TABLE!?)
-;; Query should look something like:
-;; SELECT USERS__via__SENDER_ID.NAME AS NAME, count(*) AS count
-;; FROM PUBLIC.MESSAGES
-;; LEFT JOIN PUBLIC.USERS USERS__via__RECIEVER_ID
-;; ON PUBLIC.MESSAGES.RECIEVER_ID = USERS__via__RECIEVER_ID.ID
-;; LEFT JOIN PUBLIC.USERS USERS__via__SENDER_ID
-;; ON PUBLIC.MESSAGES.SENDER_ID = USERS__via__SENDER_ID.ID
-;; WHERE USERS__via__RECIEVER_ID.NAME = 'Rasta Toucan'
-;; GROUP BY USERS__via__SENDER_ID.NAME
-;; ORDER BY USERS__via__SENDER_ID.NAME ASC
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :foreign-keys)
- [["Bob the Sea Gull" 2]
- ["Brenda Blackbird" 2]
- ["Lucky Pigeon" 2]
- ["Peter Pelican" 5]
- ["Ronald Raven" 1]]
- (mt/dataset avian-singles
- (qp.test/format-rows-by [str int]
- (qp.test/rows
- (mt/run-mbql-query messages
- {:aggregation [[:count]]
- :breakout [$sender_id->users.name]
- :filter [:= $receiver_id->users.name "Rasta Toucan"]})))))
+(deftest multiple-joins-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys)
+ (testing "Can we join against the same table twice (multiple fks to a single table?)"
+ (mt/dataset avian-singles
+ ;; Query should look something like:
+ ;; SELECT USERS__via__SENDER_ID.NAME AS NAME, count(*) AS count
+ ;; FROM PUBLIC.MESSAGES
+ ;; LEFT JOIN PUBLIC.USERS USERS__via__RECIEVER_ID
+ ;; ON PUBLIC.MESSAGES.RECIEVER_ID = USERS__via__RECIEVER_ID.ID
+ ;; LEFT JOIN PUBLIC.USERS USERS__via__SENDER_ID
+ ;; ON PUBLIC.MESSAGES.SENDER_ID = USERS__via__SENDER_ID.ID
+ ;; WHERE USERS__via__RECIEVER_ID.NAME = 'Rasta Toucan'
+ ;; GROUP BY USERS__via__SENDER_ID.NAME
+ ;; ORDER BY USERS__via__SENDER_ID.NAME ASC
+ (is (= [["Bob the Sea Gull" 2]
+ ["Brenda Blackbird" 2]
+ ["Lucky Pigeon" 2]
+ ["Peter Pelican" 5]
+ ["Ronald Raven" 1]]
+ (mt/formatted-rows [str int]
+ (mt/run-mbql-query messages
+ {:aggregation [[:count]]
+ :breakout [$sender_id->users.name]
+ :filter [:= $receiver_id->users.name "Rasta Toucan"]}))))))))
diff --git a/test/metabase/query_processor_test/nested_field_test.clj b/test/metabase/query_processor_test/nested_field_test.clj
index 89e60a1371974cd828a389e6ad39af671fcfd087..04e1f7ae0a600fe38a50fc907705ddb26154a5d2 100644
--- a/test/metabase/query_processor_test/nested_field_test.clj
+++ b/test/metabase/query_processor_test/nested_field_test.clj
@@ -1,143 +1,144 @@
(ns metabase.query-processor-test.nested-field-test
"Tests for nested field access."
- (:require [metabase.query-processor-test :as qp.test]
- [metabase.test :as mt]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]))
+ (:require [clojure.test :refer :all]
+ [metabase.test :as mt]))
-;;; Nested Field in FILTER
-;; Get the first 10 tips where tip.venue.name == "Kyle's Low-Carb Grill"
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [[8 "Kyle's Low-Carb Grill"]
- [67 "Kyle's Low-Carb Grill"]
- [80 "Kyle's Low-Carb Grill"]
- [83 "Kyle's Low-Carb Grill"]
- [295 "Kyle's Low-Carb Grill"]
- [342 "Kyle's Low-Carb Grill"]
- [417 "Kyle's Low-Carb Grill"]
- [426 "Kyle's Low-Carb Grill"]
- [470 "Kyle's Low-Carb Grill"]]
- (mapv
- (fn [[id _ _ _ {venue-name :name}]] [id venue-name])
- (qp.test/rows
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:filter [:= $tips.venue.name "Kyle's Low-Carb Grill"]
- :order-by [[:asc $id]]
- :limit 10})))))
+(deftest filter-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field in FILTER"
+ (mt/dataset geographical-tips
+ ;; Get the first 10 tips where tip.venue.name == "Kyle's Low-Carb Grill"
+ (is (= [[8 "Kyle's Low-Carb Grill"]
+ [67 "Kyle's Low-Carb Grill"]
+ [80 "Kyle's Low-Carb Grill"]
+ [83 "Kyle's Low-Carb Grill"]
+ [295 "Kyle's Low-Carb Grill"]
+ [342 "Kyle's Low-Carb Grill"]
+ [417 "Kyle's Low-Carb Grill"]
+ [426 "Kyle's Low-Carb Grill"]
+ [470 "Kyle's Low-Carb Grill"]]
+ (mapv
+ (fn [[id _ _ _ {venue-name :name}]] [id venue-name])
+ (mt/rows
+ (mt/run-mbql-query tips
+ {:filter [:= $tips.venue.name "Kyle's Low-Carb Grill"]
+ :order-by [[:asc $id]]
+ :limit 10})))))))))
-;;; Nested Field in ORDER
-;; Let's get all the tips Kyle posted on Twitter sorted by tip.venue.name
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [[446
- {:mentions ["@cams_mexican_gastro_pub"], :tags ["#mexican" "#gastro" "#pub"], :service "twitter", :username "kyle"}
- "Cam's Mexican Gastro Pub is a historical and underappreciated place to conduct a business meeting with friends."
- {:large "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/large.jpg",
- :medium "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/med.jpg",
- :small "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/small.jpg"}
- {:phone "415-320-9123", :name "Cam's Mexican Gastro Pub", :categories ["Mexican" "Gastro Pub"], :id "bb958ac5-758e-4f42-b984-6b0e13f25194"}]
- [230
- {:mentions ["@haight_european_grill"], :tags ["#european" "#grill"], :service "twitter", :username "kyle"}
- "Haight European Grill is a horrible and amazing place to have a birthday party during winter."
- {:large "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/large.jpg",
- :medium "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/med.jpg",
- :small "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/small.jpg"}
- {:phone "415-191-2778", :name "Haight European Grill", :categories ["European" "Grill"], :id "7e6281f7-5b17-4056-ada0-85453247bc8f"}]
- [319
- {:mentions ["@haight_soul_food_pop_up_food_stand"], :tags ["#soul" "#food" "#pop-up" "#food" "#stand"], :service "twitter", :username "kyle"}
- "Haight Soul Food Pop-Up Food Stand is a underground and modern place to have breakfast on a Tuesday afternoon."
- {:large "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/large.jpg",
- :medium "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/med.jpg",
- :small "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/small.jpg"}
- {:phone "415-741-8726", :name "Haight Soul Food Pop-Up Food Stand", :categories ["Soul Food" "Pop-Up Food Stand"], :id "9735184b-1299-410f-a98e-10d9c548af42"}]
- [224
- {:mentions ["@pacific_heights_free_range_eatery"], :tags ["#free-range" "#eatery"], :service "twitter", :username "kyle"}
- "Pacific Heights Free-Range Eatery is a wonderful and modern place to take visiting friends and relatives Friday nights."
- {:large "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/large.jpg",
- :medium "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/med.jpg",
- :small "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/small.jpg"}
- {:phone "415-901-6541", :name "Pacific Heights Free-Range Eatery", :categories ["Free-Range" "Eatery"], :id "88b361c8-ce69-4b2e-b0f2-9deedd574af6"}]]
- (qp.test/rows
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:filter [:and
- [:= $tips.source.service "twitter"]
- [:= $tips.source.username "kyle"]]
- :order-by [[:asc $tips.venue.name]]}))))
+(deftest order-by-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field in ORDER-BY"
+ (mt/dataset geographical-tips
+ ;; Let's get all the tips Kyle posted on Twitter sorted by tip.venue.name
+ (is (= [[446
+ {:mentions ["@cams_mexican_gastro_pub"], :tags ["#mexican" "#gastro" "#pub"], :service "twitter", :username "kyle"}
+ "Cam's Mexican Gastro Pub is a historical and underappreciated place to conduct a business meeting with friends."
+ {:large "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/large.jpg",
+ :medium "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/med.jpg",
+ :small "http://cloudfront.net/6e3a5256-275f-4056-b61a-25990b4bb484/small.jpg"}
+ {:phone "415-320-9123", :name "Cam's Mexican Gastro Pub", :categories ["Mexican" "Gastro Pub"], :id "bb958ac5-758e-4f42-b984-6b0e13f25194"}]
+ [230
+ {:mentions ["@haight_european_grill"], :tags ["#european" "#grill"], :service "twitter", :username "kyle"}
+ "Haight European Grill is a horrible and amazing place to have a birthday party during winter."
+ {:large "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/large.jpg",
+ :medium "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/med.jpg",
+ :small "http://cloudfront.net/1dcef7de-a1c4-405b-a9e1-69c92d686ef1/small.jpg"}
+ {:phone "415-191-2778", :name "Haight European Grill", :categories ["European" "Grill"], :id "7e6281f7-5b17-4056-ada0-85453247bc8f"}]
+ [319
+ {:mentions ["@haight_soul_food_pop_up_food_stand"], :tags ["#soul" "#food" "#pop-up" "#food" "#stand"], :service "twitter", :username "kyle"}
+ "Haight Soul Food Pop-Up Food Stand is a underground and modern place to have breakfast on a Tuesday afternoon."
+ {:large "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/large.jpg",
+ :medium "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/med.jpg",
+ :small "http://cloudfront.net/8f613909-550f-4d79-96f6-dc498ff65d1b/small.jpg"}
+ {:phone "415-741-8726", :name "Haight Soul Food Pop-Up Food Stand", :categories ["Soul Food" "Pop-Up Food Stand"], :id "9735184b-1299-410f-a98e-10d9c548af42"}]
+ [224
+ {:mentions ["@pacific_heights_free_range_eatery"], :tags ["#free-range" "#eatery"], :service "twitter", :username "kyle"}
+ "Pacific Heights Free-Range Eatery is a wonderful and modern place to take visiting friends and relatives Friday nights."
+ {:large "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/large.jpg",
+ :medium "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/med.jpg",
+ :small "http://cloudfront.net/cedd4221-dbdb-46c3-95a9-935cce6b3fe5/small.jpg"}
+ {:phone "415-901-6541", :name "Pacific Heights Free-Range Eatery", :categories ["Free-Range" "Eatery"], :id "88b361c8-ce69-4b2e-b0f2-9deedd574af6"}]]
+ (mt/rows
+ (mt/run-mbql-query tips
+ {:filter [:and
+ [:= $tips.source.service "twitter"]
+ [:= $tips.source.username "kyle"]]
+ :order-by [[:asc $tips.venue.name]]}))))))))
-;; Nested Field in AGGREGATION
-;; Let's see how many *distinct* venue names are mentioned
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [99]
- (qp.test/first-row
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:aggregation [[:distinct $tips.venue.name]]}))))
+(deftest aggregation-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field in AGGREGATION"
+ (mt/dataset geographical-tips
+ (testing ":distinct aggregation"
+ ;; Let's see how many *distinct* venue names are mentioned
+ (is (= [99]
+ (mt/first-row
+ (mt/run-mbql-query tips
+ {:aggregation [[:distinct $tips.venue.name]]})))))
-;; Now let's just get the regular count
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [500]
- (qp.test/first-row
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:aggregation [[:count $tips.venue.name]]}))))
+ (testing ":count aggregation"
+ ;; Now let's just get the regular count
+ (is (= [500]
+ (mt/first-row
+ (mt/run-mbql-query tips
+ {:aggregation [[:count $tips.venue.name]]})))))))))
-;;; Nested Field in BREAKOUT
-;; Let's see how many tips we have by source.service
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [["facebook" 107]
- ["flare" 105]
- ["foursquare" 100]
- ["twitter" 98]
- ["yelp" 90]]
- (qp.test/rows
- (qp.test/format-rows-by [str int]
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:aggregation [[:count]]
- :breakout [$tips.source.service]})))))
+(deftest breakout-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field in BREAKOUT"
+ ;; Let's see how many tips we have by source.service
+ (mt/dataset geographical-tips
+ (is (= [["facebook" 107]
+ ["flare" 105]
+ ["foursquare" 100]
+ ["twitter" 98]
+ ["yelp" 90]]
+ (mt/formatted-rows [str int]
+ (mt/run-mbql-query tips
+ {:aggregation [[:count]]
+ :breakout [$tips.source.service]}))))))))
-;;; Nested Field in FIELDS
-;; Return the first 10 tips with just tip.venue.name
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [["Lucky's Gluten-Free Café"]
- ["Joe's Homestyle Eatery"]
- ["Lower Pac Heights Cage-Free Coffee House"]
- ["Oakland European Liquor Store"]
- ["Tenderloin Gormet Restaurant"]
- ["Marina Modern Sushi"]
- ["Sunset Homestyle Grill"]
- ["Kyle's Low-Carb Grill"]
- ["Mission Homestyle Churros"]
- ["Sameer's Pizza Liquor Store"]]
- (qp.test/rows
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:fields [$tips.venue.name]
- :order-by [[:asc $id]]
- :limit 10}))))
+(deftest fields-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field in FIELDS"
+ (mt/dataset geographical-tips
+ ;; Return the first 10 tips with just tip.venue.name
+ (is (= [["Lucky's Gluten-Free Café"]
+ ["Joe's Homestyle Eatery"]
+ ["Lower Pac Heights Cage-Free Coffee House"]
+ ["Oakland European Liquor Store"]
+ ["Tenderloin Gormet Restaurant"]
+ ["Marina Modern Sushi"]
+ ["Sunset Homestyle Grill"]
+ ["Kyle's Low-Carb Grill"]
+ ["Mission Homestyle Churros"]
+ ["Sameer's Pizza Liquor Store"]]
+ (mt/rows
+ (mt/run-mbql-query tips
+ {:fields [$tips.venue.name]
+ :order-by [[:asc $id]]
+ :limit 10}))))))))
-
-;;; Nested Field w/ ordering by aggregation
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :nested-fields)
- [["jane" 4]
- ["kyle" 5]
- ["tupac" 5]
- ["jessica" 6]
- ["bob" 7]
- ["lucky_pigeon" 7]
- ["joe" 8]
- ["mandy" 8]
- ["amy" 9]
- ["biggie" 9]
- ["sameer" 9]
- ["cam_saul" 10]
- ["rasta_toucan" 13]
- [nil 400]]
- (qp.test/format-rows-by [identity int]
- (qp.test/rows
- (data/dataset geographical-tips
- (data/run-mbql-query tips
- {:aggregation [[:count]]
- :breakout [$tips.source.mayor]
- :order-by [[:asc [:aggregation 0]]]})))))
+(deftest order-by-aggregation-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-fields)
+ (testing "Nested Field w/ ordering by aggregation"
+ (mt/dataset geographical-tips
+ (is (= [["jane" 4]
+ ["kyle" 5]
+ ["tupac" 5]
+ ["jessica" 6]
+ ["bob" 7]
+ ["lucky_pigeon" 7]
+ ["joe" 8]
+ ["mandy" 8]
+ ["amy" 9]
+ ["biggie" 9]
+ ["sameer" 9]
+ ["cam_saul" 10]
+ ["rasta_toucan" 13]
+ [nil 400]]
+ (mt/formatted-rows [identity int]
+ (mt/run-mbql-query tips
+ {:aggregation [[:count]]
+ :breakout [$tips.source.mayor]
+ :order-by [[:asc [:aggregation 0]]]}))))))))
diff --git a/test/metabase/query_processor_test/sum_where_test.clj b/test/metabase/query_processor_test/sum_where_test.clj
index 0ce4eb2b21763dee90dbb0e376b5462d955a1f8d..19929b2eef9d2ed06e21e75c414d397d87272487 100644
--- a/test/metabase/query_processor_test/sum_where_test.clj
+++ b/test/metabase/query_processor_test/sum_where_test.clj
@@ -1,90 +1,106 @@
(ns metabase.query-processor-test.sum-where-test
- (:require [metabase.models.metric :refer [Metric]]
+ (:require [clojure.test :refer :all]
+ [metabase.models.metric :refer [Metric]]
[metabase.models.segment :refer [Segment]]
- [metabase.query-processor-test :refer :all]
- [metabase.test :as mt]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]
- [metabase.test.util :as tu]
- [toucan.util.test :as tt]))
+ [metabase.test :as mt]))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 179.0
- (->> {:aggregation [[:sum-where [:field-id (data/id :venues :price)] [:< [:field-id (data/id :venues :price)] 4]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double))
+(deftest basic-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= 179.0
+ (->> {:aggregation [[:sum-where
+ [:field-id (mt/id :venues :price)]
+ [:< [:field-id (mt/id :venues :price)] 4]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double)))
-;; Test normalization
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 179.0
- (->> {:aggregation [["sum-where" ["field-id" (data/id :venues :price)] ["<" ["field-id" (data/id :venues :price)] 4]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double))
+ (testing "Should get normalized correctly and work as expected"
+ (is (= 179.0
+ (->> {:aggregation [["sum-where"
+ ["field-id" (mt/id :venues :price)]
+ ["<" ["field-id" (mt/id :venues :price)] 4]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double))))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 34.0
- (->> {:aggregation [[:sum-where
- [:field-id (data/id :venues :price)]
- [:and [:< [:field-id (data/id :venues :price)] 4]
- [:or [:starts-with [:field-id (data/id :venues :name)] "M"]
- [:ends-with [:field-id (data/id :venues :name)] "t"]]]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double))
+(deftest compound-condition-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= 34.0
+ (->> {:aggregation [[:sum-where
+ [:field-id (mt/id :venues :price)]
+ [:and [:< [:field-id (mt/id :venues :price)] 4]
+ [:or [:starts-with [:field-id (mt/id :venues :name)] "M"]
+ [:ends-with [:field-id (mt/id :venues :name)] "t"]]]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- nil
- (->> {:aggregation [[:sum-where [:field-id (data/id :venues :price)] [:< [:field-id (data/id :venues :price)] 4]]]
- :filter [:> [:field-id (data/id :venues :price)] Long/MAX_VALUE]}
- (data/run-mbql-query venues)
- rows
- ffirst))
+(deftest filter-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= nil
+ (->> {:aggregation [[:sum-where [:field-id (mt/id :venues :price)] [:< [:field-id (mt/id :venues :price)] 4]]]
+ :filter [:> [:field-id (mt/id :venues :price)] Long/MAX_VALUE]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- [[2 0.0]
- [3 0.0]
- [4 1.0]
- [5 1.0]]
- (->> {:aggregation [[:sum-where [:field-id (data/id :venues :price)] [:< [:field-id (data/id :venues :price)] 2]]]
- :breakout [[:field-id (data/id :venues :category_id)]]
- :limit 4}
- (data/run-mbql-query venues)
- (tu/round-all-decimals 2)
- rows
- (map (fn [[k v]]
- [(long k) (double v)]))))
+(deftest breakout-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (is (= [[2 0.0]
+ [3 0.0]
+ [4 1.0]
+ [5 1.0]]
+ (->> {:aggregation [[:sum-where
+ [:field-id (mt/id :venues :price)]
+ [:< [:field-id (mt/id :venues :price)] 2]]]
+ :breakout [[:field-id (mt/id :venues :category_id)]]
+ :limit 4}
+ (mt/run-mbql-query venues)
+ (mt/round-all-decimals 2)
+ mt/rows
+ (map (fn [[k v]]
+ [(long k) (double v)])))))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations :expressions)
- 90.5
- (->> {:aggregation [[:+ [:/ [:sum-where [:field-id (data/id :venues :price)] [:< [:field-id (data/id :venues :price)] 4]] 2] 1]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double))
+(deftest sum-where-inside-expressions-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations :expressions)
+ (is (= 90.5
+ (->> {:aggregation [[:+
+ [:/
+ [:sum-where
+ [:field-id (mt/id :venues :price)]
+ [:< [:field-id (mt/id :venues :price)] 4]]
+ 2]
+ 1]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double)))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 179.0
- (tt/with-temp* [Segment [{segment-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :filter [:< [:field-id (data/id :venues :price)] 4]}}]]
- (->> {:aggregation [[:sum-where [:field-id (data/id :venues :price)] [:segment segment-id]]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double)))
+(deftest segment-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (mt/with-temp Segment [{segment-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :filter [:< [:field-id (mt/id :venues :price)] 4]}}]
+ (is (= 179.0
+ (->> {:aggregation [[:sum-where [:field-id (mt/id :venues :price)] [:segment segment-id]]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double))))))
-(datasets/expect-with-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- 179.0
- (tt/with-temp* [Metric [{metric-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :aggregation [:sum-where [:field-id (data/id :venues :price)] [:< [:field-id (data/id :venues :price)] 4]]}}]]
- (->> {:aggregation [[:metric metric-id]]}
- (data/run-mbql-query venues)
- rows
- ffirst
- double)))
+(deftest metric-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
+ (mt/with-temp Metric [{metric-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :aggregation [:sum-where
+ [:field-id (mt/id :venues :price)]
+ [:< [:field-id (mt/id :venues :price)] 4]]}}]
+ (is (= 179.0
+ (->> {:aggregation [[:metric metric-id]]}
+ (mt/run-mbql-query venues)
+ mt/rows
+ ffirst
+ double))))))
diff --git a/test/metabase/server/middleware/json_test.clj b/test/metabase/server/middleware/json_test.clj
index 03db9f5aded15346a10d8afbaafbc0b37fdad9b3..9a75b0cfd57f047f8d90e905cdaff984328ec0bb 100644
--- a/test/metabase/server/middleware/json_test.clj
+++ b/test/metabase/server/middleware/json_test.clj
@@ -1,14 +1,13 @@
(ns metabase.server.middleware.json-test
(:require [cheshire.core :as json]
- [expectations :refer [expect]]
- [metabase.plugins.classloader :as classloader]))
+ [clojure.test :refer :all]
+ [metabase.server.middleware.json :as mw.json]))
-;;; JSON encoding tests
-;; Required here so so custom Cheshire encoders are loaded
-(classloader/require 'metabase.server.middleware.json)
+(comment mw.json/keep-me) ; so custom Cheshire encoders are loaded
-;; Check that we encode byte arrays as the hex values of their first four bytes
-(expect
- "{\"my-bytes\":\"0xC42360D7\"}"
- (json/generate-string {:my-bytes (byte-array [196 35 96 215 8 106 108 248 183 215 244 143 17 160 53 186
- 213 30 116 25 87 31 123 172 207 108 47 107 191 215 76 92])}))
+(deftest encode-byte-arrays-test
+ (testing "Check that we encode byte arrays as the hex values of their first four bytes"
+ (is (= "{\"my-bytes\":\"0xC42360D7\"}"
+ (json/generate-string
+ {:my-bytes (byte-array [196 35 96 215 8 106 108 248 183 215 244 143 17 160 53 186
+ 213 30 116 25 87 31 123 172 207 108 47 107 191 215 76 92])})))))
diff --git a/test/metabase/server/middleware/util_test.clj b/test/metabase/server/middleware/util_test.clj
index 52b57ea46b13862fea5b8547e704e5bbf576ee5a..42b08fea773562df7d6d0b518817e22373512f94 100644
--- a/test/metabase/server/middleware/util_test.clj
+++ b/test/metabase/server/middleware/util_test.clj
@@ -1,24 +1,20 @@
(ns metabase.server.middleware.util-test
- (:require [expectations :refer [expect]]
+ (:require [clojure.test :refer :all]
[metabase.server.middleware.util :as mw.util]))
-(defn- https? [headers]
- (mw.util/https-request? {:headers headers}))
-
-(expect true (https? {"x-forwarded-proto" "https"}))
-(expect false (https? {"x-forwarded-proto" "http"}))
-
-(expect true (https? {"x-forwarded-protocol" "https"}))
-(expect false (https? {"x-forwarded-protocol" "http"}))
-
-(expect true (https? {"x-url-scheme" "https"}))
-(expect false (https? {"x-url-scheme" "http"}))
-
-(expect true (https? {"x-forwarded-ssl" "on"}))
-(expect false (https? {"x-forwarded-ssl" "off"}))
-
-(expect true (https? {"front-end-https" "on"}))
-(expect false (https? {"front-end-https" "off"}))
-
-(expect true (https? {"origin" "https://mysite.com"}))
-(expect false (https? {"origin" "http://mysite.com"}))
+(deftest https-request?-test
+ (doseq [[headers expected] {{"x-forwarded-proto" "https"} true
+ {"x-forwarded-proto" "http"} false
+ {"x-forwarded-protocol" "https"} true
+ {"x-forwarded-protocol" "http"} false
+ {"x-url-scheme" "https"} true
+ {"x-url-scheme" "http"} false
+ {"x-forwarded-ssl" "on"} true
+ {"x-forwarded-ssl" "off"} false
+ {"front-end-https" "on"} true
+ {"front-end-https" "off"} false
+ {"origin" "https://mysite.com"} true
+ {"origin" "http://mysite.com"} false}]
+ (testing (pr-str (list 'https-request? {:headers headers}))
+ (is (= expected
+ (mw.util/https-request? {:headers headers}))))))
diff --git a/test/metabase/sync/sync_metadata/metabase_metadata_test.clj b/test/metabase/sync/sync_metadata/metabase_metadata_test.clj
index b167640d941a8a035ef789d6cd94a262d9fa791f..fd45b51f4fac40d98fcad3b7918140660ba6b71a 100644
--- a/test/metabase/sync/sync_metadata/metabase_metadata_test.clj
+++ b/test/metabase/sync/sync_metadata/metabase_metadata_test.clj
@@ -1,47 +1,45 @@
(ns metabase.sync.sync-metadata.metabase-metadata-test
"Tests for the logic that syncs the `_metabase_metadata` Table."
- (:require [expectations :refer :all]
+ (:require [clojure.test :refer :all]
[metabase.models.database :refer [Database]]
[metabase.models.field :refer [Field]]
[metabase.models.table :refer [Table]]
[metabase.sync.sync-metadata.metabase-metadata :as metabase-metadata]
+ [metabase.test :as mt]
[metabase.test.mock.moviedb :as moviedb]
- [metabase.test.util :as tu]
[metabase.util :as u]
[toucan.db :as db]
- [toucan.hydrate :refer [hydrate]]
- [toucan.util.test :as tt]))
+ [toucan.hydrate :refer [hydrate]]))
-;; Test that the `_metabase_metadata` table can be used to populate values for things like descriptions
-(defn- get-table-and-fields-descriptions [table-or-id]
- (-> (db/select-one [Table :id :name :description], :id (u/get-id table-or-id))
- (hydrate :fields)
- (update :fields #(for [field %]
- (select-keys field [:name :description])))
- tu/boolean-ids-and-timestamps))
-
-(expect
- [{:name "movies"
- :description nil
- :id true
- :fields [{:name "filming", :description nil}]}
- {:name "movies"
- :description "A cinematic adventure."
- :id true
- :fields [{:name "filming", :description "If the movie is currently being filmed."}]}]
- (tt/with-temp* [Database [db {:engine ::moviedb/moviedb}]]
- ;; manually add in the movies table
- (let [table (db/insert! Table
- :db_id (u/get-id db)
- :name "movies"
- :active true)]
- (db/insert! Field
- :database_type "BOOL"
- :base_type :type/Boolean
- :table_id (u/get-id table)
- :name "filming")
- ;; here we go
- [(get-table-and-fields-descriptions table)
- (do
- (metabase-metadata/sync-metabase-metadata! db)
- (get-table-and-fields-descriptions table))])))
+(deftest sync-metabase-metadata-test
+ (testing ":Test that the `_metabase_metadata` table can be used to populate values for things like descriptions"
+ (letfn [(get-table-and-fields-descriptions [table-or-id]
+ (-> (db/select-one [Table :id :name :description], :id (u/the-id table-or-id))
+ (hydrate :fields)
+ (update :fields #(for [field %]
+ (select-keys field [:name :description])))
+ mt/boolean-ids-and-timestamps))]
+ (mt/with-temp Database [db {:engine ::moviedb/moviedb}]
+ ;; manually add in the movies table
+ (let [table (db/insert! Table
+ :db_id (u/the-id db)
+ :name "movies"
+ :active true)]
+ (db/insert! Field
+ :database_type "BOOL"
+ :base_type :type/Boolean
+ :table_id (u/the-id table)
+ :name "filming")
+ (testing "before"
+ (is (= {:name "movies"
+ :description nil
+ :id true
+ :fields [{:name "filming", :description nil}]}
+ (get-table-and-fields-descriptions table))))
+ (metabase-metadata/sync-metabase-metadata! db)
+ (testing "after"
+ (is (= {:name "movies"
+ :description "A cinematic adventure."
+ :id true
+ :fields [{:name "filming", :description "If the movie is currently being filmed."}]}
+ (get-table-and-fields-descriptions table)))))))))
diff --git a/test/metabase/sync/sync_metadata/sync_timezone_test.clj b/test/metabase/sync/sync_metadata/sync_timezone_test.clj
index 2762d1933e62b68038f09e8fe0b06355e32a7f5a..b00b9cb5b3d7529d6bcfb4d9a29943f7f48ddf4f 100644
--- a/test/metabase/sync/sync_metadata/sync_timezone_test.clj
+++ b/test/metabase/sync/sync_metadata/sync_timezone_test.clj
@@ -1,67 +1,63 @@
(ns metabase.sync.sync-metadata.sync-timezone-test
(:require [clj-time.core :as time]
+ [clojure.test :refer :all]
[metabase.driver :as driver]
[metabase.models.database :refer [Database]]
[metabase.sync.sync-metadata.sync-timezone :as sync-tz]
[metabase.sync.util-test :as sut]
- [metabase.test.data :as data]
- [metabase.test.data.datasets :as datasets]
- [metabase.test.util :as tu]
+ [metabase.test :as mt]
[metabase.util :as u]
[toucan.db :as db]))
(defn- db-timezone [db-or-id]
- (db/select-one-field :timezone Database :id (u/get-id db-or-id)))
+ (db/select-one-field :timezone Database :id (u/the-id db-or-id)))
-;; This tests populating the timezone field for a given database. The
-;; sync happens automatically, so this test removes it first to ensure
-;; that it gets set when missing
-(datasets/expect-with-drivers #{:h2 :postgres}
- (concat
- (repeat 2 {:timezone-id "UTC"})
- [true true true])
- (data/dataset test-data
- (let [db (data/db)
- tz-on-load (db-timezone db)
- _ (db/update! Database (:id db) :timezone nil)
- tz-after-update (db-timezone db)
- ;; It looks like we can get some stale timezone information depending on which thread is used for querying the
- ;; database in sync. Clearing the connection pool to ensure we get the most updated TZ data
- _ (driver/notify-database-updated driver/*driver* db)
- {:keys [step-info task-history]} (sut/sync-database! "sync-timezone" db)]
+(deftest sync-timezone-test
+ (mt/test-drivers #{:h2 :postgres}
+ (testing (str "This tests populating the timezone field for a given database. The sync happens automatically, so "
+ "this test removes it first to ensure that it gets set when missing")
+ (mt/dataset test-data
+ (let [db (mt/db)
+ tz-on-load (db-timezone db)
+ _ (db/update! Database (:id db) :timezone nil)
+ tz-after-update (db-timezone db)
+ ;; It looks like we can get some stale timezone information depending on which thread is used for querying the
+ ;; database in sync. Clearing the connection pool to ensure we get the most updated TZ data
+ _ (driver/notify-database-updated driver/*driver* db)
+ {:keys [step-info task-history]} (sut/sync-database! "sync-timezone" db)]
+ (testing "only step keys"
+ (is (= {:timezone-id "UTC"}
+ (sut/only-step-keys step-info))))
+ (testing "task details"
+ (is (= {:timezone-id "UTC"}
+ (:task_details task-history))))
+ (testing "On startup is the timezone specified?"
+ (is (time/time-zone-for-id tz-on-load)))
+ (testing "Check to make sure the test removed the timezone"
+ (is (nil? tz-after-update)))
+ (testing "Check that the value was set again after sync"
+ (is (time/time-zone-for-id (db-timezone db)))))))))
- [(sut/only-step-keys step-info)
- (:task_details task-history)
- ;; On startup is the timezone specified?
- (boolean (time/time-zone-for-id tz-on-load))
- ;; Check to make sure the test removed the timezone
- (nil? tz-after-update)
- ;; Check that the value was set again after sync
- (boolean (time/time-zone-for-id (db-timezone db)))])))
-
-;; Test that if timezone is changed to something that fails timezone is unaffected.
-;;
-;; Setting timezone to "Austrailia/Sydney" fails on some computers, especially the CI ones. In that case it fails as
-;; the dates on PostgreSQL return 'AEST' for the time zone name. The Exception is logged, but the timezone column
-;; should be left alone and processing should continue.
-;;
-;; TODO - Recently this call has started *succeeding* for me on Java 10/11 and Postgres 9.6. I've seen it sync as both
-;; "Australia/Hobart" and "Australia/Sydney". Since setting the timezone no longer always fails it's no longer a good
-;; test. We need to think of something else here. In the meantime, I'll go ahead and consider any of the three options
-;; valid answers.
-(datasets/expect-with-drivers #{:postgres}
- {:before "UTC"
- :after true}
- (data/dataset test-data
- ;; use `with-temp-vals-in-db` to make sure the test data DB timezone gets reset to whatever it was before the test
- ;; ran if we accidentally end up setting it in the `:after` part
- (tu/with-temp-vals-in-db Database (data/db) {:timezone (db-timezone (data/db))}
- (sync-tz/sync-timezone! (data/db))
- {:before
- (db-timezone (data/db))
-
- ;; TODO - this works for me ok with Postgres 9.6 & Java 10. Returns
- :after
- (tu/with-temporary-setting-values [report-timezone "Australia/Sydney"]
- (sync-tz/sync-timezone! (data/db))
- (contains? #{"Australia/Hobart" "Australia/Sydney" "UTC"} (db-timezone (data/db))))})))
+(deftest bad-change-test
+ (mt/test-drivers #{:postgres}
+ (testing "Test that if timezone is changed to something that fails, timezone is unaffected."
+ ;; Setting timezone to "Austrailia/Sydney" fails on some computers, especially the CI ones. In that case it fails as
+ ;; the dates on PostgreSQL return 'AEST' for the time zone name. The Exception is logged, but the timezone column
+ ;; should be left alone and processing should continue.
+ ;;
+ ;; TODO - Recently this call has started *succeeding* for me on Java 10/11 and Postgres 9.6. I've seen it sync as both
+ ;; "Australia/Hobart" and "Australia/Sydney". Since setting the timezone no longer always fails it's no longer a good
+ ;; test. We need to think of something else here. In the meantime, I'll go ahead and consider any of the three options
+ ;; valid answers.
+ (mt/dataset test-data
+ ;; use `with-temp-vals-in-db` to make sure the test data DB timezone gets reset to whatever it was before the test
+ ;; ran if we accidentally end up setting it in the `:after` part
+ (mt/with-temp-vals-in-db Database (mt/db) {:timezone (db-timezone (mt/db))}
+ (sync-tz/sync-timezone! (mt/db))
+ (testing "before"
+ (is (= "UTC"
+ (db-timezone (mt/db)))))
+ (testing "after"
+ (mt/with-temporary-setting-values [report-timezone "Australia/Sydney"]
+ (sync-tz/sync-timezone! (mt/db))
+ (is (contains? #{"Australia/Hobart" "Australia/Sydney" "UTC"} (db-timezone (mt/db)))))))))))
diff --git a/test/metabase/test/data/datasets.clj b/test/metabase/test/data/datasets.clj
index feb60b02e36bda5822617d2562039d948953eecf..9315521f39843f21340c056fcbc0a9d624084e8b 100644
--- a/test/metabase/test/data/datasets.clj
+++ b/test/metabase/test/data/datasets.clj
@@ -63,54 +63,3 @@
`(doseq [driver# ~drivers]
(test-driver driver#
~@body)))
-
-(defmacro ^:deprecated expect-with-drivers
- "Generate unit tests for all drivers in env var `DRIVERS`; each test will only run if we're currently testing the
- corresponding driver. `*driver*` is bound to the current driver inside each test.
-
- DEPRECATED: use `deftest` with `test-drivers` instead.
-
- (deftest my-test
- (datasets/test-drivers #{:h2 :postgres}
- (is (= ...))))"
- {:style/indent 1}
- [drivers expected actual]
- ;; Make functions to get expected/actual so the code is only compiled one time instead of for every single driver
- ;; speeds up loading of metabase.driver.query-processor-test significantly
- (let [symb (symbol (str "expect-with-drivers-" (hash &form)))]
- `(t/deftest ~symb
- (t/testing (str "\n" (format ~(str (name (ns-name *ns*)) ":%d") (:line (meta #'~symb))))
- (test-drivers ~drivers
- (t/is (~'expect= ~expected ~actual)))))))
-
-(defmacro ^:deprecated expect-with-driver
- "Generate a unit test that only runs if we're currently testing against `driver`, and that binds `*driver*` when it
- runs.
-
- DEPRECATED: Use `deftest` with `test-driver` instead.
-
- (deftest my-test
- (datasets/test-driver :mysql
- (is (= ...))))"
- {:style/indent 1}
- [driver expected actual]
- `(expect-with-drivers [~driver] ~expected ~actual))
-
-(defmacro test-all-drivers
- "Execute body (presumably containing tests) against all drivers we're currently testing against."
- {:style/indent 0}
- [& body]
- `(test-drivers (tx.env/test-drivers) ~@body))
-
-(defmacro ^:deprecated expect-with-all-drivers
- "Generate unit tests for all drivers specified in env var `DRIVERS`. `*driver*` is bound to the current driver inside
- each test.
-
- DEPRECATED: Use `test-all-drivers` instead.
-
- (deftest my-test
- (datasets/test-all-drivers
- (is (= ...))))"
- {:style/indent 0}
- [expected actual]
- `(expect-with-drivers (tx.env/test-drivers) ~expected ~actual))
diff --git a/test/metabase/util/infer_spaces_test.clj b/test/metabase/util/infer_spaces_test.clj
index 8364c1b1443cdc80175d618435a592109a3a1d3f..6a7c1fe75afc99a41f178b03bd01d175e7f9502f 100644
--- a/test/metabase/util/infer_spaces_test.clj
+++ b/test/metabase/util/infer_spaces_test.clj
@@ -1,13 +1,15 @@
(ns metabase.util.infer-spaces-test
- (:require [expectations :refer :all]
- [metabase.util.infer-spaces :refer :all]))
+ (:require [clojure.test :refer :all]
+ [metabase.util.infer-spaces :as infer-spaces]))
-(expect ["user"] (infer-spaces "user"))
-(expect ["users"] (infer-spaces "users"))
-(expect ["orders"] (infer-spaces "orders"))
-(expect ["products"] (infer-spaces "products"))
-(expect ["events"] (infer-spaces "events"))
-
-(expect ["checkins"] (infer-spaces "checkins"))
-
-(expect ["dashboard" "card" "subscription"] (infer-spaces "dashboardcardsubscription"))
+(deftest infer-spaces-test
+ (doseq [[input expected] {"user" ["user"]
+ "users" ["users"]
+ "orders" ["orders"]
+ "products" ["products"]
+ "events" ["events"]
+ "checkins" ["checkins"]
+ "dashboardcardsubscription" ["dashboard" "card" "subscription"]}]
+ (testing (pr-str (list 'infer-spaces input))
+ (is (= expected
+ (infer-spaces/infer-spaces input))))))
diff --git a/test/metabase/util/password_test.clj b/test/metabase/util/password_test.clj
index 28f589d93c1ba56a0a7c7088c96bc17e17461d81..579a6a979ef31cebe16fe42527f33c06a3030a57 100644
--- a/test/metabase/util/password_test.clj
+++ b/test/metabase/util/password_test.clj
@@ -1,43 +1,59 @@
(ns metabase.util.password-test
(:require [expectations :refer [expect]]
+ [clojure.test :refer :all]
[metabase.util.password :as pwu]))
;; Password Complexity testing
-;; Check that password occurance counting works
-(expect {:total 3, :lower 3, :upper 0, :letter 3, :digit 0, :special 0} (#'pwu/count-occurrences "abc"))
-(expect {:total 8, :lower 0, :upper 8, :letter 8, :digit 0, :special 0} (#'pwu/count-occurrences "PASSWORD"))
-(expect {:total 3, :lower 0, :upper 0, :letter 0, :digit 3, :special 0} (#'pwu/count-occurrences "123"))
-(expect {:total 8, :lower 4, :upper 2, :letter 6, :digit 0, :special 2} (#'pwu/count-occurrences "GoodPw!!"))
-(expect {:total 9, :lower 7, :upper 1, :letter 8, :digit 1, :special 0} (#'pwu/count-occurrences "passworD1"))
-(expect {:total 10, :lower 3, :upper 2, :letter 5, :digit 1, :special 4} (#'pwu/count-occurrences "^^Wut4nG^^"))
+(deftest count-occurrences-test
+ (testing "Check that password occurance counting works"
+ (doseq [[input expected] {"abc" {:total 3, :lower 3, :upper 0, :letter 3, :digit 0, :special 0}
+ "PASSWORD" {:total 8, :lower 0, :upper 8, :letter 8, :digit 0, :special 0}
+ "123" {:total 3, :lower 0, :upper 0, :letter 0, :digit 3, :special 0}
+ "GoodPw!!" {:total 8, :lower 4, :upper 2, :letter 6, :digit 0, :special 2}
+ "passworD1" {:total 9, :lower 7, :upper 1, :letter 8, :digit 1, :special 0}
+ "^^Wut4nG^^" {:total 10, :lower 3, :upper 2, :letter 5, :digit 1, :special 4}}]
+ (testing (pr-str (list 'count-occurrences input))
+ (is (= expected
+ (#'pwu/count-occurrences input)))))))
-;; Check that password length complexity applies
-(expect true (#'pwu/password-has-char-counts? {:total 3} "god1"))
-(expect true (#'pwu/password-has-char-counts? {:total 4} "god1"))
-(expect false (#'pwu/password-has-char-counts? {:total 5} "god1"))
+(deftest password-has-char-counts?-test
+ (doseq [[group input->expected]
+ {"Check that password length complexity applies"
+ {[{:total 3} "god1"] true
+ [{:total 4} "god1"] true
+ [{:total 5} "god1"] false}
-;; Check that testing password character type complexity works
-(expect true (#'pwu/password-has-char-counts? {} "ABC"))
-(expect false (#'pwu/password-has-char-counts? {:lower 1} "ABC"))
-(expect true (#'pwu/password-has-char-counts? {:lower 1} "abc"))
-(expect false (#'pwu/password-has-char-counts? {:digit 1} "abc"))
-(expect true (#'pwu/password-has-char-counts? {:digit 1, :special 2} "!0!"))
+ "Check that testing password character type complexity works"
+ {[{} "ABC"] true
+ [{:lower 1} "ABC"] false
+ [{:lower 1} "abc"] true
+ [{:digit 1} "abc"] false
+ [{:digit 1, :special 2} "!0!"] true}
-;; Do some tests that combine both requirements
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^aA2"))
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password"))
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1"))
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1!"))
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD!"))
-(expect false (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1"))
-(expect true (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1!"))
-(expect true (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "paSS&&word1"))
-(expect true (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passW0rd))"))
-(expect true (#'pwu/password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^^Wut4nG^^"))
+ "Do some tests that combine both requirements"
+ {[{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^aA2"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1!"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD!"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1"] false
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1!"] true
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "paSS&&word1"] true
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passW0rd))"] true
+ [{:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^^Wut4nG^^"] true}}]
+ (testing group
+ (doseq [[input expected] input->expected]
+ (testing (pr-str (cons 'password-has-char-counts? input))
+ (is (= expected
+ (apply #'pwu/password-has-char-counts? input))))))))
-;; Do some tests with the default (:normal) password requirements
-(expect false (pwu/is-complex? "ABC"))
-(expect false (pwu/is-complex? "ABCDEF"))
-(expect true (pwu/is-complex? "ABCDE1"))
-(expect true (pwu/is-complex? "123456"))
+(deftest is-complex?-test
+ (testing "Do some tests with the default (:normal) password requirements"
+ (doseq [[input expected] {"ABC" false
+ "ABCDEF" false
+ "ABCDE1" true
+ "123456" true}]
+ (testing (pr-str (list 'is-complex? input))
+ (is (= expected
+ (pwu/is-complex? input)))))))