Code cleanup

test/metabase/models/dashboard_test.clj

 (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

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...

test/metabase/query_processor_test/fields_test.clj

 (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])))

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."}

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}

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))

test/metabase/test/data/sql.clj

 (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) ""))))