diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index ea52f7f9cabcf5ed37b25053f7411be1a774e186..3237ce2a763d7cf95bb4c5cccd2be18a6a5d7433 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -163,8 +163,7 @@ toucan.models/primary-key {:message "Use metabase.db.util/primary-key instead of toucan.models/primary-key"} toucan.models/model? {:message "Use mdb.u/toucan-model? instead of toucan.models/model?"} toucan.util.test/with-temp {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp"} - ;; toucan.util.test/with-temp* {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp*"} - } + toucan.util.test/with-temp* {:message "Use t2.with-temp/with-temp instead of toucan.util.test/with-temp*"}} :discouraged-namespace {camel-snake-kebab.core {:message "CSK is not Turkish-safe, use the versions in metabase.util instead."} diff --git a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj index 33de5b1ba79c72ed2f51c8493fd2b6bfcc6166ee..4ce30b18afbfdc6cb2494972b5af5aba9dcd42c5 100644 --- a/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj +++ b/enterprise/backend/test/metabase_enterprise/serialization/test_util.clj @@ -48,6 +48,7 @@ "Wraps with-temp*, but binding `*allow-deleting-personal-collections*` to true so that temporary personal collections can still be deleted." [model-bindings & body] + #_{:clj-kondo/ignore [:discouraged-var]} `(binding [collection/*allow-deleting-personal-collections* true] (tt/with-temp* ~model-bindings ~@body))) diff --git a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj index 8212b94f8b6c9e5191f95e6a63a0af46772a6e42..7781ecce76dd3a0e944581bf6bb930a344a9b9e5 100644 --- a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj +++ b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj @@ -21,7 +21,7 @@ [metabase.test.util.timezone :as test.tz] [metabase.util :as u] [metabase.util.honeysql-extensions :as hx] - [toucan.util.test :as tt])) + [toucan2.tools.with-temp :as t2.with-temp])) (deftest native-query-test (mt/test-driver :bigquery-cloud-sdk @@ -153,23 +153,23 @@ (native-timestamp-query (mt/id) "2018-08-31 00:00:00" "UTC")) "A UTC date is returned, we should read/return it as UTC") - (is (= "2018-08-31T00:00:00-05:00" - (test.tz/with-system-timezone-id "America/Chicago" - (tt/with-temp* [Database [db {:engine :bigquery-cloud-sdk - :details (assoc (:details (mt/db)) - :use-jvm-timezone true)}]] - (native-timestamp-query db "2018-08-31 00:00:00-05" "America/Chicago")))) - (str "This test includes a `use-jvm-timezone` flag of true that will assume that the date coming from BigQuery " - "is already in the JVM's timezone. The test puts the JVM's timezone into America/Chicago an ensures that " - "the correct date is compared")) - - (is (= "2018-08-31T00:00:00+07:00" - (test.tz/with-system-timezone-id "Asia/Jakarta" - (tt/with-temp* [Database [db {:engine :bigquery-cloud-sdk - :details (assoc (:details (mt/db)) - :use-jvm-timezone true)}]] - (native-timestamp-query db "2018-08-31 00:00:00+07" "Asia/Jakarta")))) - "Similar to the above test, but covers a positive offset"))) + (test.tz/with-system-timezone-id "America/Chicago" + (t2.with-temp/with-temp [Database db {:engine :bigquery-cloud-sdk + :details (assoc (:details (mt/db)) + :use-jvm-timezone true)}] + (is (= "2018-08-31T00:00:00-05:00" + (native-timestamp-query db "2018-08-31 00:00:00-05" "America/Chicago")) + (str "This test includes a `use-jvm-timezone` flag of true that will assume that the date coming from BigQuery " + "is already in the JVM's timezone. The test puts the JVM's timezone into America/Chicago an ensures that " + "the correct date is compared")))) + + (test.tz/with-system-timezone-id "Asia/Jakarta" + (t2.with-temp/with-temp [Database db {:engine :bigquery-cloud-sdk + :details (assoc (:details (mt/db)) + :use-jvm-timezone true)}] + (is (= "2018-08-31T00:00:00+07:00" + (native-timestamp-query db "2018-08-31 00:00:00+07" "Asia/Jakarta")) + "Similar to the above test, but covers a positive offset"))))) ;; if I run a BigQuery query, does it get a remark added to it? (defn- query->native [query] @@ -211,25 +211,25 @@ " `v3_test_data.venues`.`name` AS `name` " "FROM `v3_test_data.venues` " "LIMIT 1") - (tt/with-temp* [Database [db {:engine :bigquery-cloud-sdk - :details (assoc (:details (mt/db)) - :include-user-id-and-hash false)}] - Table [table {:name "venues" - :db_id (u/the-id db) - :schema (get-in db [:details :dataset-filters-patterns])}] - Field [_ {:table_id (u/the-id table) - :name "id" - :base_type "type/Integer"}] - Field [_ {:table_id (u/the-id table) - :name "name" - :base_type "type/Text"}]] + (t2.with-temp/with-temp [Database db {:engine :bigquery-cloud-sdk + :details (assoc (:details (mt/db)) + :include-user-id-and-hash false)} + Table table {:name "venues" + :db_id (u/the-id db) + :schema (get-in db [:details :dataset-filters-patterns])} + Field _ {:table_id (u/the-id table) + :name "id" + :base_type "type/Integer"} + Field _ {:table_id (u/the-id table) + :name "name" + :base_type "type/Text"}] (query->native - {:database (u/the-id db) - :type :query - :query {:source-table (u/the-id table) - :limit 1} - :info {:executed-by 1000 - :query-hash (byte-array [1 2 3 4])}})))))) + {:database (u/the-id db) + :type :query + :query {:source-table (u/the-id table) + :limit 1} + :info {:executed-by 1000 + :query-hash (byte-array [1 2 3 4])}})))))) (deftest unprepare-params-test (mt/test-driver :bigquery-cloud-sdk @@ -282,9 +282,9 @@ (deftest reconcile-temporal-types-test (mt/with-everything-store - (tt/with-temp* [Field [date-field {:name "date", :base_type :type/Date, :database_type "date"}] - Field [datetime-field {:name "datetime", :base_type :type/DateTime, :database_type "datetime"}] - Field [timestamp-field {:name "timestamp", :base_type :type/DateTimeWithLocalTZ, :database_type "timestamp"}]] + (t2.with-temp/with-temp [Field date-field {:name "date", :base_type :type/Date, :database_type "date"} + Field datetime-field {:name "datetime", :base_type :type/DateTime, :database_type "datetime"} + Field timestamp-field {:name "timestamp", :base_type :type/DateTimeWithLocalTZ, :database_type "timestamp"}] (binding [*print-meta* true] (let [fields {:date date-field :datetime datetime-field diff --git a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj index ad572ce5765e7ce45e03847eac12f904ae232b95..8eb5e5dc3ebc5a81cdaa3919b3cdb534c57bca09 100644 --- a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj +++ b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj @@ -33,7 +33,6 @@ #_{:clj-kondo/ignore [:discouraged-namespace]} [metabase.util.honeysql-extensions :as hx] [metabase.util.log :as log] - [toucan.util.test :as tt] [toucan2.core :as t2] [toucan2.tools.with-temp :as t2.with-temp]) (:import @@ -287,17 +286,17 @@ (execute! "CREATE TABLE \"%s\".\"messages\" (\"id\" %s, \"message\" CLOB)" username pk-type) (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (1, 'Hello')" username) (execute! "INSERT INTO \"%s\".\"messages\" (\"id\", \"message\") VALUES (2, NULL)" username) - (tt/with-temp* [Table [table {:schema username, :name "messages", :db_id (mt/id)}] - Field [id-field {:table_id (u/the-id table), :name "id", :base_type "type/Integer"}] - Field [_ {:table_id (u/the-id table), :name "message", :base_type "type/Text"}]] + (t2.with-temp/with-temp [Table table {:schema username, :name "messages", :db_id (mt/id)} + Field id-field {:table_id (u/the-id table), :name "id", :base_type "type/Integer"} + Field _ {:table_id (u/the-id table), :name "message", :base_type "type/Text"}] (is (= [[1M "Hello"] [2M nil]] (qp.test/rows - (qp/process-query - {:database (mt/id) - :type :query - :query {:source-table (u/the-id table) - :order-by [[:asc [:field (u/the-id id-field) nil]]]}})))))))))) + (qp/process-query + {:database (mt/id) + :type :query + :query {:source-table (u/the-id table) + :order-by [[:asc [:field (u/the-id id-field) nil]]]}})))))))))) (deftest handle-slashes-test (mt/test-driver :oracle diff --git a/test/metabase/analytics/stats_test.clj b/test/metabase/analytics/stats_test.clj index 22b2642f3c43d73133013e3faf77d778cccc43fe..4f0b1d9154905d10f2542cfb1e6fd2a26e42f3bd 100644 --- a/test/metabase/analytics/stats_test.clj +++ b/test/metabase/analytics/stats_test.clj @@ -14,12 +14,12 @@ [metabase.util :as u] [metabase.util.schema :as su] [schema.core :as s] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (use-fixtures :once (fixtures/initialize :db)) -(deftest bin-small-number-test +(deftest ^:parallel bin-small-number-test (are [expected n] (= expected (#'stats/bin-small-number n)) "0" 0 @@ -32,7 +32,7 @@ "25+" 26 "25+" 500)) -(deftest bin-medium-number-test +(deftest ^:parallel bin-medium-number-test (are [expected n] (= expected (#'stats/bin-medium-number n)) "0" 0 @@ -51,7 +51,7 @@ "250+" 251 "250+" 5000)) -(deftest bin-large-number-test +(deftest ^:parallel bin-large-number-test (are [expected n] (= expected (#'stats/bin-large-number n)) "0" 0 @@ -91,7 +91,7 @@ (is (schema= DBMSVersionStats (-> stats :stats :database :dbms_versions))))))) -(deftest conversion-test +(deftest ^:parallel conversion-test (is (= #{true} (let [system-stats (get-in (anonymous-usage-stats) [:stats :system])] (into #{} (map #(contains? system-stats %) [:java_version :java_runtime_name :max_memory])))) @@ -110,7 +110,7 @@ :num_by_latency (frequencies (for [{latency :running_time} executions] (#'stats/bin-large-number (/ latency 1000))))})) -(deftest new-impl-test +(deftest ^:parallel new-impl-test (is (= (old-execution-metrics) (#'stats/execution-metrics)) "the new lazy-seq version of the executions metrics works the same way the old one did")) @@ -126,54 +126,54 @@ ;; alert_first_only boolean, -- True if the alert should be disabled after the first notification ;; alert_above_goal boolean, -- For a goal condition, alert when above the goal (deftest pulses-and-alerts-test - (tt/with-temp* [Card [c] - ;; ---------- Pulses ---------- - Pulse [p1] - Pulse [p2] - Pulse [p3] - PulseChannel [_ {:pulse_id (u/the-id p1), :schedule_type "daily", :channel_type "email"}] - PulseChannel [_ {:pulse_id (u/the-id p1), :schedule_type "weekly", :channel_type "email"}] - PulseChannel [_ {:pulse_id (u/the-id p2), :schedule_type "daily", :channel_type "slack"}] - ;; Pulse 1 gets 2 Cards (1 CSV) - PulseCard [_ {:pulse_id (u/the-id p1), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id p1), :card_id (u/the-id c), :include_csv true}] - ;; Pulse 2 gets 1 Card - PulseCard [_ {:pulse_id (u/the-id p1), :card_id (u/the-id c)}] - ;; Pulse 3 gets 7 Cards (1 CSV, 2 XLS, 2 BOTH) - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true, :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true, :include_xls true}] - ;; ---------- Alerts ---------- - Pulse [a1 {:alert_condition "rows", :alert_first_only false}] - Pulse [a2 {:alert_condition "rows", :alert_first_only true}] - Pulse [a3 {:alert_condition "goal", :alert_first_only false}] - Pulse [_ {:alert_condition "goal", :alert_first_only false, :alert_above_goal true}] - ;; Alert 1 is Email, Alert 2 is Email & Slack, Alert 3 is Slack-only - PulseChannel [_ {:pulse_id (u/the-id a1), :channel_type "email"}] - PulseChannel [_ {:pulse_id (u/the-id a1), :channel_type "email"}] - PulseChannel [_ {:pulse_id (u/the-id a2), :channel_type "slack"}] - PulseChannel [_ {:pulse_id (u/the-id a3), :channel_type "slack"}] - ;; Alert 1 gets 2 Cards (1 CSV) - PulseCard [_ {:pulse_id (u/the-id a1), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id a1), :card_id (u/the-id c), :include_csv true}] - ;; Alert 2 gets 1 Card - PulseCard [_ {:pulse_id (u/the-id a1), :card_id (u/the-id c)}] - ;; Alert 3 gets 7 Cards (1 CSV, 2 XLS, 2 BOTH) - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true, :include_xls true}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true, :include_xls true}] - ;; Alert 4 gets 3 Cards - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}] - PulseCard [_ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}]] + (t2.with-temp/with-temp [Card c {} + ;; ---------- Pulses ---------- + Pulse p1 {} + Pulse p2 {} + Pulse p3 {} + PulseChannel _ {:pulse_id (u/the-id p1), :schedule_type "daily", :channel_type "email"} + PulseChannel _ {:pulse_id (u/the-id p1), :schedule_type "weekly", :channel_type "email"} + PulseChannel _ {:pulse_id (u/the-id p2), :schedule_type "daily", :channel_type "slack"} + ;; Pulse 1 gets 2 Cards (1 CSV) + PulseCard _ {:pulse_id (u/the-id p1), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id p1), :card_id (u/the-id c), :include_csv true} + ;; Pulse 2 gets 1 Card + PulseCard _ {:pulse_id (u/the-id p1), :card_id (u/the-id c)} + ;; Pulse 3 gets 7 Cards (1 CSV, 2 XLS, 2 BOTH) + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_xls true} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_xls true} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true, :include_xls true} + PulseCard _ {:pulse_id (u/the-id p3), :card_id (u/the-id c), :include_csv true, :include_xls true} + ;; ---------- Alerts ---------- + Pulse a1 {:alert_condition "rows", :alert_first_only false} + Pulse a2 {:alert_condition "rows", :alert_first_only true} + Pulse a3 {:alert_condition "goal", :alert_first_only false} + Pulse _ {:alert_condition "goal", :alert_first_only false, :alert_above_goal true} + ;; Alert 1 is Email, Alert 2 is Email & Slack, Alert 3 is Slack-only + PulseChannel _ {:pulse_id (u/the-id a1), :channel_type "email"} + PulseChannel _ {:pulse_id (u/the-id a1), :channel_type "email"} + PulseChannel _ {:pulse_id (u/the-id a2), :channel_type "slack"} + PulseChannel _ {:pulse_id (u/the-id a3), :channel_type "slack"} + ;; Alert 1 gets 2 Cards (1 CSV) + PulseCard _ {:pulse_id (u/the-id a1), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id a1), :card_id (u/the-id c), :include_csv true} + ;; Alert 2 gets 1 Card + PulseCard _ {:pulse_id (u/the-id a1), :card_id (u/the-id c)} + ;; Alert 3 gets 7 Cards (1 CSV, 2 XLS, 2 BOTH) + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_xls true} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_xls true} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true, :include_xls true} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c), :include_csv true, :include_xls true} + ;; Alert 4 gets 3 Cards + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c)} + PulseCard _ {:pulse_id (u/the-id a3), :card_id (u/the-id c)}] (letfn [(>= [n] (s/pred #(clojure.core/>= % n) (format ">= %s" n)))] (is (schema= {:pulses (>= 3) diff --git a/test/metabase/api/automagic_dashboards_test.clj b/test/metabase/api/automagic_dashboards_test.clj index a1111210d022741f6995207f520217c13fe31426..799596f2b5dfef6d7e32a8b5e00d6782439a9ccc 100644 --- a/test/metabase/api/automagic_dashboards_test.clj +++ b/test/metabase/api/automagic_dashboards_test.clj @@ -15,7 +15,6 @@ [metabase.transforms.materialize :as tf.materialize] [metabase.transforms.specs :as tf.specs] [metabase.util :as u] - [toucan.util.test :as tt] [toucan2.tools.with-temp :as t2.with-temp])) (use-fixtures :once (fixtures/initialize :db :web-server :test-users :test-users-personal-collections)) @@ -112,13 +111,13 @@ (is (some? (api-call "question/%s/cell/%s/rule/example/indepth" [card-id cell-query] #(revoke-collection-permissions! collection-id))))))]] - (tt/with-temp* [Collection [{collection-id :id}] - Card [{card-id :id} {:table_id (mt/id :venues) - :collection_id collection-id - :dataset_query (mt/mbql-query venues - {:filter [:> $price 10]})}]] - (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-id) - (test-fn collection-id card-id)))))) + (t2.with-temp/with-temp [Collection {collection-id :id} {} + Card {card-id :id} {:table_id (mt/id :venues) + :collection_id collection-id + :dataset_query (mt/mbql-query venues + {:filter [:> $price 10]})}] + (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-id) + (test-fn collection-id card-id)))))) (deftest model-xray-test @@ -142,14 +141,14 @@ (is (some? (api-call "model/%s/cell/%s/rule/example/indepth" [card-id cell-query] #(revoke-collection-permissions! collection-id))))))]] - (tt/with-temp* [Collection [{collection-id :id}] - Card [{card-id :id} {:table_id (mt/id :venues) - :collection_id collection-id - :dataset_query (mt/mbql-query venues - {:filter [:> $price 10]}) - :dataset true}]] - (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-id) - (test-fn collection-id card-id))))))) + (t2.with-temp/with-temp [Collection {collection-id :id} {} + Card {card-id :id} {:table_id (mt/id :venues) + :collection_id collection-id + :dataset_query (mt/mbql-query venues + {:filter [:> $price 10]}) + :dataset true}] + (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection-id) + (test-fn collection-id card-id))))))) (deftest adhoc-query-xray-test (let [query (#'magic/encode-base64-json diff --git a/test/metabase/api/native_query_snippet_test.clj b/test/metabase/api/native_query_snippet_test.clj index df220b5b6ec4e1409bc6cb221f116e3923747072..5bf1ee6a34c70e4415cbbe3449c650cd29243e2b 100644 --- a/test/metabase/api/native_query_snippet_test.clj +++ b/test/metabase/api/native_query_snippet_test.clj @@ -11,7 +11,6 @@ [metabase.util :as u] [metabase.util.schema :as su] [schema.core :as s] - [toucan.util.test :as tt] [toucan2.core :as t2] [toucan2.tools.with-temp :as t2.with-temp])) @@ -189,8 +188,8 @@ (grant-native-perms) (testing "PUT /api/native-query-snippet/:id" (testing "\nChange collection_id" - (tt/with-temp* [Collection [collection-1 {:name "a Collection", :namespace "snippets"}] - Collection [collection-2 {:name "another Collection", :namespace "snippets"}]] + (t2.with-temp/with-temp [Collection collection-1 {:name "a Collection", :namespace "snippets"} + Collection collection-2 {:name "another Collection", :namespace "snippets"}] (let [no-collection {:name "no Collection"}] (doseq [[source dest] [[no-collection collection-1] [collection-1 collection-2] @@ -206,8 +205,8 @@ (t2/select-one-fn :collection_id NativeQuerySnippet :id snippet-id))))))))) (testing "\nShould throw an error if you try to move it to a Collection not in the 'snippets' namespace" - (tt/with-temp* [Collection [{collection-id :id}] - NativeQuerySnippet [{snippet-id :id}]] + (t2.with-temp/with-temp [Collection {collection-id :id} {} + NativeQuerySnippet {snippet-id :id} {}] (is (= {:errors {:collection_id "A NativeQuerySnippet can only go in Collections in the :snippets namespace."} :allowed-namespaces ["snippets"] :collection-namespace nil} diff --git a/test/metabase/api/revision_test.clj b/test/metabase/api/revision_test.clj index ba93e1013588b78e7d91f2794a56da738d0b59a3..bea78d66d687948b0264f69b7fe7c5746a36ecf8 100644 --- a/test/metabase/api/revision_test.clj +++ b/test/metabase/api/revision_test.clj @@ -10,7 +10,6 @@ [metabase.test.data.users :as test.users] [metabase.test.fixtures :as fixtures] [metabase.util :as u] - [toucan.util.test :as tt] [toucan2.core :as t2] [toucan2.tools.with-temp :as t2.with-temp])) @@ -118,8 +117,8 @@ (deftest revert-test (testing "Reverting through API works" - (tt/with-temp* [Dashboard [{:keys [id] :as dash}] - Card [{card-id :id, :as card}]] + (t2.with-temp/with-temp [Dashboard {:keys [id] :as dash} {} + Card {card-id :id, :as card} {}] (is (=? {:id id} (create-dashboard-revision! (:id dash) true :rasta))) (let [dashcard (first (t2/insert-returning-instances! DashboardCard diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj index 87f952e22004cdf23d5ce3b75ead5c5adb582027..bd1bd10bb67dec72b4486b33e77a78ac81ea3523 100644 --- a/test/metabase/models/card_test.clj +++ b/test/metabase/models/card_test.clj @@ -19,17 +19,17 @@ (deftest dashboard-count-test (testing "Check that the :dashboard_count delay returns the correct count of Dashboards a Card is in" - (tt/with-temp* [:model/Card [{card-id :id}] - Dashboard [dash-1] - Dashboard [dash-2]] + (t2.with-temp/with-temp [:model/Card {card-id :id} {} + Dashboard dash-1 {} + Dashboard dash-2 {}] (letfn [(add-card-to-dash! [dash] (t2/insert! DashboardCard - {:card_id card-id - :dashboard_id (u/the-id dash) - :row 0 - :col 0 - :size_x 4 - :size_y 4})) + {:card_id card-id + :dashboard_id (u/the-id dash) + :row 0 + :col 0 + :size_x 4 + :size_y 4})) (get-dashboard-count [] (card/dashboard-count (t2/select-one :model/Card :id card-id)))] (is (= 0 @@ -57,21 +57,21 @@ (t2.with-temp/with-temp [:model/Card card] (is (zero? (hydrated-count card))))) (testing "With one" - (tt/with-temp* [:model/Card [{card-id :id :as card}] - Dashboard [_ {:parameters [(card-params card-id)]}]] + (t2.with-temp/with-temp [:model/Card {card-id :id :as card} {} + Dashboard _ {:parameters [(card-params card-id)]}] (is (= 1 (hydrated-count card))))) (testing "With several" - (tt/with-temp* [:model/Card [{card-id :id :as card}] - Dashboard [_ {:parameters [(card-params card-id)]}] - Dashboard [_ {:parameters [(card-params card-id)]}] - Dashboard [_ {:parameters [(card-params card-id)]}]] + (t2.with-temp/with-temp [:model/Card {card-id :id :as card} {} + Dashboard _ {:parameters [(card-params card-id)]} + Dashboard _ {:parameters [(card-params card-id)]} + Dashboard _ {:parameters [(card-params card-id)]}] (is (= 3 (hydrated-count card))))))) (deftest remove-from-dashboards-when-archiving-test (testing "Test that when somebody archives a Card, it is removed from any Dashboards it belongs to" - (tt/with-temp* [Dashboard [dashboard] - :model/Card [card] - DashboardCard [_dashcard {:dashboard_id (u/the-id dashboard), :card_id (u/the-id card)}]] + (t2.with-temp/with-temp [Dashboard dashboard {} + :model/Card card {} + DashboardCard _dashcard {:dashboard_id (u/the-id dashboard), :card_id (u/the-id card)}] (t2/update! :model/Card (u/the-id card) {:archived true}) (is (= 0 (t2/count DashboardCard :dashboard_id (u/the-id dashboard))))))) @@ -191,24 +191,24 @@ (is (thrown? Exception (t2/update! :model/Card (u/the-id card) - (card-with-source-table (str "card__" (u/the-id card)))))))) + (card-with-source-table (str "card__" (u/the-id card)))))))) (testing "Do the same stuff with circular reference between two Cards... (A -> B -> A)" - (tt/with-temp* [:model/Card [card-a (card-with-source-table (mt/id :venues))] - :model/Card [card-b (card-with-source-table (str "card__" (u/the-id card-a)))]] + (t2.with-temp/with-temp [:model/Card card-a (card-with-source-table (mt/id :venues)) + :model/Card card-b (card-with-source-table (str "card__" (u/the-id card-a)))] (is (thrown? Exception (t2/update! :model/Card (u/the-id card-a) - (card-with-source-table (str "card__" (u/the-id card-b)))))))) + (card-with-source-table (str "card__" (u/the-id card-b)))))))) (testing "ok now try it with A -> C -> B -> A" - (tt/with-temp* [:model/Card [card-a (card-with-source-table (mt/id :venues))] - :model/Card [card-b (card-with-source-table (str "card__" (u/the-id card-a)))] - :model/Card [card-c (card-with-source-table (str "card__" (u/the-id card-b)))]] + (t2.with-temp/with-temp [:model/Card card-a (card-with-source-table (mt/id :venues)) + :model/Card card-b (card-with-source-table (str "card__" (u/the-id card-a))) + :model/Card card-c (card-with-source-table (str "card__" (u/the-id card-b)))] (is (thrown? Exception (t2/update! :model/Card (u/the-id card-a) - (card-with-source-table (str "card__" (u/the-id card-c))))))))) + (card-with-source-table (str "card__" (u/the-id card-c))))))))) (deftest validate-collection-namespace-test (mt/with-temp Collection [{collection-id :id} {:namespace "currency"}] @@ -438,12 +438,11 @@ :id "_CATEGORY_NAME_" :type "category"}] (testing "parameter with source is card create ParameterCard" - (tt/with-temp* [:model/Card [{source-card-id-1 :id}] - :model/Card [{source-card-id-2 :id}] - :model/Card [{card-id :id} - {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id source-card-id-1}})]}]] + (t2.with-temp/with-temp [:model/Card {source-card-id-1 :id} {} + :model/Card {source-card-id-2 :id} {} + :model/Card {card-id :id} {:parameters [(merge default-params + {:values_source_type "card" + :values_source_config {:card_id source-card-id-1}})]}] (is (=? [{:card_id source-card-id-1 :parameterized_object_type :card :parameterized_object_id card-id @@ -452,8 +451,8 @@ (testing "update values_source_config.card_id will update ParameterCard" (t2/update! :model/Card card-id {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id source-card-id-2}})]}) + {:values_source_type "card" + :values_source_config {:card_id source-card-id-2}})]}) (is (=? [{:card_id source-card-id-2 :parameterized_object_type :card :parameterized_object_id card-id @@ -466,15 +465,13 @@ (t2/select 'ParameterCard :parameterized_object_type "card" :parameterized_object_id card-id)))))) (testing "Delete a card will delete any ParameterCard that linked to it" - (tt/with-temp* [:model/Card [{source-card-id :id}] - :model/Card [{card-id-1 :id} - {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id source-card-id}})]}] - :model/Card [{card-id-2 :id} - {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id source-card-id}})]}]] + (t2.with-temp/with-temp [:model/Card {source-card-id :id} {} + :model/Card {card-id-1 :id} {:parameters [(merge default-params + {:values_source_type "card" + :values_source_config {:card_id source-card-id}})]} + :model/Card {card-id-2 :id} {:parameters [(merge default-params + {:values_source_type "card" + :values_source_config {:card_id source-card-id}})]}] ;; makes sure we have ParameterCard to start with (is (=? [{:card_id source-card-id :parameterized_object_type :card diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj index c3150c3f493daca187b648f128ad831d47d7938f..8f7c6dbcd30bd8a00c21b66df37d32312d36abac 100644 --- a/test/metabase/models/dashboard_test.clj +++ b/test/metabase/models/dashboard_test.clj @@ -31,13 +31,13 @@ (deftest serialize-dashboard-test (testing "without tabs" - (tt/with-temp* [Dashboard [{dashboard-id :id :as dashboard} {:name "Test Dashboard"}] - Card [{card-id :id}] - Card [{series-id-1 :id}] - Card [{series-id-2 :id}] - DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id, :card_id card-id}] - DashboardCardSeries [_ {:dashboardcard_id dashcard-id, :card_id series-id-1, :position 0}] - DashboardCardSeries [_ {:dashboardcard_id dashcard-id, :card_id series-id-2, :position 1}]] + (t2.with-temp/with-temp [Dashboard {dashboard-id :id :as dashboard} {:name "Test Dashboard"} + Card {card-id :id} {} + Card {series-id-1 :id} {} + Card {series-id-2 :id} {} + DashboardCard {dashcard-id :id} {:dashboard_id dashboard-id, :card_id card-id} + DashboardCardSeries _ {:dashboardcard_id dashcard-id, :card_id series-id-1, :position 0} + DashboardCardSeries _ {:dashboardcard_id dashcard-id, :card_id series-id-2, :position 1}] (is (= {:name "Test Dashboard" :auto_apply_filters true :collection_id nil @@ -58,12 +58,13 @@ [(assoc card :id (= dashcard-id id) :card_id (= card-id card_id) - :series (= [series-id-1 series-id-2] series))])))))) + :series (= [series-id-1 series-id-2] series))]))))))) +(deftest serialize-dashboard-with-tabs-test (testing "with tabs" - (mt/with-temp* [Dashboard [{dashboard-id :id :as dashboard} {:name "Test Dashboard"}] - :model/DashboardTab [{tab-id :id} {:dashboard_id dashboard-id :name "Test Tab" :position 0}] - DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id :dashboard_tab_id tab-id}]] + (t2.with-temp/with-temp [Dashboard {dashboard-id :id :as dashboard} {:name "Test Dashboard"} + :model/DashboardTab {tab-id :id} {:dashboard_id dashboard-id :name "Test Tab" :position 0} + DashboardCard {dashcard-id :id} {:dashboard_id dashboard-id :dashboard_tab_id tab-id}] (is (=? {:name "Test Dashboard" :auto_apply_filters true :collection_id nil @@ -78,174 +79,148 @@ :tabs [{:id tab-id :name "Test Tab" :position 0}]} - (revision/serialize-instance Dashboard (:id dashboard) dashboard)))))) + (revision/serialize-instance Dashboard (:id dashboard) dashboard)))))) (deftest diff-dashboards-str-test (testing "update general info ---" - (is (= "added a description and renamed it from \"Diff Test\" to \"Diff Test Changed\"." - (build-sentence - (revision/diff-strings - Dashboard - {:name "Diff Test" - :description nil - :cards []} - {:name "Diff Test Changed" - :description "foobar" - :cards []})))) - - (is (= "renamed it from \"Apple\" to \"Next\" and modified the cards." - (build-sentence - (revision/diff-strings - Dashboard - {:name "Apple" - :cards [{:id 1} {:id 2}]} - {:name "Next" - :cards [{:id 1} {:id 3}]})))) - - (is (= "set auto apply filters to false." - (build-sentence - (revision/diff-strings - Dashboard - {:name "Diff Test" - :auto_apply_filters true} - {:name "Diff Test" - :auto_apply_filters false})))) - - ;; multiple changes - (is (= "changed the cache ttl from \"333\" to \"1,227\", rearranged the cards, modified the series on card 1 and added some series to card 2." - (build-sentence - (revision/diff-strings - Dashboard - {:name "Diff Test" - :description nil - :cache_ttl 333 - :cards [{:size_x 4 - :size_y 4 - :row 0 - :col 0 - :id 1 - :card_id 1 - :series [5 6]} - {:size_x 4 - :size_y 4 - :row 0 - :col 0 - :id 2 - :card_id 2 - :series []}]} - {:name "Diff Test" - :description nil - :cache_ttl 1227 - :cards [{:size_x 4 - :size_y 4 - :row 0 - :col 0 - :id 1 - :card_id 1 - :series [4 5]} - {:size_x 4 - :size_y 4 - :row 2 - :col 0 - :id 2 - :card_id 2 - :series [3 4 5]}]}))))) - - (testing "update cards ---" - (is (= "added a card." - (build-sentence - (revision/diff-strings - Dashboard - {:cards [{:id 1} {:id 2}]} - {:cards [{:id 1} {:id 2} {:id 3}]})))) - - (is (= "removed a card." - (build-sentence - (revision/diff-strings - Dashboard - {:cards [{:id 1} {:id 2}]} - {:cards [{:id 1}]})))) - - (is (= "rearranged the cards." - (build-sentence - (revision/diff-strings - Dashboard - {:cards [{:id 1 :row 0} {:id 2 :row 1}]} - {:cards [{:id 1 :row 1} {:id 2 :row 2}]})))) - - (is (= "modified the cards." - (build-sentence - (revision/diff-strings - Dashboard - {:cards [{:id 1} {:id 2}]} - {:cards [{:id 1} {:id 3}]}))))) - - (testing "update collection ---" - (t2.with-temp/with-temp - [Collection {coll-id :id} {:name "New collection"}] - (is (= "moved this Dashboard to New collection." - (build-sentence - (revision/diff-strings + (are [x y expected] (= expected + (build-sentence (revision/diff-strings Dashboard x y))) + {:name "Diff Test" + :description nil + :cards []} + {:name "Diff Test Changed" + :description "foobar" + :cards []} + "added a description and renamed it from \"Diff Test\" to \"Diff Test Changed\"." + + {:name "Apple" + :cards [{:id 1} {:id 2}]} + {:name "Next" + :cards [{:id 1} {:id 3}]} + "renamed it from \"Apple\" to \"Next\" and modified the cards." + + {:name "Diff Test" + :auto_apply_filters true} + {:name "Diff Test" + :auto_apply_filters false} + "set auto apply filters to false." + + ;; multiple changes + {:name "Diff Test" + :description nil + :cache_ttl 333 + :cards [{:size_x 4 + :size_y 4 + :row 0 + :col 0 + :id 1 + :card_id 1 + :series [5 6]} + {:size_x 4 + :size_y 4 + :row 0 + :col 0 + :id 2 + :card_id 2 + :series []}]} + {:name "Diff Test" + :description nil + :cache_ttl 1227 + :cards [{:size_x 4 + :size_y 4 + :row 0 + :col 0 + :id 1 + :card_id 1 + :series [4 5]} + {:size_x 4 + :size_y 4 + :row 2 + :col 0 + :id 2 + :card_id 2 + :series [3 4 5]}]} + (str "changed the cache ttl from \"333\" to \"1,227\", rearranged the cards, modified the series on card 1 and " + "added some series to card 2.")))) + +(deftest ^:parallel diff-dashboards-str-update-cards-test + (testing "update cards ---" + (are [x y expected] (= expected + (build-sentence (revision/diff-strings Dashboard x y))) + {:cards [{:id 1} {:id 2}]} + {:cards [{:id 1} {:id 2} {:id 3}]} + "added a card." + + {:cards [{:id 1} {:id 2}]} + {:cards [{:id 1}]} + "removed a card." + + {:cards [{:id 1 :row 0} {:id 2 :row 1}]} + {:cards [{:id 1 :row 1} {:id 2 :row 2}]} + "rearranged the cards." + + {:cards [{:id 1} {:id 2}]} + {:cards [{:id 1} {:id 3}]} + "modified the cards."))) + +(deftest diff-dashboards-str-update-collection-test + (testing "update collection ---" + (t2.with-temp/with-temp + [Collection {coll-id :id} {:name "New collection"}] + (is (= "moved this Dashboard to New collection." + (build-sentence + (revision/diff-strings Dashboard {:name "Apple"} {:name "Apple" :collection_id coll-id}))))) - (t2.with-temp/with-temp - [Collection {coll-id-1 :id} {:name "Old collection"} - Collection {coll-id-2 :id} {:name "New collection"}] - (is (= "moved this Dashboard from Old collection to New collection." - (build-sentence - (revision/diff-strings + (t2.with-temp/with-temp + [Collection {coll-id-1 :id} {:name "Old collection"} + Collection {coll-id-2 :id} {:name "New collection"}] + (is (= "moved this Dashboard from Old collection to New collection." + (build-sentence + (revision/diff-strings Dashboard {:name "Apple" :collection_id coll-id-1} {:name "Apple" - :collection_id coll-id-2})))))) - - (testing "update tabs" - (is (= "added a tab." - (build-sentence - (revision/diff-strings - Dashboard - {:tabs [{:id 0 :name "First tab" :position 0}]} - {:tabs [{:id 0 :name "First tab" :position 0} - {:id 1 :name "Second tab" :position 1}]})))) - - (is (= "removed 2 tabs." - (build-sentence - (revision/diff-strings - Dashboard - {:tabs [{:id 0 :name "First tab" :position 0} - {:id 1 :name "Second tab" :position 1} - {:id 2 :name "Third tab" :position 2}]} - {:tabs [{:id 0 :name "First tab" :position 0}]})))) - - (is (= "rearranged the tabs." - (build-sentence - (revision/diff-strings - Dashboard - {:tabs [{:id 0 :name "First tab" :position 0} - {:id 1 :name "Second tab" :position 1}]} - {:tabs [{:id 1 :name "Second tab" :position 0} - {:id 0 :name "First tab" :position 1}]})))) - - (is (= "modified the tabs." - (build-sentence - (revision/diff-strings - Dashboard - {:tabs [{:id 0 :name "Tab A" :position 0} - {:id 1 :name "Tab B" :position 1}]} - {:tabs [{:id 1 :name "Tab B new name and position" :position 0} - {:id 0 :name "Tab A new name and position" :position 1}]})))))) + :collection_id coll-id-2}))))))) + +(deftest ^:parallel diff-dashboards-str-update-tabs-test + (testing "update tabs" + (are [x y expected] (= expected + (build-sentence (revision/diff-strings Dashboard x y))) + {:tabs [{:id 0 :name "First tab" :position 0}]} + {:tabs [{:id 0 :name "First tab" :position 0} + {:id 1 :name "Second tab" :position 1}]} + "added a tab." + + {:tabs [{:id 0 :name "First tab" :position 0} + {:id 1 :name "Second tab" :position 1} + {:id 2 :name "Third tab" :position 2}]} + {:tabs [{:id 0 :name "First tab" :position 0}]} + "removed 2 tabs." + + {:tabs [{:id 0 :name "First tab" :position 0} + {:id 1 :name "Second tab" :position 1}]} + {:tabs [{:id 1 :name "Second tab" :position 0} + {:id 0 :name "First tab" :position 1}]} + "rearranged the tabs." + + {:tabs [{:id 0 :name "Tab A" :position 0} + {:id 1 :name "Tab B" :position 1}]} + {:tabs [{:id 1 :name "Tab B new name and position" :position 0} + {:id 0 :name "Tab A new name and position" :position 1}]} + "modified the tabs."))) (deftest revert-dashboard!-test - (tt/with-temp* [Dashboard [{dashboard-id :id, :as dashboard} {:name "Test Dashboard"}] - Card [{card-id :id}] - Card [{series-id-1 :id}] - Card [{series-id-2 :id}] - DashboardCard [{dashcard-id :id :as dashboard-card} {:dashboard_id dashboard-id, :card_id card-id}] - DashboardCardSeries [_ {:dashboardcard_id dashcard-id, :card_id series-id-1, :position 0}] - DashboardCardSeries [_ {:dashboardcard_id dashcard-id, :card_id series-id-2, :position 1}]] + (t2.with-temp/with-temp [Dashboard {dashboard-id :id, :as dashboard} {:name "Test Dashboard"} + Card {card-id :id} {} + Card {series-id-1 :id} {} + Card {series-id-2 :id} {} + DashboardCard {dashcard-id :id :as dashboard-card} {:dashboard_id dashboard-id, :card_id card-id} + DashboardCardSeries _ {:dashboardcard_id dashcard-id, :card_id series-id-1, :position 0} + DashboardCardSeries _ {:dashboardcard_id dashcard-id, :card_id series-id-2, :position 1}] (let [check-ids (fn [[{:keys [id card_id series] :as card}]] [(assoc card :id (= dashcard-id id) @@ -525,13 +500,13 @@ (:public_uuid dashboard)))))))) (deftest post-update-test - (tt/with-temp* [Collection [{collection-id-1 :id}] - Collection [{collection-id-2 :id}] - Dashboard [{dashboard-id :id} {:name "Lucky the Pigeon's Lucky Stuff", :collection_id collection-id-1}] - Card [{card-id :id}] - Pulse [{pulse-id :id} {:dashboard_id dashboard-id, :collection_id collection-id-1}] - DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id, :card_id card-id}] - PulseCard [_ {:pulse_id pulse-id, :card_id card-id, :dashboard_card_id dashcard-id}]] + (t2.with-temp/with-temp [Collection {collection-id-1 :id} {} + Collection {collection-id-2 :id} {} + Dashboard {dashboard-id :id} {:name "Lucky the Pigeon's Lucky Stuff", :collection_id collection-id-1} + Card {card-id :id} {} + Pulse {pulse-id :id} {:dashboard_id dashboard-id, :collection_id collection-id-1} + DashboardCard {dashcard-id :id} {:dashboard_id dashboard-id, :card_id card-id} + PulseCard _ {:pulse_id pulse-id, :card_id card-id, :dashboard_card_id dashcard-id}] (testing "Pulse name and collection-id updates" (t2/update! Dashboard dashboard-id {:name "Lucky's Close Shaves" :collection_id collection-id-2}) (is (= "Lucky's Close Shaves" @@ -554,11 +529,10 @@ :id "_CATEGORY_NAME_" :type "category"}] (testing "A new dashboard creates a new ParameterCard" - (tt/with-temp* [Card [{card-id :id}] - Dashboard [{dashboard-id :id} - {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id card-id}})]}]] + (t2.with-temp/with-temp [Card {card-id :id} {} + Dashboard {dashboard-id :id} {:parameters [(merge default-params + {:values_source_type "card" + :values_source_config {:card_id card-id}})]}] (is (=? {:card_id card-id :parameterized_object_type :dashboard :parameterized_object_id dashboard-id @@ -566,9 +540,8 @@ (t2/select-one 'ParameterCard :card_id card-id))))) (testing "Adding a card_id creates a new ParameterCard" - (tt/with-temp* [Card [{card-id :id}] - Dashboard [{dashboard-id :id} - {:parameters [default-params]}]] + (t2.with-temp/with-temp [Card {card-id :id} {} + Dashboard {dashboard-id :id} {:parameters [default-params]}] (is (nil? (t2/select-one 'ParameterCard :card_id card-id))) (t2/update! Dashboard dashboard-id {:parameters [(merge default-params {:values_source_type "card" @@ -580,11 +553,10 @@ (t2/select-one 'ParameterCard :card_id card-id))))) (testing "Removing a card_id deletes old ParameterCards" - (tt/with-temp* [Card [{card-id :id}] - Dashboard [{dashboard-id :id} - {:parameters [(merge default-params - {:values_source_type "card" - :values_source_config {:card_id card-id}})]}]] + (t2.with-temp/with-temp [Card {card-id :id} {} + Dashboard {dashboard-id :id} {:parameters [(merge default-params + {:values_source_type "card" + :values_source_config {:card_id card-id}})]}] ;; same setup as earlier test, we know the ParameterCard exists right now (t2/delete! Dashboard :id dashboard-id) (is (nil? (t2/select-one 'ParameterCard :card_id card-id))))))) @@ -595,14 +567,14 @@ (defn do-with-dash-in-collection [f] (tu/with-non-admin-groups-no-root-collection-perms - (tt/with-temp* [Collection [collection] - Dashboard [dash {:collection_id (u/the-id collection)}] - Database [db {:engine :h2}] - Table [table {:db_id (u/the-id db)}] - Card [card {:dataset_query {:database (u/the-id db) - :type :query - :query {:source-table (u/the-id table)}}}] - DashboardCard [_ {:dashboard_id (u/the-id dash), :card_id (u/the-id card)}]] + (t2.with-temp/with-temp [Collection collection {} + Dashboard dash {:collection_id (u/the-id collection)} + Database db {:engine :h2} + Table table {:db_id (u/the-id db)} + Card card {:dataset_query {:database (u/the-id db) + :type :query + :query {:source-table (u/the-id table)}}} + DashboardCard _ {:dashboard_id (u/the-id dash), :card_id (u/the-id card)}] (f db collection dash)))) (defmacro with-dash-in-collection diff --git a/test/metabase/models/pulse_card_test.clj b/test/metabase/models/pulse_card_test.clj index a8a179c956dbbefb36e4bc82fb285df08fd691df..8c485167ccde8e5a40f81adeb42fc9b633a8e484 100644 --- a/test/metabase/models/pulse_card_test.clj +++ b/test/metabase/models/pulse_card_test.clj @@ -8,8 +8,6 @@ [metabase.models.pulse :refer [Pulse]] [metabase.models.pulse-card :as pulse-card :refer [PulseCard]] [metabase.models.serialization :as serdes] - [metabase.test :as mt] - [toucan.util.test :as tt] [toucan2.tools.with-temp :as t2.with-temp]) (:import (java.time LocalDateTime))) @@ -21,24 +19,24 @@ (t2.with-temp/with-temp [Pulse {pulse-id :id}] (is (zero? (pulse-card/next-position-for pulse-id))))) (testing "With cards" - (tt/with-temp* [Pulse [{pulse-id :id}] - Card [{card-id :id}] - Dashboard [{dashboard-id :id}] - DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id}] - PulseCard [_ {:pulse_id pulse-id - :card_id card-id - :dashboard_card_id dashcard-id - :position 2}]] + (t2.with-temp/with-temp [Pulse {pulse-id :id} {} + Card {card-id :id} {} + Dashboard {dashboard-id :id} {} + DashboardCard {dashcard-id :id} {:dashboard_id dashboard-id} + PulseCard _ {:pulse_id pulse-id + :card_id card-id + :dashboard_card_id dashcard-id + :position 2}] (is (= 3 (pulse-card/next-position-for pulse-id)))))) (deftest identity-hash-test (testing "Pulse card hashes are composed of the pulse's hash and the card's hash" (let [now (LocalDateTime/of 2022 9 1 12 34 56)] - (mt/with-temp* [Collection [coll1 {:name "field-db" :location "/" :created_at now}] - Collection [coll2 {:name "other collection" :location "/" :created_at now}] - Card [card {:name "the card" :collection_id (:id coll1) :created_at now}] - Pulse [pulse {:name "my pulse" :collection_id (:id coll2) :created_at now}] - PulseCard [pulse-card {:card_id (:id card) :pulse_id (:id pulse) :position 4}]] + (t2.with-temp/with-temp [Collection coll1 {:name "field-db" :location "/" :created_at now} + Collection coll2 {:name "other collection" :location "/" :created_at now} + Card card {:name "the card" :collection_id (:id coll1) :created_at now} + Pulse pulse {:name "my pulse" :collection_id (:id coll2) :created_at now} + PulseCard pulse-card {:card_id (:id card) :pulse_id (:id pulse) :position 4}] (is (= "9ad1b5a4" (serdes/raw-hash [(serdes/identity-hash pulse) (serdes/identity-hash card) 4]) (serdes/identity-hash pulse-card))))))) diff --git a/test/metabase/models/pulse_test.clj b/test/metabase/models/pulse_test.clj index 4a07657f914319fc791543483cc307893b57a1f9..d1d56e3dbf0a601d35a6b468a097b612a3ba1653 100644 --- a/test/metabase/models/pulse_test.clj +++ b/test/metabase/models/pulse_test.clj @@ -25,7 +25,8 @@ [schema.core :as s] [toucan.hydrate :refer [hydrate]] [toucan.util.test :as tt] - [toucan2.core :as t2]) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp]) (:import (java.time LocalDateTime))) @@ -72,11 +73,11 @@ (deftest retrieve-pulse-test (testing "this should cover all the basic Pulse attributes" - (tt/with-temp* [Pulse [{pulse-id :id} {:name "Lodi Dodi"}] - PulseChannel [{channel-id :id} {:pulse_id pulse-id - :details {:other "stuff" - :emails ["foo@bar.com"]}}] - Card [{card-id :id} {:name "Test Card"}]] + (t2.with-temp/with-temp [Pulse {pulse-id :id} {:name "Lodi Dodi"} + PulseChannel {channel-id :id} {:pulse_id pulse-id + :details {:other "stuff" + :emails ["foo@bar.com"]}} + Card {card-id :id} {:name "Test Card"}] (t2/insert! PulseCard, :pulse_id pulse-id, :card_id card-id, :position 0) (t2/insert! PulseChannelRecipient, :pulse_channel_id channel-id, :user_id (mt/user->id :rasta)) (is (= (merge diff --git a/test/metabase/pulse/test_util.clj b/test/metabase/pulse/test_util.clj index 962e502606c73a7078505d4a236f3d20768a3639..dd7b20148caec5cb516ab644757bd5c2082b9e9b 100644 --- a/test/metabase/pulse/test_util.clj +++ b/test/metabase/pulse/test_util.clj @@ -10,15 +10,15 @@ [metabase.test :as mt] [metabase.test.data.users :as test.users] [metabase.util :as u] - [toucan.util.test :as tt])) + [toucan2.tools.with-temp :as t2.with-temp])) (set! *warn-on-reflection* true) (defn send-pulse-created-by-user! "Create a Pulse with `:creator_id` of `user-kw`, and simulate sending it, executing it and returning the results." [user-kw card] - (tt/with-temp* [Pulse [pulse {:creator_id (test.users/user->id user-kw)}] - PulseCard [_ {:pulse_id (:id pulse), :card_id (u/the-id card)}]] + (t2.with-temp/with-temp [Pulse pulse {:creator_id (test.users/user->id user-kw)} + PulseCard _ {:pulse_id (:id pulse), :card_id (u/the-id card)}] (with-redefs [pulse/send-notifications! identity pulse/parts->notifications (fn [_ results] (vec results))] diff --git a/test/metabase/query_processor/middleware/annotate_test.clj b/test/metabase/query_processor/middleware/annotate_test.clj index 8ca414cd3a8045418c5cab3eafc7f6ef5ee04c07..9a1b4b0358da06ae14d29d8298b83b6fccf87238 100644 --- a/test/metabase/query_processor/middleware/annotate_test.clj +++ b/test/metabase/query_processor/middleware/annotate_test.clj @@ -9,8 +9,8 @@ [metabase.query-processor.store :as qp.store] [metabase.test :as mt] [metabase.util :as u] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (set! *warn-on-reflection* true) @@ -209,33 +209,33 @@ (deftest col-info-combine-parent-field-names-test (testing "For fields with parents we should return them with a combined name including parent's name" - (tt/with-temp* [Field [parent {:name "parent", :table_id (mt/id :venues)}] - Field [child {:name "child", :table_id (mt/id :venues), :parent_id (u/the-id parent)}]] - (mt/with-everything-store + (t2.with-temp/with-temp [Field parent {:name "parent", :table_id (mt/id :venues)} + Field child {:name "child", :table_id (mt/id :venues), :parent_id (u/the-id parent)}] + (mt/with-everything-store (is (= {:description nil - :table_id (mt/id :venues) - :semantic_type nil - :effective_type nil - ;; these two are a gross symptom. there's some tension. sometimes it makes sense to have an effective - ;; type: the db type is different and we have a way to convert. Othertimes, it doesn't make sense: - ;; when the info is inferred. the solution to this might be quite extensive renaming - :coercion_strategy nil - :name "parent.child" - :settings nil - :field_ref [:field (u/the-id child) nil] - :nfc_path nil - :parent_id (u/the-id parent) - :id (u/the-id child) - :visibility_type :normal - :display_name "Child" - :fingerprint nil - :base_type :type/Text} + :table_id (mt/id :venues) + :semantic_type nil + :effective_type nil + ;; these two are a gross symptom. there's some tension. sometimes it makes sense to have an effective + ;; type: the db type is different and we have a way to convert. Othertimes, it doesn't make sense: + ;; when the info is inferred. the solution to this might be quite extensive renaming + :coercion_strategy nil + :name "parent.child" + :settings nil + :field_ref [:field (u/the-id child) nil] + :nfc_path nil + :parent_id (u/the-id parent) + :id (u/the-id child) + :visibility_type :normal + :display_name "Child" + :fingerprint nil + :base_type :type/Text} (into {} (#'annotate/col-info-for-field-clause {} [:field (u/the-id child) nil]))))))) (testing "nested-nested fields should include grandparent name (etc)" - (tt/with-temp* [Field [grandparent {:name "grandparent", :table_id (mt/id :venues)}] - Field [parent {:name "parent", :table_id (mt/id :venues), :parent_id (u/the-id grandparent)}] - Field [child {:name "child", :table_id (mt/id :venues), :parent_id (u/the-id parent)}]] + (t2.with-temp/with-temp [Field grandparent {:name "grandparent", :table_id (mt/id :venues)} + Field parent {:name "parent", :table_id (mt/id :venues), :parent_id (u/the-id grandparent)} + Field child {:name "child", :table_id (mt/id :venues), :parent_id (u/the-id parent)}] (mt/with-everything-store (is (= {:description nil :table_id (mt/id :venues) diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj index c1c606f5e20b38d3a867ca4fc0f57c28fd3683b3..eb625ab2989ad3a659ac1baee5044a713742f552 100644 --- a/test/metabase/query_processor/test_util.clj +++ b/test/metabase/query_processor/test_util.clj @@ -79,8 +79,8 @@ (defn card-with-source-metadata-for-query "Given an MBQL `query`, return the relevant keys for creating a Card with that query and matching `:result_metadata`. - (tt/with-temp Card [card (qp.test-util/card-with-source-metadata-for-query - (data/mbql-query venues {:aggregation [[:count]]}))] + (t2.with-temp/with-temp [Card card (qp.test-util/card-with-source-metadata-for-query + (data/mbql-query venues {:aggregation [[:count]]}))] ...)" [query] (let [results (qp/process-userland-query query) diff --git a/test/metabase/sync/analyze/classify_test.clj b/test/metabase/sync/analyze/classify_test.clj index e20b8e012815983585833a07c29662cd1a862263..dbb87a2828a3bea4061bee612f4acb612c02b8fb 100644 --- a/test/metabase/sync/analyze/classify_test.clj +++ b/test/metabase/sync/analyze/classify_test.clj @@ -9,88 +9,90 @@ [metabase.sync.analyze.classify :as classify] [metabase.sync.interface :as i] [metabase.util :as u] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (deftest fields-to-classify-test (testing "Finds current fingerprinted versions that are not analyzed" - (tt/with-temp* [Table [table] - Field [_ {:table_id (u/the-id table) - :name "expected" - :description "Current fingerprint, not analyzed" - :fingerprint_version i/latest-fingerprint-version - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "not expected 1" - :description "Current fingerprint, already analzed" - :fingerprint_version i/latest-fingerprint-version - :last_analyzed #t "2017-08-09"}] - Field [_ {:table_id (u/the-id table) - :name "not expected 2" - :description "Old fingerprint, not analyzed" - :fingerprint_version (dec i/latest-fingerprint-version) - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "not expected 3" - :description "Old fingerprint, already analzed" - :fingerprint_version (dec i/latest-fingerprint-version) - :last_analyzed #t "2017-08-09"}]] + (t2.with-temp/with-temp [Table table {} + Field _ {:table_id (u/the-id table) + :name "expected" + :description "Current fingerprint, not analyzed" + :fingerprint_version i/latest-fingerprint-version + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "not expected 1" + :description "Current fingerprint, already analzed" + :fingerprint_version i/latest-fingerprint-version + :last_analyzed #t "2017-08-09"} + Field _ {:table_id (u/the-id table) + :name "not expected 2" + :description "Old fingerprint, not analyzed" + :fingerprint_version (dec i/latest-fingerprint-version) + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "not expected 3" + :description "Old fingerprint, already analzed" + :fingerprint_version (dec i/latest-fingerprint-version) + :last_analyzed #t "2017-08-09"}] (is (= ["expected"] (for [field (#'classify/fields-to-classify table)] (:name field)))))) (testing "Finds previously marked :type/category fields for state" - (tt/with-temp* [Table [table] - Field [_ {:table_id (u/the-id table) - :name "expected" - :description "Current fingerprint, not analyzed" - :fingerprint_version i/latest-fingerprint-version - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "not expected 1" - :description "Current fingerprint, already analzed" - :fingerprint_version i/latest-fingerprint-version - :last_analyzed #t "2017-08-09"}] - Field [_ {:table_id (u/the-id table) - :name "not expected 2" - :description "Old fingerprint, not analyzed" - :fingerprint_version (dec i/latest-fingerprint-version) - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "not expected 3" - :description "Old fingerprint, already analzed" - :fingerprint_version (dec i/latest-fingerprint-version) - :last_analyzed #t "2017-08-09"}]]))) + (t2.with-temp/with-temp [Table table {} + Field _ {:table_id (u/the-id table) + :name "expected" + :description "Current fingerprint, not analyzed" + :fingerprint_version i/latest-fingerprint-version + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "not expected 1" + :description "Current fingerprint, already analzed" + :fingerprint_version i/latest-fingerprint-version + :last_analyzed #t "2017-08-09"} + Field _ {:table_id (u/the-id table) + :name "not expected 2" + :description "Old fingerprint, not analyzed" + :fingerprint_version (dec i/latest-fingerprint-version) + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "not expected 3" + :description "Old fingerprint, already analzed" + :fingerprint_version (dec i/latest-fingerprint-version) + :last_analyzed #t "2017-08-09"}]))) (deftest classify-fields-for-db!-test (testing "We classify decimal fields that have specially handled NaN values" - (tt/with-temp* [Database [db] - Table [table {:db_id (u/the-id db)}] - Field [field {:table_id (u/the-id table) - :name "Income" - :base_type :type/Float - :semantic_type nil - :fingerprint_version i/latest-fingerprint-version - :fingerprint {:type {:type/Number {:min "NaN" - :max "NaN" - :avg "NaN"}} - :global {:distinct-count 3}} - :last_analyzed nil}]] + (t2.with-temp/with-temp [Database db {} + Table table {:db_id (u/the-id db)} + Field field {:table_id (u/the-id table) + :name "Income" + :base_type :type/Float + :semantic_type nil + :fingerprint_version i/latest-fingerprint-version + :fingerprint {:type {:type/Number {:min "NaN" + :max "NaN" + :avg "NaN"}} + :global {:distinct-count 3}} + :last_analyzed nil}] (is (nil? (:semantic_type (t2/select-one Field :id (u/the-id field))))) (classify/classify-fields-for-db! db [table] (constantly nil)) - (is (= :type/Income (:semantic_type (t2/select-one Field :id (u/the-id field))))))) + (is (= :type/Income (:semantic_type (t2/select-one Field :id (u/the-id field)))))))) + +(deftest classify-decimal-fields-test (testing "We can classify decimal fields that have specially handled infinity values" - (tt/with-temp* [Database [db] - Table [table {:db_id (u/the-id db)}] - Field [field {:table_id (u/the-id table) - :name "Income" - :base_type :type/Float - :semantic_type nil - :fingerprint_version i/latest-fingerprint-version - :fingerprint {:type {:type/Number {:min "-Infinity" - :max "Infinity" - :avg "Infinity"}} - :global {:distinct-count 3}} - :last_analyzed nil}]] + (t2.with-temp/with-temp [Database db {} + Table table {:db_id (u/the-id db)} + Field field {:table_id (u/the-id table) + :name "Income" + :base_type :type/Float + :semantic_type nil + :fingerprint_version i/latest-fingerprint-version + :fingerprint {:type {:type/Number {:min "-Infinity" + :max "Infinity" + :avg "Infinity"}} + :global {:distinct-count 3}} + :last_analyzed nil}] (is (nil? (:semantic_type (t2/select-one Field :id (u/the-id field))))) (classify/classify-fields-for-db! db [table] (constantly nil)) (is (= :type/Income (:semantic_type (t2/select-one Field :id (u/the-id field)))))))) diff --git a/test/metabase/sync/analyze/fingerprint_test.clj b/test/metabase/sync/analyze/fingerprint_test.clj index 30ab075c609b349bf6d3d055a33e1c972cdfa2c5..7ee75d3c7d5e0c3aee177ee56d17ff1291accdfd 100644 --- a/test/metabase/sync/analyze/fingerprint_test.clj +++ b/test/metabase/sync/analyze/fingerprint_test.clj @@ -13,7 +13,6 @@ [metabase.test.data :as data] [metabase.util :as u] [schema.core :as s] - [toucan.util.test :as tt] [toucan2.core :as t2] [toucan2.tools.with-temp :as t2.with-temp])) @@ -133,8 +132,8 @@ qp/process-query (fn [_ {:keys [rff]}] (transduce identity (rff :metadata) [[1] [2] [3] [4] [5]])) fingerprint/save-fingerprint! (fn [& _] (reset! fingerprinted? true))] - (tt/with-temp* [Table [table] - Field [_ (assoc field-properties :table_id (u/the-id table))]] + (t2.with-temp/with-temp [Table table {} + Field _ (assoc field-properties :table_id (u/the-id table))] [(fingerprint/fingerprint-fields! table) @fingerprinted?])))) diff --git a/test/metabase/sync/analyze_test.clj b/test/metabase/sync/analyze_test.clj index 605291661a6fd39945a46d422f3265f9d9526acb..87f4b015b262222f30b1098c775b43f2da5be8e2 100644 --- a/test/metabase/sync/analyze_test.clj +++ b/test/metabase/sync/analyze_test.clj @@ -8,8 +8,10 @@ [metabase.sync.analyze :as analyze] [metabase.sync.analyze.classifiers.category :as classifiers.category] [metabase.sync.analyze.classifiers.name :as classifiers.name] - [metabase.sync.analyze.classifiers.no-preview-display :as classifiers.no-preview-display] - [metabase.sync.analyze.classifiers.text-fingerprint :as classifiers.text-fingerprint] + [metabase.sync.analyze.classifiers.no-preview-display + :as classifiers.no-preview-display] + [metabase.sync.analyze.classifiers.text-fingerprint + :as classifiers.text-fingerprint] [metabase.sync.analyze.fingerprint.fingerprinters :as fingerprinters] [metabase.sync.concurrent :as sync.concurrent] [metabase.sync.interface :as i] @@ -18,8 +20,8 @@ [metabase.test.data :as data] [metabase.test.sync :as test.sync :refer [sync-survives-crash?]] [metabase.util :as u] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (deftest skip-analysis-of-fields-with-current-fingerprint-version-test (testing "Check that Fields do *not* get analyzed if they're not newly created and fingerprint version is current" @@ -42,8 +44,8 @@ ;; ...but they *SHOULD* get analyzed if they ARE newly created (expcept for PK which we skip) (deftest analyze-table-test - (tt/with-temp* [Database [db {:engine "h2", :details (:details (data/db))}] - Table [table {:name "VENUES", :db_id (u/the-id db)}]] + (t2.with-temp/with-temp [Database db {:engine "h2", :details (:details (data/db))} + Table table {:name "VENUES", :db_id (u/the-id db)}] ;; sync the metadata, but DON't do analysis YET (sync-metadata/sync-table-metadata! table) ;; ok, NOW run the analysis process @@ -61,23 +63,23 @@ (deftest mark-fields-as-analyzed-test (testing "Make sure that only the correct Fields get marked as recently analyzed" (with-redefs [i/latest-fingerprint-version Short/MAX_VALUE] - (tt/with-temp* [Table [table] - Field [_ {:table_id (u/the-id table) - :name "Current fingerprint, not analyzed" - :fingerprint_version Short/MAX_VALUE - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "Current fingerprint, already analzed" - :fingerprint_version Short/MAX_VALUE - :last_analyzed #t "2017-08-09T00:00Z"}] - Field [_ {:table_id (u/the-id table) - :name "Old fingerprint, not analyzed" - :fingerprint_version (dec Short/MAX_VALUE) - :last_analyzed nil}] - Field [_ {:table_id (u/the-id table) - :name "Old fingerprint, already analzed" - :fingerprint_version (dec Short/MAX_VALUE) - :last_analyzed #t "2017-08-09T00:00Z"}]] + (t2.with-temp/with-temp [Table table {} + Field _ {:table_id (u/the-id table) + :name "Current fingerprint, not analyzed" + :fingerprint_version Short/MAX_VALUE + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "Current fingerprint, already analzed" + :fingerprint_version Short/MAX_VALUE + :last_analyzed #t "2017-08-09T00:00Z"} + Field _ {:table_id (u/the-id table) + :name "Old fingerprint, not analyzed" + :fingerprint_version (dec Short/MAX_VALUE) + :last_analyzed nil} + Field _ {:table_id (u/the-id table) + :name "Old fingerprint, already analzed" + :fingerprint_version (dec Short/MAX_VALUE) + :last_analyzed #t "2017-08-09T00:00Z"}] (#'analyze/update-fields-last-analyzed! table) (is (= #{"Current fingerprint, not analyzed"} (t2/select-fn-set :name Field :table_id (u/the-id table), :last_analyzed [:> #t "2018-01-01"]))))))) @@ -103,7 +105,7 @@ field (transduce identity (fingerprinters/fingerprinter field) values))))) -(deftest classify-json-test +(deftest ^:parallel classify-json-test (doseq [[group values->expected] {"When all the values are valid JSON dicts they're valid JSON" {["{\"this\":\"is\",\"valid\":\"json\"}" "{\"this\":\"is\",\"valid\":\"json\"}" @@ -130,7 +132,7 @@ (is (= (when expected :type/SerializedJSON) (classified-semantic-type values))))))) -(deftest classify-emails-test +(deftest ^:parallel classify-emails-test (testing "Check that things that are valid emails are marked as Emails" (doseq [[values expected] {["helper@metabase.com"] true ["helper@metabase.com", "someone@here.com", "help@nope.com"] true diff --git a/test/metabase/sync/sync_metadata/fields/sync_instances_test.clj b/test/metabase/sync/sync_metadata/fields/sync_instances_test.clj index 047aaa3b56afe6c2742416ee6e13e4ef642a65ee..5812a5325b53bf9bb482e871eaea3aa407f2f67c 100644 --- a/test/metabase/sync/sync_metadata/fields/sync_instances_test.clj +++ b/test/metabase/sync/sync_metadata/fields/sync_instances_test.clj @@ -8,8 +8,8 @@ [metabase.sync.sync-metadata.fields :as sync-fields] [metabase.test.mock.toucanery :as toucanery] [metabase.util :as u] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (def ^:private toucannery-transactions-expected-fields-hierarchy {"ts" nil @@ -29,8 +29,8 @@ (format-fields (get parent-id->children nil)))) (deftest sync-fields-test - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}] - Table [table {:name "transactions", :db_id (u/the-id db)}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery} + Table table {:name "transactions", :db_id (u/the-id db)}] ;; do the initial sync (sync-fields/sync-fields-for-table! table) (let [transactions-table-id (u/the-id (t2/select-one-pk Table :db_id (u/the-id db), :name "transactions"))] @@ -40,8 +40,8 @@ (deftest delete-nested-field-test (testing (str "If you delete a nested Field, and re-sync a Table, it should recreate the Field as it was before! It " "should not create any duplicate Fields (#8950)") - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}] - Table [table {:name "transactions", :db_id (u/the-id db)}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery} + Table table {:name "transactions", :db_id (u/the-id db)}] ;; do the initial sync (sync-fields/sync-fields-for-table! table) (let [transactions-table-id (u/the-id (t2/select-one-pk Table :db_id (u/the-id db), :name "transactions"))] @@ -54,12 +54,13 @@ (is (= toucannery-transactions-expected-fields-hierarchy (actual-fields-hierarchy transactions-table-id))))))) -(deftest sync-db-metadata-test - ;; TODO: this uses the higher level `sync-metadata/sync-db-metadata!` entry but serves as a test for - ;; `sync-instances` and perhaps can be moved to use this entry. This is a bit more mecahnical for code org so I - ;; don't want to get into that in this change. +;; TODO: this uses the higher level `sync-metadata/sync-db-metadata!` entry but serves as a test for +;; `sync-instances` and perhaps can be moved to use this entry. This is a bit more mecahnical for code org so I +;; don't want to get into that in this change. + +(deftest resync-nested-fields-test (testing "Make sure nested fields get resynced correctly if their parent field didnt' change" - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery}] ;; do the initial sync (sync-metadata/sync-db-metadata! db) ;; delete our entry for the `transactions.toucan.details.age` field @@ -72,10 +73,11 @@ (sync-metadata/sync-db-metadata! db) ;; field should be added back (is (= #{"weight" "age"} - (t2/select-fn-set :name Field :table_id transactions-table-id, :parent_id details-field-id, :active true)))))) + (t2/select-fn-set :name Field :table_id transactions-table-id, :parent_id details-field-id, :active true))))))) +(deftest reactivate-field-test (testing "Syncing can reactivate a field" - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery}] ;; do the initial sync (sync-metadata/sync-db-metadata! db) ;; delete our entry for the `transactions.toucan.details.age` field @@ -87,10 +89,11 @@ ;; now sync again. (sync-metadata/sync-db-metadata! db) ;; field should be reactivated - (is (t2/select-fn-set :active Field :id age-field-id))))) + (is (t2/select-fn-set :active Field :id age-field-id)))))) +(deftest reactivate-nested-field-when-parent-is-reactivated-test (testing "Nested fields get reactivated if the parent field gets reactivated" - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery}] ;; do the initial sync (sync-metadata/sync-db-metadata! db) ;; delete our entry for the `transactions.toucan.details.age` field @@ -102,10 +105,11 @@ ;; now sync again. (sync-metadata/sync-db-metadata! db) ;; field should be reactivated - (is (t2/select-fn-set :active Field :id age-field-id))))) + (is (t2/select-fn-set :active Field :id age-field-id)))))) +(deftest mark-nested-field-inactive-test (testing "Nested fields can be marked inactive" - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery}] ;; do the initial sync (sync-metadata/sync-db-metadata! db) ;; Add an entry for a `transactions.toucan.details.gender` field @@ -123,10 +127,11 @@ ;; now sync again. (sync-metadata/sync-db-metadata! db) ;; field should become inactive - (is (false? (t2/select-one-fn :active Field :id gender-field-id)))))) + (is (false? (t2/select-one-fn :active Field :id gender-field-id))))))) +(deftest mark-nested-field-children-inactive-test (testing "When a nested field is marked inactive so are its children" - (tt/with-temp* [Database [db {:engine ::toucanery/toucanery}]] + (t2.with-temp/with-temp [Database db {:engine ::toucanery/toucanery}] ;; do the initial sync (sync-metadata/sync-db-metadata! db) ;; Add an entry for a `transactions.toucan.details.gender` field diff --git a/test/metabase/sync/util_test.clj b/test/metabase/sync/util_test.clj index 15f44f844dcf8e84c1651b0760df1fddfda4d355..69690ccc90dce2dba8587dba87af43164c130e40 100644 --- a/test/metabase/sync/util_test.clj +++ b/test/metabase/sync/util_test.clj @@ -14,8 +14,8 @@ [metabase.sync.util :as sync-util] [metabase.test :as mt] [metabase.test.util :as tu] - [toucan.util.test :as tt] - [toucan2.core :as t2])) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) (set! *warn-on-reflection* true) @@ -42,7 +42,7 @@ (testing "only one sync process be going on at a time" ;; describe-database gets called once during a single sync process, and the results are used for syncing tables ;; and syncing the _metabase_metadata table. - (tt/with-temp* [Database [db {:engine ::concurrent-sync-test}]] + (t2.with-temp/with-temp [Database db {:engine ::concurrent-sync-test}] (reset! calls-to-describe-database 0) ;; start a sync processes in the background. It should take 1000 ms to finish (let [f1 (future (sync/sync-database! db)) diff --git a/test/metabase/test.clj b/test/metabase/test.clj index af97de848d0c30a8fe51ff14e9f739bdc08fe325..ad7b4c555180b727a9c38cebc25dd8633902574f 100644 --- a/test/metabase/test.clj +++ b/test/metabase/test.clj @@ -427,7 +427,5 @@ ;;; other libaries any other way. They're marked deprecated to encourage you to use the `t2.with-temp` versions. #_{:clj-kondo/ignore [:discouraged-var]} (doseq [varr [#'with-temp - #'with-temp* - #'tt/with-temp - #'tt/with-temp*]] + #'with-temp*]] (alter-meta! varr assoc :deprecated true)) diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj index 8a5ba49abd04f1da4c2adb4e2277a75c705def0b..a1d503b391065da6c7d81fcdf82a2568c0c55655 100644 --- a/test/metabase/test/data/users.clj +++ b/test/metabase/test/data/users.clj @@ -15,8 +15,8 @@ [metabase.util :as u] [metabase.util.password :as u.password] [schema.core :as s] - [toucan.util.test :as tt] - [toucan2.core :as t2]) + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp]) (:import (clojure.lang ExceptionInfo))) @@ -215,9 +215,9 @@ (u/the-id test-user-name-or-user-id))) (defn do-with-group-for-user [group test-user-name-or-user-id f] - (tt/with-temp* [PermissionsGroup [group group] - PermissionsGroupMembership [_ {:group_id (u/the-id group) - :user_id (test-user-name-or-user-id->user-id test-user-name-or-user-id)}]] + (t2.with-temp/with-temp [PermissionsGroup group group + PermissionsGroupMembership _ {:group_id (u/the-id group) + :user_id (test-user-name-or-user-id->user-id test-user-name-or-user-id)}] (f group))) (defmacro with-group diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index efb92d358c551c0db350082cdb5a0b12cbd1bd3f..3b75ec654a45ee68fcf6bf53db09fa6458c50e9f 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -56,7 +56,6 @@ [metabase.util :as u] [metabase.util.files :as u.files] [methodical.core :as methodical] - [toucan.util.test :as tt] [toucan2.core :as t2] [toucan2.model :as t2.model] [toucan2.tools.with-temp :as t2.with-temp]) @@ -869,16 +868,16 @@ (testing (format "With human readable values remapping %s -> %s\n" (describe-field original) (pr-str values-map)) (thunk)))] - (tt/with-temp* [Dimension [_ {:field_id (:id original) - :name (format "%s [internal remap]" (:display_name original)) - :type :internal}]] + (t2.with-temp/with-temp [Dimension _ {:field_id (:id original) + :name (format "%s [internal remap]" (:display_name original)) + :type :internal}] (if preexisting-id (with-temp-vals-in-db FieldValues preexisting-id {:values (keys values-map) :human_readable_values (vals values-map)} (testing-thunk)) - (tt/with-temp* [FieldValues [_ {:field_id (:id original) - :values (keys values-map) - :human_readable_values (vals values-map)}]] + (t2.with-temp/with-temp [FieldValues _ {:field_id (:id original) + :values (keys values-map) + :human_readable_values (vals values-map)}] (testing-thunk))))))))))) orig->remapped))