Add missing "is" assertions to various tests (#15254)

* Add missing "is" assertions to various tests

* Fixing save-card-with-empty-result-metadata-test by simply asserting on API response

* Fix more failing tests

* Switch row-type-agnostic-test back to strings for expected

* Modify check for convert-id-to-string to also include keys whose :base_type is :type/Number, in addition to :type/Integer to fix failures in Oracle and Snowflake

modules/drivers/redshift/test/metabase/driver/redshift_test.clj

@@ -227,6 +227,7 @@
                               (is (= 14 (.getFetchSize (.getStatement rs))))
                               (orig-fn driver rs rsmeta canceled-chan))]
                 (with-redefs [sql-jdbc.execute/reducible-rows new-fn]
-                  (= [1] (-> {:query "SELECT 1"}
-                             (mt/native-query)
-                             (qp/process-query))))))))))))
+                  (is (= [[1]] (-> {:query "SELECT 1"}
+                                   (mt/native-query)
+                                   (qp/process-query)
+                                   (mt/rows)))))))))))))

src/metabase/query_processor/middleware/large_int_id.clj

@@ -50,7 +50,8 @@
                                                                    :id field-id))]
                                                 (when (and (or (isa? (:semantic_type field) :type/PK)
                                                                (isa? (:semantic_type field) :type/FK))
-                                                           (isa? (:base_type field) :type/Integer))
+                                                           (or (isa? (:base_type field) :type/Integer)
+                                                               (isa? (:base_type field) :type/Number)))
                                                   idx)))
                                             (:fields (:query query))))]
                    (fn [metadata]

test/metabase/api/automagic_dashboards_test.clj

@@ -36,8 +36,7 @@
                         (revoke-fn)
                         (let [result (mt/user-http-request :rasta :get 403 api-endpoint)]
                           (is (= "You don't have permissions to do that."
-                                 result))
-                          (= "You don't have permissions to do that." result)))
+                                 result))))
                       (finally
                         (perms/grant-permissions! (perms-group/all-users) (perms/object-path (mt/id))))))
            result))))))

test/metabase/api/card_test.clj

