Test fixes (#10202)

modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj

@@ -54,9 +54,9 @@
 ;; ordering shouldn't apply (Issue #2821)
 (expect-with-driver :bigquery
   {:columns ["venue_id" "user_id" "checkins_id"],
-   :cols    [{:name "venue_id",    :display_name "Venue ID",    :source :native, :base_type :type/Integer}
-             {:name "user_id",     :display_name  "User ID",    :source :native, :base_type :type/Integer}
-             {:name "checkins_id", :display_name "Checkins ID", :source :native, :base_type :type/Integer}]}
+   :cols    [{:name "venue_id",    :display_name "venue_id",    :source :native, :base_type :type/Integer}
+             {:name "user_id",     :display_name "user_id",     :source :native, :base_type :type/Integer}
+             {:name "checkins_id", :display_name "checkins_id", :source :native, :base_type :type/Integer}]}
 
   (select-keys (:data (qp/process-query
                         {:native   {:query (str "SELECT `test_data.checkins`.`venue_id` AS `venue_id`, "

modules/drivers/druid/test/metabase/driver/druid_test.clj

@@ -103,12 +103,12 @@
                :rows        [["2013-01-03T08:00:00.000Z" "931" "Simcha Yan" "1" "Kinaree Thai Bistro"       1]
                              ["2013-01-10T08:00:00.000Z" "285" "Kfir Caj"   "2" "Ruen Pair Thai Restaurant" 1]]
                :cols        (mapv #(merge col-defaults %)
-                                  [{:name "timestamp",   :source :native, :display_name "Timestamp"}
-                                   {:name "id",          :source :native, :display_name "ID"}
-                                   {:name "user_name",   :source :native, :display_name "User Name"}
-                                   {:name "venue_price", :source :native, :display_name "Venue Price"}
-                                   {:name "venue_name",  :source :native, :display_name "Venue Name"}
-                                   {:name "count",       :source :native, :display_name "Count", :base_type :type/Integer}])
+                                  [{:name "timestamp",   :source :native, :display_name "timestamp"}
+                                   {:name "id",          :source :native, :display_name "id"}
+                                   {:name "user_name",   :source :native, :display_name "user_name"}
+                                   {:name "venue_price", :source :native, :display_name "venue_price"}
+                                   {:name "venue_name",  :source :native, :display_name "venue_name"}
+                                   {:name "count",       :source :native, :display_name "count", :base_type :type/Integer}])
                :native_form {:query native-query-1}}}
   (-> (process-native-query native-query-1)
       (m/dissoc-in [:data :insights])))

modules/drivers/mongo/test/metabase/driver/mongo_test.clj

@@ -80,7 +80,7 @@
    :row_count 1
    :data      {:rows        [[1]]
                :columns     ["count"]
-               :cols        [{:name "count", :display_name "Count", :base_type :type/Integer, :source :native}]
+               :cols        [{:name "count", :display_name "count", :base_type :type/Integer, :source :native}]
                :native_form {:collection "venues"
                              :query      native-query}}}
   (-> (qp/process-query {:native   {:query      native-query

test/expectation_options.clj

@@ -191,7 +191,7 @@
       ;; Uncomment `check-test-data-unchanged` when you want to debug situations where tests are being bad citizens
       ;; and leaving the metadata about the test data in a different state than it started out with. This is not
       ;; enabled by default because it adds ~5ms to each test which adds up when we have ~4000 tests (as of May 2019)
-      #_check-test-data-unchanged
+      check-test-data-unchanged
       log-slow-tests
       enforce-timeout
       check-table-cleanup))

test/metabase/api/database_test.clj

@@ -190,7 +190,8 @@
 (expect-with-temp-db-created-via-api [{db-id :id, db-name :name}]
   (vec
    (sort-by
-    (fn [{db-name :name, driver :engine}] (mapv (comp str/lower-case name) [db-name driver]))
+    (fn [{db-name :name, driver :engine}]
+      [(str/lower-case db-name) (str/lower-case (name driver))])
     (cons
      (merge
       default-db-details

test/metabase/api/embed_test.clj

@@ -4,11 +4,11 @@
              [jwt :as jwt]
              [util :as buddy-util]]
             [clj-time.core :as time]
+            [clojure.string :as str]
             [crypto.random :as crypto-random]
             [dk.ative.docjure.spreadsheet :as spreadsheet]
             [expectations :refer :all]
             [metabase
-             [config :as config]
              [http-client :as http]
              [util :as u]]
             [metabase.api
@@ -325,7 +325,7 @@
   (with-embedding-enabled-and-new-secret-key
     (tt/with-temp Card [card (card-with-date-field-filter)]
       ;; make sure the URL doesn't include /api/ at the beginning like it normally would
-      (binding [http/*url-prefix* (str "http://localhost:" (config/config-str :mb-jetty-port) "/")]
+      (binding [http/*url-prefix* (str/replace http/*url-prefix* #"/api/$" "/")]
         (http/client :get 200 (str "embed/question/" (card-token card) ".csv?date=Q1-2014"))))))
 
 

test/metabase/api/public_test.clj

 (ns metabase.api.public-test
   "Tests for `api/public/` (public links) endpoints."
   (:require [cheshire.core :as json]
+            [clojure.string :as str]
             [dk.ative.docjure.spreadsheet :as spreadsheet]
             [expectations :refer :all]
             [metabase
-             [config :as config]
              [http-client :as http]
              [query-processor-test :as qp.test]
              [util :as u]]
@@ -219,7 +219,7 @@
   (tu/with-temporary-setting-values [enable-public-sharing true]
     (tt/with-temp Card [{uuid :public_uuid} (card-with-date-field-filter)]
       ;; make sure the URL doesn't include /api/ at the beginning like it normally would
-      (binding [http/*url-prefix* (str "http://localhost:" (config/config-str :mb-jetty-port) "/")]
+      (binding [http/*url-prefix* (str/replace http/*url-prefix* #"/api/$" "/")]
         (http/client :get 200 (str "public/question/" uuid ".csv")
                      :parameters (json/encode [{:type   :date/quarter-year
                                                 :target [:dimension [:template-tag :date]]

test/metabase/http_client.clj

@@ -18,7 +18,7 @@
   (str "http://localhost:" (config/config-str :mb-jetty-port) "/api/"))
 
 (defn build-url
-  "Build an API URL for `localhost` and `MB_JETTY_PORT` with URL-PARAM-KWARGS.
+  "Build an API URL for `localhost` and `MB_JETTY_PORT` with `url-param-kwargs`.
 
      (build-url \"db/1\" {:x true}) -> \"http://localhost:3000/api/db/1?x=true\""
   [url url-param-kwargs]
@@ -36,8 +36,8 @@
   #{:created_at :updated_at :last_login :date_joined :started_at :finished_at :last_analyzed})
 
 (defn- auto-deserialize-dates
-  "Automatically recurse over RESPONSE and look for keys that are known to correspond to dates.
-   Parse their values and convert to `java.sql.Timestamps`."
+  "Automatically recurse over `response` and look for keys that are known to correspond to dates. Parse their values and
+  convert to `java.sql.Timestamps`."
   [response]
   (cond (sequential? response) (map auto-deserialize-dates response)
         (map? response) (->> response
@@ -67,8 +67,8 @@
 (declare client)
 
 (s/defn authenticate
-  "Authenticate a test user with USERNAME and PASSWORD, returning their Metabase Session token;
-   or throw an Exception if that fails."
+  "Authenticate a test user with `username` and `password`, returning their Metabase Session token; or throw an
+  Exception if that fails."
   [credentials :- {:username s/Str, :password s/Str}]
   (try
     (:id (client :post 200 "session" credentials))
@@ -91,11 +91,13 @@
      {:body (json/generate-string http-body)})))
 
 (defn- check-status-code
-  "If an EXPECTED-STATUS-CODE was passed to the client, check that the actual status code matches, or throw an exception."
+  "If an `expected-status-code` was passed to the client, check that the actual status code matches, or throw an
+  exception."
   [method-name url body expected-status-code actual-status-code]
   (when expected-status-code
     (when (not= actual-status-code expected-status-code)
-      (let [message (format "%s %s expected a status code of %d, got %d." method-name url expected-status-code actual-status-code)
+      (let [message (format "%s %s expected a status code of %d, got %d."
+                            method-name url expected-status-code actual-status-code)
             body    (try
                       (json/parse-string body keyword)
                       (catch Throwable _
@@ -148,7 +150,7 @@
 
 (defn client
   "Perform an API call and return the response (for test purposes).
-   The first arg after URL will be passed as a JSON-encoded body if it is a map.
+   The first arg after `url` will be passed as a JSON-encoded body if it is a map.
    Other &rest kwargs will be passed as `GET` parameters.
 
   Examples:
@@ -159,13 +161,13 @@
 
   Args:
 
-   *  CREDENTIALS           Optional map of `:username` and `:password` or `X-METABASE-SESSION` token of a User who we
-                            should perform the request as
-   *  METHOD                `:get`, `:post`, `:delete`, or `:put`
-   *  EXPECTED-STATUS-CODE  When passed, throw an exception if the response has a different status code.
-   *  URL                   Base URL of the request, which will be appended to `*url-prefix*`. e.g. `card/1/favorite`
-   *  HTTP-BODY-MAP         Optional map to send a the JSON-serialized HTTP body of the request
-   *  URL-KWARGS            key-value pairs that will be encoded and added to the URL as GET params"
+   *  `credentials`          Optional map of `:username` and `:password` or `X-METABASE-SESSION` token of a User who we
+                             should perform the request as
+   *  `method`               `:get`, `:post`, `:delete`, or `:put`
+   *  `expected-status-code` When passed, throw an exception if the response has a different status code.
+   *  `url`                  Base URL of the request, which will be appended to `*url-prefix*`. e.g. `card/1/favorite`
+   *  `http-body-map`        Optional map to send a the JSON-serialized HTTP body of the request
+   *  `url-kwargs`           key-value pairs that will be encoded and added to the URL as GET params"
   {:arglists '([credentials? method expected-status-code? url request-options? http-body-map? & url-kwargs])}
   [& args]
   (:body (apply client-full-response args)))

test/metabase/query_processor_test/field_visibility_test.clj

 (ns metabase.query-processor-test.field-visibility-test
   "Tests for behavior of fields with different visibility settings."
   (:require [metabase
-             [query-processor-test :refer :all]
+             [query-processor-test :as qp.test]
              [util :as u]]
             [metabase.models.field :refer [Field]]
             [metabase.test
@@ -11,7 +11,7 @@
 ;;; ---------------------------------------------- :details-only fields ----------------------------------------------
 
 ;; make sure that rows where visibility_type = details-only are included and properly marked up
-(defn- get-col-names []
+(defn- venues-cols-from-query []
   (-> (data/run-mbql-query venues
         {:order-by [[:asc $id]]
          :limit    1})
@@ -20,27 +20,27 @@
       :cols
       set))
 
-(expect-with-non-timeseries-dbs
-  (u/key-by :id (venues-cols))
-  (u/key-by :id (get-col-names)))
+(qp.test/expect-with-non-timeseries-dbs
+  (u/key-by :id (qp.test/venues-cols))
+  (u/key-by :id (venues-cols-from-query)))
 
-(expect-with-non-timeseries-dbs
-  (u/key-by :id (for [col (venues-cols)]
+(qp.test/expect-with-non-timeseries-dbs
+  (u/key-by :id (for [col (qp.test/venues-cols)]
                   (if (= (data/id :venues :price) (u/get-id col))
                     (assoc col :visibility_type :details-only)
                     col)))
   (tu/with-temp-vals-in-db Field (data/id :venues :price) {:visibility_type :details-only}
-    (u/key-by :id (get-col-names))))
+    (u/key-by :id (venues-cols-from-query))))
 
 
 ;;; ----------------------------------------------- :sensitive fields ------------------------------------------------
 
 ;;; Make sure :sensitive information fields are never returned by the QP
-(qp-expect-with-all-drivers
-  {:columns     (->columns "id" "name" "last_login")
-   :cols        [(users-col :id)
-                 (users-col :name)
-                 (users-col :last_login)]
+(qp.test/qp-expect-with-all-drivers
+  {:columns     (qp.test/->columns "id" "name" "last_login")
+   :cols        [(qp.test/users-col :id)
+                 (qp.test/users-col :name)
+                 (qp.test/users-col :last_login)]
    :rows        [[ 1 "Plato Yeshua"]
                  [ 2 "Felipinho Asklepios"]
                  [ 3 "Kaneonuskatew Eiran"]
@@ -60,7 +60,7 @@
   ;; Filter out the timestamps from the results since they're hard to test :/
   (-> (data/run-mbql-query users
         {:order-by [[:asc $id]]})
-      booleanize-native-form
+      qp.test/booleanize-native-form
       tu/round-fingerprint-cols
       (update-in [:data :rows] (partial mapv (fn [[id name last-login]]
                                                [(int id) name])))))

test/metabase/sync/analyze/query_results_test.clj

@@ -6,6 +6,7 @@
              [util :as u]]
             [metabase.mbql.schema :as mbql.s]
             [metabase.models.card :refer [Card]]
+            [metabase.query-processor.test-util :as qp.test-util]
             [metabase.sync.analyze.fingerprint.fingerprinters :as fprint]
             [metabase.sync.analyze.query-results :as qr]
             [metabase.test
@@ -55,9 +56,9 @@
 ;; Getting the result metadata for a card backed by an MBQL query should use the fingerprints from the related fields
 (expect
   mock.u/venue-fingerprints
-  (tu/throw-if-called fprint/with-global-fingerprinter
-    (tt/with-temp Card [card {:dataset_query (data/mbql-query venues)}]
-      ;; check for a "proper" fingerprinter, fallthrough for PKs is fine.
+  (tt/with-temp Card [card (qp.test-util/card-with-source-metadata-for-query (data/mbql-query venues))]
+    ;; check for a "proper" fingerprinter, fallthrough for PKs is fine.
+    (tu/throw-if-called fprint/with-global-fingerprinter
       (name->fingerprints
        (query->result-metadata (query-for-card card))))))
 
@@ -94,7 +95,7 @@
 ;; Limiting to just 1 column on an MBQL query should still get the result metadata from the Field
 (expect
   (select-keys mock.u/venue-fingerprints [:longitude])
-  (tt/with-temp Card [card {:dataset_query (data/mbql-query venues)}]
+  (tt/with-temp Card [card (qp.test-util/card-with-source-metadata-for-query (data/mbql-query venues))]
     (tu/throw-if-called fprint/fingerprinter
       (name->fingerprints
        (query->result-metadata (assoc-in (query-for-card card) [:query :fields] [[:field-id (data/id :venues :longitude)]]))))))

test/metabase/test_setup.clj

@@ -7,6 +7,7 @@
              [config :as config]
              [db :as mdb]
              [handler :as handler]
+             [metabot :as metabot]
              [plugins :as plugins]
              [server :as server]
              [task :as task]
@@ -108,6 +109,7 @@
   []
   (log/info "Shutting down Metabase unit test runner")
   (server/stop-web-server!)
+  (metabot/stop-metabot!)
   (when config/is-test?
     (shutdown-threads!)))