From a2ba5438451d95acfefe0324595cea3c8afc5f74 Mon Sep 17 00:00:00 2001 From: Cam Saul <cammsaul@gmail.com> Date: Thu, 16 May 2019 17:53:56 -0700 Subject: [PATCH] Code cleanup :shower: --- test/metabase/models/dashboard_test.clj | 2 +- .../middleware/parameters/sql_test.clj | 15 +++++++------ .../query_processor_test/fields_test.clj | 22 +++++++++---------- .../query_processor_test/joins_test.clj | 2 +- .../nested_queries_test.clj | 3 ++- test/metabase/test/data/datasets.clj | 17 +++++++------- test/metabase/test/data/sql.clj | 22 +++++++++---------- 7 files changed, 43 insertions(+), 40 deletions(-) diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj index f23c01044c6..e96686ef8a4 100644 --- a/test/metabase/models/dashboard_test.clj +++ b/test/metabase/models/dashboard_test.clj @@ -1,5 +1,5 @@ (ns metabase.models.dashboard-test - (:require [expectations :refer :all] + (:require [expectations :refer [expect]] [metabase.api.common :as api] [metabase.automagic-dashboards.core :as magic] [metabase.models diff --git a/test/metabase/query_processor/middleware/parameters/sql_test.clj b/test/metabase/query_processor/middleware/parameters/sql_test.clj index 9f7741663ea..60bad2ee46a 100644 --- a/test/metabase/query_processor/middleware/parameters/sql_test.clj +++ b/test/metabase/query_processor/middleware/parameters/sql_test.clj @@ -778,13 +778,14 @@ :param {:type :date/all-options :target [:dimension [:template-tag "checkin_date"]] :value "past5days"}} - (#'sql/dimension-value-for-tag {:name "checkin_date" - :display-name "Checkin Date" - :type :dimension - :dimension [:field-id (data/id :checkins :date)] - :default "past5days" - :widget-type :date/all-options} - nil)) + (#'sql/dimension-value-for-tag + {:name "checkin_date" + :display-name "Checkin Date" + :type :dimension + :dimension [:field-id (data/id :checkins :date)] + :default "past5days" + :widget-type :date/all-options} + nil)) ;; Make sure we can specify the type of a default value for a "Dimension" (Field Filter) by setting the ;; `:widget-type` key. Check that it works correctly with relative dates... diff --git a/test/metabase/query_processor_test/fields_test.clj b/test/metabase/query_processor_test/fields_test.clj index f565d872767..5b81258761a 100644 --- a/test/metabase/query_processor_test/fields_test.clj +++ b/test/metabase/query_processor_test/fields_test.clj @@ -1,11 +1,11 @@ (ns metabase.query-processor-test.fields-test "Tests for the `:fields` clause." - (:require [metabase.query-processor-test :refer :all] + (:require [metabase.query-processor-test :as qp.test] [metabase.test.data :as data])) ;; Test that we can restrict the Fields that get returned to the ones specified, and that results come back in the ;; order of the IDs in the `fields` clause -(qp-expect-with-all-drivers +(qp.test/qp-expect-with-all-drivers {:rows [["Red Medicine" 1] ["Stout Burgers & Beers" 2] ["The Apple Pan" 3] @@ -16,13 +16,13 @@ ["25°" 8] ["Krua Siri" 9] ["Fred 62" 10]] - :columns (->columns "name" "id") - :cols [(venues-col :name) - (venues-col :id)] + :columns (qp.test/->columns "name" "id") + :cols [(qp.test/venues-col :name) + (qp.test/venues-col :id)] :native_form true} - (->> (data/run-mbql-query venues - {:fields [$name $id] - :limit 10 - :order-by [[:asc $id]]}) - booleanize-native-form - (format-rows-by [str int]))) + (->> (data/run-mbql-query venues + {:fields [$name $id] + :limit 10 + :order-by [[:asc $id]]}) + qp.test/booleanize-native-form + (qp.test/format-rows-by [str int]))) diff --git a/test/metabase/query_processor_test/joins_test.clj b/test/metabase/query_processor_test/joins_test.clj index 69db368fa1e..f6b3f50d89b 100644 --- a/test/metabase/query_processor_test/joins_test.clj +++ b/test/metabase/query_processor_test/joins_test.clj @@ -93,7 +93,7 @@ (qp.test/format-rows-by [int int int]))) -;; Check that trying to use a Foreign Key fails for Mongo +;; Check that trying to use a Foreign Key fails for Mongo and other DBs (datasets/expect-with-drivers (qp.test/non-timeseries-drivers-without-feature :foreign-keys) {:status :failed :error "foreign-keys is not supported by this driver."} diff --git a/test/metabase/query_processor_test/nested_queries_test.clj b/test/metabase/query_processor_test/nested_queries_test.clj index 249f87b97b3..0fcbcd06d19 100644 --- a/test/metabase/query_processor_test/nested_queries_test.clj +++ b/test/metabase/query_processor_test/nested_queries_test.clj @@ -285,7 +285,8 @@ "\"PUBLIC\".\"VENUES\".\"CATEGORY_ID\" AS \"CATEGORY_ID\", \"PUBLIC\".\"VENUES\".\"LATITUDE\" AS \"LATITUDE\", " "\"PUBLIC\".\"VENUES\".\"LONGITUDE\" AS \"LONGITUDE\", \"PUBLIC\".\"VENUES\".\"PRICE\" AS \"PRICE\" FROM \"PUBLIC\".\"VENUES\") \"source\"")) -;; make sure that dots in field literal identifiers get escaped so you can't reference fields from other tables using them +;; make sure that dots in field literal identifiers get escaped so you can't reference fields from other tables using +;; them (expect {:query (format "SELECT * FROM %s WHERE \"BIRD.ID\" = 1 LIMIT 10" venues-source-sql) :params nil} diff --git a/test/metabase/test/data/datasets.clj b/test/metabase/test/data/datasets.clj index 7ac9570a146..a886c2dd883 100644 --- a/test/metabase/test/data/datasets.clj +++ b/test/metabase/test/data/datasets.clj @@ -10,7 +10,7 @@ ;; # Helper Macros (defn do-when-testing-driver - "Call function F (always with no arguments) *only* if we are currently testing against ENGINE. + "Call function `f` (always with no arguments) *only* if we are currently testing against `driver`. (This does NOT bind `*driver*`; use `driver/with-driver` if you want to do that.)" {:style/indent 1} [driver f] @@ -25,7 +25,8 @@ `(do-when-testing-driver ~driver (fn [] ~@body))) (defmacro with-driver-when-testing - "When `driver` is specified in `DRIVERS`, bins `*driver*` and executes `body`." + "When `driver` is specified in `DRIVERS` env var, binds `metabase.driver/*driver*` and executes `body`. The currently + bound driver is used for calls like `(data/db)` and `(data/id)`." {:style/indent 1} [driver & body] `(let [driver# ~driver] @@ -34,8 +35,8 @@ ~@body)))) (defmacro expect-with-driver - "Generate a unit test that only runs if we're currently testing against ENGINE, and that binds `*driver*` to the - driver for ENGINE." + "Generate a unit test that only runs if we're currently testing against `driver`, and that binds `*driver*` when it + runs." {:style/indent 1} [driver expected actual] `(when-testing-driver ~driver @@ -70,8 +71,8 @@ nil))) (defmacro expect-with-drivers - "Generate unit tests for all drivers in `DRIVERS`; each test will only run if we're currently testing the - corresponding dataset. `*driver*` is bound to the current dataset inside each test." + "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." {: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 @@ -84,8 +85,8 @@ (doexpect-with-drivers (get-drivers-or-fail (fn [] ~drivers)) get-e# get-a# '~expected '~actual)))) (defmacro expect-with-all-drivers - "Generate unit tests for all drivers specified in `DRIVERS`. `*driver*` is bound to the current driver inside each - test." + "Generate unit tests for all drivers specified in env var `DRIVERS`. `*driver*` is bound to the current driver inside + each test." {:style/indent 0} [expected actual] `(expect-with-drivers tx.env/test-drivers ~expected ~actual)) diff --git a/test/metabase/test/data/sql.clj b/test/metabase/test/data/sql.clj index 9b8720c872f..11e060de5a6 100644 --- a/test/metabase/test/data/sql.clj +++ b/test/metabase/test/data/sql.clj @@ -1,6 +1,7 @@ (ns metabase.test.data.sql "Common test extension functionality for all SQL drivers." - (:require [metabase.driver :as driver] + (:require [clojure.string :as str] + [metabase.driver :as driver] [metabase.driver.sql [query-processor :as sql.qp] [util :as sql.u]] @@ -199,16 +200,15 @@ pk-field-name (quot (pk-field-name driver))] (format "CREATE TABLE %s (%s, %s %s, PRIMARY KEY (%s)) %s;" (qualify-and-quote driver database-name table-name) - (->> field-definitions - (map (fn [{:keys [field-name base-type field-comment]}] - (format "%s %s %s" - (quot field-name) - (if (map? base-type) - (:native base-type) - (field-base-type->sql-type driver base-type)) - (or (inline-column-comment-sql driver field-comment) "")))) - (interpose ", ") - (apply str)) + (str/join + " ," + (for [{:keys [field-name base-type field-comment]} field-definitions] + (format "%s %s %s" + (quot field-name) + (if (map? base-type) + (:native base-type) + (field-base-type->sql-type driver base-type)) + (or (inline-column-comment-sql driver field-comment) "")))) pk-field-name (pk-sql-type driver) pk-field-name (or (inline-table-comment-sql driver table-comment) "")))) -- GitLab