@@ -367,10 +367,15 @@
 (deftest save-card-with-empty-result-metadata-test
   (testing "we should be able to save a Card if the `result_metadata` is *empty* (but not nil) (#9286)"
     (mt/with-model-cleanup [Card]
-      (= :wow
-         (mt/user-http-request :rasta :post 202 "card" (assoc (card-with-name-and-query)
-                                                              :result_metadata    []
-                                                              :metadata_checksum  (#'results-metadata/metadata-checksum [])))))))
+      (let [card        (card-with-name-and-query)
+            md-checksum (#'results-metadata/metadata-checksum [])]
+        (is (schema= {:id su/IntGreaterThanZero, s/Keyword s/Any}
+                     (mt/user-http-request :rasta
+                                           :post
+                                           202
+                                           "card"
+                                           (assoc card :result_metadata   []
+                                                       :metadata_checksum md-checksum))))))))
 
 (defn- fingerprint-integers->doubles
   "Converts the min/max fingerprint values to doubles so simulate how the FE will change the metadata when POSTing a
@@ -1209,8 +1214,8 @@
         (let [card (mt/user-http-request :rasta :post 202 "card"
                                          (assoc (card-with-name-and-query)
                                                 :collection_id (u/the-id collection)))]
-          (= (db/select-one-field :collection_id Card :id (u/the-id card))
-             (u/the-id collection)))))))
+          (is (= (db/select-one-field :collection_id Card :id (u/the-id card))
+                 (u/the-id collection))))))))
 
 (deftest make-sure-we-card-creation-fails-if-we-try-to-set-a--collection-id--we-don-t-have-permissions-for
   (testing "POST /api/card"
@@ -1227,8 +1232,8 @@
     (mt/with-temp* [Card       [card]
                     Collection [collection]]
       (mt/user-http-request :crowberto :put 202 (str "card/" (u/the-id card)) {:collection_id (u/the-id collection)})
-      (= (db/select-one-field :collection_id Card :id (u/the-id card))
-         (u/the-id collection)))))
+      (is (= (db/select-one-field :collection_id Card :id (u/the-id card))
+             (u/the-id collection))))))
 
 (deftest update-card-require-parent-perms-test
   (testing "Should require perms for the parent collection to change a Card's properties"
@@ -1438,8 +1443,10 @@
     (testing "Attempting to share a Card that's already shared should return the existing public UUID"
       (mt/with-temporary-setting-values [enable-public-sharing true]
         (mt/with-temp Card [card (shared-card)]
-          (= (:public_uuid card)
-             (:uuid (mt/user-http-request :crowberto :post 200 (format "card/%d/public_link" (u/the-id card))))))))))
+          (is (= (:public_uuid card)
+                 (:uuid (mt/user-http-request :crowberto :post 200 (format
+                                                                    "card/%d/public_link"
+                                                                    (u/the-id card)))))))))))
 
 (deftest unshare-card-test
   (testing "DELETE /api/card/:id/public_link"

test/metabase/api/dashboard_test.clj

@@ -355,8 +355,8 @@
           ;; now make an API call to move collections
           (mt/user-http-request :rasta :put 200 (str "dashboard/" (u/get-id dash)) {:collection_id (u/get-id new-collection)})
           ;; Check to make sure the ID has changed in the DB
-          (= (db/select-one-field :collection_id Dashboard :id (u/get-id dash))
-             (u/get-id new-collection)))))
+          (is (= (db/select-one-field :collection_id Dashboard :id (u/get-id dash))
+                 (u/get-id new-collection))))))
 
     (testing "if we don't have the Permissions for the old collection, we should get an Exception"
       (mt/with-non-admin-groups-no-root-collection-perms

test/metabase/api/search_test.clj

@@ -204,7 +204,7 @@
                     DashboardCard [_               {:card_id card-id-5, :dashboard_id dashboard-id}]
                     DashboardCard [_               {:card_id card-id-5, :dashboard_id dashboard-id}]]
       (is (= dashboard-count-results
-             (unsorted-search-request :rasta :q "dashboard-count" ))))))
+             (unsorted-search-request :rasta :q "dashboard-count"))))))
 
 (deftest permissions-test
   (testing (str "Ensure that users without perms for the root collection don't get results NOTE: Metrics and segments "

test/metabase/api/setting_test.clj

@@ -64,8 +64,8 @@
         "Updated setting should be visible from API endpoint")
 
     (testing "Check non-superuser can't set a Setting"
-      (= "You don't have permissions to do that."
-         (mt/user-http-request :rasta :put 403 "setting/test-setting-1" {:value "NICE!"})))))
+      (is (= "You don't have permissions to do that."
+             (mt/user-http-request :rasta :put 403 "setting/test-setting-1" {:value "NICE!"}))))))
 
 (deftest fetch-sensitive-setting-test
   (testing "Sensitive settings should always come back obfuscated"

test/metabase/api/user_test.clj

@@ -215,21 +215,21 @@
             email     (mt/random-email)]
         (mt/with-model-cleanup [User]
           (mt/with-fake-inbox
-            (= (merge user-defaults
-                      (merge
-                       user-defaults
-                       {:email            email
-                        :first_name       user-name
-                        :last_name        user-name
-                        :common_name      (str user-name " " user-name)
-                        :group_ids        [(u/the-id (group/all-users))]
-                        :login_attributes {:test "value"}}))
-               (mt/boolean-ids-and-timestamps
-                (mt/user-http-request :crowberto :post 200 "user"
-                                      {:first_name       user-name
-                                       :last_name        user-name
-                                       :email            email
-                                       :login_attributes {:test "value"}})))))))
+            (is (= (merge user-defaults
+                          (merge
+                           user-defaults
+                           {:email            email
+                            :first_name       user-name
+                            :last_name        user-name
+                            :common_name      (str user-name " " user-name)
+                            :group_ids        [(u/the-id (group/all-users))]
+                            :login_attributes {:test "value"}}))
+                   (mt/boolean-ids-and-timestamps
+                    (mt/user-http-request :crowberto :post 200 "user"
+                                          {:first_name       user-name
+                                           :last_name        user-name
+                                           :email            email
+                                           :login_attributes {:test "value"}}))))))))
 
     (testing "Check that non-superusers are denied access"
       (is (= "You don't have permissions to do that."

test/metabase/models/collection_test.clj

@@ -508,8 +508,8 @@
              (descendants c))))
 
     (testing "try for a grandchild"
-      (= #{{:name "E", :id true, :description nil, :location "/A/C/D/", :children #{}}}
-         (descendants d)))))
+      (is (= #{{:name "E", :id true, :description nil, :location "/A/C/D/", :children #{}}}
+             (descendants d))))))
 
 (deftest root-collection-descendants-test
   (testing "For the *Root* Collection, can we get top-level Collections?"
@@ -1516,7 +1516,7 @@
              :id       2
              :children [{:name "Grandchild", :location "/1/2/", :id 3, :children []}]}]
            (collection/collections->tree [{:name "Child", :location "/1/", :id 2}
-                                          {:name "Grandchild", :location "/1/2/", :id 3} ])))))
+                                          {:name "Grandchild", :location "/1/2/", :id 3}])))))
 
 (deftest collections->tree-permutations-test
   (testing "The tree should build a proper tree regardless of which order the Collections are passed in (#14280)"

test/metabase/query_processor/middleware/permissions_test.clj

@@ -65,13 +65,13 @@
     (mt/with-temp* [Database [db]
                     Table    [table {:db_id (u/the-id db)}]]
       ;; query should be returned by middleware unchanged
-      (= {:database (u/the-id db)
-          :type     :query
-          :query    {:source-table (u/the-id table)}}
-         (check-perms-for-rasta
-          {:database (u/the-id db)
-           :type     :query
-           :query    {:source-table (u/the-id table)}})))))
+      (is (= {:database (u/the-id db)
+              :type     :query
+              :query    {:source-table (u/the-id table)}}
+             (check-perms-for-rasta
+              {:database (u/the-id db)
+               :type     :query
+               :query    {:source-table (u/the-id table)}}))))))
 
 (deftest nested-native-query-test
   (testing "Make sure nested native query fails to run if current user doesn't have perms"
@@ -84,13 +84,13 @@
   (testing "...but it should work if user has perms [nested native queries]"
     (mt/with-temp Database [db]
       ;; query should be returned by middleware unchanged
-      (= {:database (u/the-id db)
-          :type     :query
-          :query    {:source-query {:native "SELECT * FROM VENUES"}}}
-         (check-perms-for-rasta
-          {:database (u/the-id db)
-           :type     :query
-           :query   {:source-query {:native "SELECT * FROM VENUES"}}})))))
+      (is (= {:database (u/the-id db)
+              :type     :query
+              :query    {:source-query {:native "SELECT * FROM VENUES"}}}
+             (check-perms-for-rasta
+              {:database (u/the-id db)
+               :type     :query
+               :query   {:source-query {:native "SELECT * FROM VENUES"}}}))))))
 
 (deftest nested-mbql-query-test
   (testing "Make sure nested MBQL query fails to run if current user doesn't have perms"
@@ -107,13 +107,13 @@
   (testing "...but it should work if user has perms [nested MBQL queries]"
     (mt/with-temp* [Database [db]
                     Table    [table {:db_id (u/the-id db)}]]
-      (= {:database (u/the-id db)
-          :type     :query
-          :query    {:source-query {:source-table (u/the-id table)}}}
-         (check-perms-for-rasta
-          {:database (u/the-id db)
-           :type     :query
-           :query    {:source-query {:source-table (u/the-id table)}}})))))
+      (is (= {:database (u/the-id db)
+              :type     :query
+              :query    {:source-query {:source-table (u/the-id table)}}}
+             (check-perms-for-rasta
+              {:database (u/the-id db)
+               :type     :query
+               :query    {:source-query {:source-table (u/the-id table)}}}))))))
 
 (deftest template-tags-referenced-queries-test
   (testing "Fails for MBQL query referenced in template tag, when user has no perms to referenced query"

test/metabase/query_processor/reducible_test.clj

@@ -192,6 +192,6 @@
                       [2147483647]]
                      (process-query)))
               (testing "Should work with the middleware options used by API requests as well"
-                (= [["1"]
-                    ["2147483647"]]
-                   (process-query :middleware @api-qp-middleware-options))))))))))
+                (is (= [["1"]
+                        ["2147483647"]]
+                       (process-query :middleware @api-qp-middleware-options)))))))))))

