From 93b653092d7a61912446b1a7b97f5728d869dd53 Mon Sep 17 00:00:00 2001
From: Cam Saul <cammsaul@gmail.com>
Date: Mon, 17 Jun 2019 16:53:41 -0700
Subject: [PATCH] Test fixes (#10202)
---
.../test/metabase/driver/bigquery_test.clj | 6 ++--
.../druid/test/metabase/driver/druid_test.clj | 12 +++----
.../mongo/test/metabase/driver/mongo_test.clj | 2 +-
test/expectation_options.clj | 2 +-
test/metabase/api/database_test.clj | 3 +-
test/metabase/api/embed_test.clj | 4 +--
test/metabase/api/public_test.clj | 4 +--
test/metabase/http_client.clj | 32 ++++++++++---------
.../field_visibility_test.clj | 28 ++++++++--------
.../sync/analyze/query_results_test.clj | 9 +++---
test/metabase/test_setup.clj | 2 ++
11 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
index e22392f8c60..e9501286f34 100644
--- a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+++ b/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`, "
diff --git a/modules/drivers/druid/test/metabase/driver/druid_test.clj b/modules/drivers/druid/test/metabase/driver/druid_test.clj
index 9094ea6b71a..e8301e7313b 100644
--- a/modules/drivers/druid/test/metabase/driver/druid_test.clj
+++ b/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])))
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
index 20f8efc1d66..06d7c0b3ca1 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
+++ b/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
diff --git a/test/expectation_options.clj b/test/expectation_options.clj
index fe61145940b..814b182f4a6 100644
--- a/test/expectation_options.clj
+++ b/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))
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index e5aea35b200..9d15e192ae1 100644
--- a/test/metabase/api/database_test.clj
+++ b/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
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index d2753185458..28348874a47 100644
--- a/test/metabase/api/embed_test.clj
+++ b/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"))))))
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index b44e147e7c1..c21265c2201 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -1,10 +1,10 @@
(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]]
diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj
index 66837924419..273c9ff14dc 100644
--- a/test/metabase/http_client.clj
+++ b/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)))
diff --git a/test/metabase/query_processor_test/field_visibility_test.clj b/test/metabase/query_processor_test/field_visibility_test.clj
index 2dbc1f84ceb..4354e5205c4 100644
--- a/test/metabase/query_processor_test/field_visibility_test.clj
+++ b/test/metabase/query_processor_test/field_visibility_test.clj
@@ -1,7 +1,7 @@
(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])))))
diff --git a/test/metabase/sync/analyze/query_results_test.clj b/test/metabase/sync/analyze/query_results_test.clj
index d54b003a366..9ac3013ce14 100644
--- a/test/metabase/sync/analyze/query_results_test.clj
+++ b/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)]]))))))
diff --git a/test/metabase/test_setup.clj b/test/metabase/test_setup.clj
index 692e99c8103..7894795653d 100644
--- a/test/metabase/test_setup.clj
+++ b/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!)))
--
GitLab