Skip to content

Commit

Permalink
Merge pull request metabase#4288 from metabase/more-test-fixes
Browse files Browse the repository at this point in the history
Even more test fixes from metabase#4147 😒
  • Loading branch information
camsaul authored Feb 1, 2017
2 parents 261b4a2 + f541aef commit ef17e31
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 94 deletions.
1 change: 0 additions & 1 deletion .dir-locals.el
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
(execute-query 1)
(execute-sql! 2)
(expect 0)
(expect-when-testing-engine 1)
(expect-with-all-engines 0)
(expect-with-engine 1)
(expect-with-engines 1)
Expand Down
141 changes: 64 additions & 77 deletions test/metabase/driver/mongo_test.clj
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
(ns metabase.driver.mongo-test
"Tests for Mongo driver."
(:require [expectations :refer :all]
(:require [clojure.walk :as walk]
[expectations :refer :all]
[toucan.db :as db]
[metabase.driver :as driver]
(metabase.models [database :refer [Database]]
[field :refer [Field]]
[field-values :refer [FieldValues]]
[table :refer [Table] :as table])
[metabase.query-processor :as qp]
[metabase.query-processor.expand :as ql]
Expand All @@ -17,16 +19,6 @@
org.joda.time.DateTime
metabase.driver.mongo.MongoDriver))

;; ## Logic for selectively running mongo

(defmacro expect-when-testing-mongo {:style/indent 0} [expected actual]
`(datasets/expect-when-testing-engine :mongo
~expected
~actual))

(defn- mongo-db []
(data/get-or-create-test-data-db! (driver/engine->driver :mongo)))

;; ## Constants + Helper Fns/Macros
;; TODO - move these to metabase.test-data ?
(def ^:private ^:const table-names
Expand All @@ -36,38 +28,37 @@
:users
:venues])

(defn- table-name->table
"Return the `Table` matching TABLE-NAME in the Mongo `Test Data` DB."
[table-name]
{:pre [(keyword? table-name)]}
(Table (datasets/with-engine :mongo
(data/id table-name))))

;; ## Tests for connection functions

(expect-when-testing-mongo false
(datasets/expect-with-engine :mongo
false
(driver/can-connect-with-details? :mongo {:host "localhost"
:port 3000
:dbname "bad-db-name"}))

(expect-when-testing-mongo false
(datasets/expect-with-engine :mongo
false
(driver/can-connect-with-details? :mongo {}))

(expect-when-testing-mongo true
(datasets/expect-with-engine :mongo
true
(driver/can-connect-with-details? :mongo {:host "localhost"
:port 27017
:dbname "metabase-test"}))

;; should use default port 27017 if not specified
(expect-when-testing-mongo true
(datasets/expect-with-engine :mongo
true
(driver/can-connect-with-details? :mongo {:host "localhost"
:dbname "metabase-test"}))

(expect-when-testing-mongo false
(datasets/expect-with-engine :mongo
false
(driver/can-connect-with-details? :mongo {:host "123.4.5.6"
:dbname "bad-db-name?connectTimeoutMS=50"}))

(expect-when-testing-mongo false
(datasets/expect-with-engine :mongo
false
(driver/can-connect-with-details? :mongo {:host "localhost"
:port 3000
:dbname "bad-db-name?connectTimeoutMS=50"}))
Expand All @@ -79,7 +70,7 @@
{\"$sort\": {\"_id\": 1}},
{\"$project\": {\"_id\": false, \"count\": true}}]")

(expect-when-testing-mongo
(datasets/expect-with-engine :mongo
{:status :completed
:row_count 1
:data {:rows [[1]]
Expand All @@ -91,87 +82,83 @@
(qp/process-query {:native {:query native-query
:collection "venues"}
:type :native
:database (:id (mongo-db))}))
:database (data/id)}))

;; ## Tests for individual syncing functions

;; DESCRIBE-DATABASE
(expect-when-testing-mongo
(datasets/expect-with-engine :mongo
{:tables #{{:schema nil, :name "checkins"}
{:schema nil, :name "categories"}
{:schema nil, :name "users"}
{:schema nil, :name "venues"}}}
(driver/describe-database (MongoDriver.) (mongo-db)))
(driver/describe-database (MongoDriver.) (data/db)))

;; DESCRIBE-TABLE
(expect-when-testing-mongo
(datasets/expect-with-engine :mongo
{:schema nil
:name "venues"
:fields #{{:name "name",
:fields #{{:name "name"
:base-type :type/Text}
{:name "latitude",
{:name "latitude"
:base-type :type/Float}
{:name "longitude",
{:name "longitude"
:base-type :type/Float}
{:name "price",
{:name "price"
:base-type :type/Integer}
{:name "category_id",
{:name "category_id"
:base-type :type/Integer}
{:name "_id",
:base-type :type/Integer,
{:name "_id"
:base-type :type/Integer
:pk? true}}}
(driver/describe-table (MongoDriver.) (mongo-db) (table-name->table :venues)))
(driver/describe-table (MongoDriver.) (data/db) (Table (data/id :venues))))

;; ANALYZE-TABLE
(expect-when-testing-mongo
(let [field-name->field (->> (table/fields (table-name->table :venues))
(group-by :name)
clojure.walk/keywordize-keys)
field-id #(:id (first (% field-name->field)))]
{:row_count 100,
:fields [{:id (field-id :category_id) :values [2 3 4 5 6 7 10 11 12 13 14 15 18 19 20 29 40 43 44 46 48 49 50 58 64 67 71 74]}
{:id (field-id :name), :values nil}
{:id (field-id :price), :values [1 2 3 4]}]})
(let [venues-table (table-name->table :venues)]
(datasets/expect-with-engine :mongo
{:row_count 100
:fields [{:id (data/id :venues :category_id) :values [2 3 4 5 6 7 10 11 12 13 14 15 18 19 20 29 40 43 44 46 48 49 50 58 64 67 71 74]}
{:id (data/id :venues :name), :values (db/select-one-field :values FieldValues, :field_id (data/id :venues :name))}
{:id (data/id :venues :price), :values [1 2 3 4]}]}
(let [venues-table (Table (data/id :venues))]
(driver/analyze-table (MongoDriver.) venues-table (set (mapv :id (table/fields venues-table))))))

;; ## Big-picture tests for the way data should look post-sync

;; Test that Tables got synced correctly, and row counts are correct
(expect-when-testing-mongo
[{:rows 75, :active true, :name "categories"}
{:rows 1000, :active true, :name "checkins"}
{:rows 15, :active true, :name "users"}
{:rows 100, :active true, :name "venues"}]
(datasets/expect-with-engine :mongo
[{:rows 75, :active true, :name "categories"}
{:rows 1000, :active true, :name "checkins"}
{:rows 15, :active true, :name "users"}
{:rows 100, :active true, :name "venues"}]
(for [field (db/select [Table :name :active :rows]
:db_id (:id (mongo-db))
:db_id (data/id)
{:order-by [:name]})]
(into {} field)))

;; Test that Fields got synced correctly, and types are correct
(expect-when-testing-mongo
[[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type nil, :base_type :type/DateTime, :name "date"}
{:special_type :type/Category, :base_type :type/Integer, :name "user_id"}
{:special_type nil, :base_type :type/Integer, :name "venue_id"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type nil, :base_type :type/DateTime, :name "last_login"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}
{:special_type :type/Category, :base_type :type/Text, :name "password"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type :type/Category, :base_type :type/Integer, :name "category_id"}
{:special_type :type/Latitude, :base_type :type/Float, :name "latitude"}
{:special_type :type/Longitude, :base_type :type/Float, :name "longitude"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}
{:special_type :type/Category, :base_type :type/Integer, :name "price"}]]
(for [nm table-names]
(for [field (db/select [Field :name :base_type :special_type]
:active true
:table_id (:id (table-name->table nm))
{:order-by [:name]})]
(into {} field))))
(datasets/expect-with-engine :mongo
[[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type nil, :base_type :type/DateTime, :name "date"}
{:special_type :type/Category, :base_type :type/Integer, :name "user_id"}
{:special_type :type/Category, :base_type :type/Integer, :name "venue_id"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type nil, :base_type :type/DateTime, :name "last_login"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}
{:special_type :type/Category, :base_type :type/Text, :name "password"}]
[{:special_type :type/PK, :base_type :type/Integer, :name "_id"}
{:special_type :type/Category, :base_type :type/Integer, :name "category_id"}
{:special_type :type/Latitude, :base_type :type/Float, :name "latitude"}
{:special_type :type/Longitude, :base_type :type/Float, :name "longitude"}
{:special_type :type/Name, :base_type :type/Text, :name "name"}
{:special_type :type/Category, :base_type :type/Integer, :name "price"}]]
(vec (for [table-name table-names]
(vec (for [field (db/select [Field :name :base_type :special_type]
:active true
:table_id (data/id table-name)
{:order-by [:name]})]
(into {} field))))))


;;; Check that we support Mongo BSON ID and can filter by it (#1367)
Expand Down Expand Up @@ -210,4 +197,4 @@
(count (rows (qp/process-query {:native {:query "[{\"$match\": {\"date\": {\"$gte\": ISODate(\"2015-12-20\")}}}]"
:collection "checkins"}
:type :native
:database (:id (mongo-db))}))))
:database (data/id)}))))
5 changes: 3 additions & 2 deletions test/metabase/query_processor_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,9 @@
{:target_table_id (data/id :venues)}
{})
:target (target-field (venues-col :id))
:special_type (when (data/fks-supported?)
:type/FK)
:special_type (if (data/fks-supported?)
:type/FK
:type/Category)
:base_type (data/expected-base-type->actual :type/Integer)
:name (data/format-name "venue_id")
:display_name "Venue ID"}
Expand Down
23 changes: 9 additions & 14 deletions test/metabase/test/data/datasets.clj
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,13 @@
(with-engine ~engine
~@body)))

(defmacro expect-when-testing-engine
"Generate a unit test that only runs if we're currently testing against ENGINE."
[engine expected actual]
`(when-testing-engine ~engine
(expect ~expected
~actual)))

(defmacro expect-with-engine
"Generate a unit test that only runs if we're currently testing against ENGINE, and that binds `*driver*` to the current dataset."
"Generate a unit test that only runs if we're currently testing against ENGINE, and that binds `*driver*` to the driver for ENGINE."
[engine expected actual]
`(expect-when-testing-engine ~engine
(with-engine ~engine ~expected)
(with-engine ~engine ~actual)))
`(when-testing-engine ~engine
(expect
(with-engine ~engine ~expected)
(with-engine ~engine ~actual))))

(defmacro expect-with-engines
"Generate unit tests for all datasets in ENGINES; each test will only run if we're currently testing the corresponding dataset.
Expand All @@ -118,9 +112,10 @@
`(let [~e (fn [] ~expected)
~a (fn [] ~actual)]
~@(for [engine (eval engines)]
`(expect-when-testing-engine ~engine
(do-with-engine ~engine ~e)
(do-with-engine ~engine ~a))))))
`(when-testing-engine ~engine
(expect
(do-with-engine ~engine ~e)
(do-with-engine ~engine ~a)))))))

(defmacro expect-with-all-engines
"Generate unit tests for all valid datasets; each test will only run if we're currently testing the corresponding dataset.
Expand Down

0 comments on commit ef17e31

Please sign in to comment.