test/metabase/query_processor_test/date_bucketing_test.clj

@@ -84,7 +84,7 @@
                              (mt/run-mbql-query incidents
                                {:fields   [$id $timestamp $severity]
                                 :order-by [[:asc $id]]
-                                :limit    5} ))))))))))))))
+                                :limit    5}))))))))))))))
 
 (defn- sad-toucan-incidents-with-bucketing
   "Returns 10 sad toucan incidents grouped by `unit`"

test/metabase/query_processor_test/nested_queries_test.clj

@@ -463,14 +463,14 @@
 (deftest drag-to-filter-timeseries-test
   (testing "make sure timeseries queries generated by \"drag-to-filter\" work correctly"
     (mt/with-temp Card [card (mbql-card-def (mt/$ids {:source-table $$checkins}))]
-      (= :completed
-         (completed-status
-          (qp/process-query
-           (query-with-source-card card
-             (mt/$ids checkins
-               {:aggregation [[:count]]
-                :breakout    [!week.*date]
-                :filter      [:between !week.*date "2014-02-01T00:00:00-08:00" "2014-05-01T00:00:00-07:00"]}))))))))
+      (is (= :completed
+             (-> (query-with-source-card card
+                   (mt/$ids checkins
+                     {:aggregation [[:count]]
+                      :breakout    [!week.*date]
+                      :filter      [:between !week.*date "2014-02-01T00:00:00-08:00" "2014-05-01T00:00:00-07:00"]}))
+                 (qp/process-query)
+                 (completed-status)))))))
 
 (deftest macroexpansion-test
   (testing "Make sure that macro expansion works inside of a neested query, when using a compound filter clause (#5974)"
@@ -718,9 +718,9 @@
                                                       :condition    [:= $category_id &c.categories.id]}]}
                        :filter       [:> [:field "count" {:base-type :type/Number}] 0]
                        :limit        3})]
-        (is (= [[ "20th Century Cafe" "Café" 1 ]
-                [ "25°" "Burger" 1 ]
-                [ "33 Taps" "Bar" 1 ]]
+        (is (= [[ "20th Century Cafe" "Café" 1]
+                [ "25°" "Burger" 1]
+                [ "33 Taps" "Bar" 1]]
                (mt/formatted-rows [str str int]
                  results)))
         (is (= (mt/$ids venues

test/metabase/query_processor_test/page_test.clj

@@ -22,9 +22,9 @@
                  (get-page 1))))
 
         (testing "get the second page"
-          (= [[ 6 "Bakery"]
-              [ 7 "Bar"]
-              [ 8 "Beer Garden"]
-              [ 9 "Breakfast / Brunch"]
-              [10 "Brewery"]]
-             (get-page 2)))))))
+          (is (= [[ 6 "Bakery"]
+                  [ 7 "Bar"]
+                  [ 8 "Beer Garden"]
+                  [ 9 "Breakfast / Brunch"]
+                  [10 "Brewery"]]
+                 (get-page 2))))))))

test/metabase/query_processor_test/share_test.clj

@@ -15,10 +15,10 @@
                {:aggregation [[:share [:< $price 4]]]}))))
 
     (testing "Normalization"
-      (= [[0.94]]
-         (mt/formatted-rows [2.0]
-           (mt/run-mbql-query venues
-             {:aggregation [["share" ["<" ["field-id" (mt/id :venues :price)] 4]]]}))))
+      (is (= [[0.94]]
+             (mt/formatted-rows [2.0]
+               (mt/run-mbql-query venues
+                 {:aggregation [["share" ["<" ["field-id" (mt/id :venues :price)] 4]]]})))))
 
     (testing "Complex filter clauses"
       (is (= [[0.17]]

test/metabase/timeseries_query_processor_test.clj

@@ -101,11 +101,11 @@
 
 (deftest sum-test
   (tqp.test/test-timeseries-drivers
-    (= {:columns ["sum"]
-        :rows    [[1992.0]]}
-       (mt/rows+column-names
-         (mt/run-mbql-query checkins
-           {:aggregation [[:sum $venue_price]]})))))
+    (is (= {:columns ["sum"]
+            :rows    [[1992.0]]}
+           (mt/rows+column-names
+             (mt/run-mbql-query checkins
+               {:aggregation [[:sum $venue_price]]}))))))
 
 (deftest avg-test
   (tqp.test/test-timeseries-drivers

test/metabase/util/date_2_test.clj

@@ -133,8 +133,8 @@
             "Passing `nil` should return `nil`"))
       (testing "blank strings"
         (is (= nil
-               (u.date/parse ""))
-            (= nil
+               (u.date/parse "")))
+        (is (= nil
                (u.date/parse "   ")))))))
 
 ;; TODO - more tests!