Use mb/once for relevant upload tests (#40499)

docs/developers-guide/driver-changelog.md

@@ -151,7 +151,7 @@ title: Driver interface changelog
 
 - A new driver feature has been added: `:schemas`. This feature signals whether the database organizes tables in
   schemas (also known as namespaces) or not. Most databases have schemas so this feature is on by default.
-  An implemention of the multimethod `metabase.driver/database-supports?` for `:schemas` is required only if the
+  An implementation of the multimethod `metabase.driver/database-supports?` for `:schemas` is required only if the
   database doesn't store tables in schemas.
 
 - Another driver feature has been added: `:uploads`. The `:uploads` feature signals whether the database supports

test/metabase/upload_test.clj

@@ -10,6 +10,7 @@
    [java-time.api :as t]
    [metabase.analytics.snowplow-test :as snowplow-test]
    [metabase.driver :as driver]
+   [metabase.driver.ddl.interface :as ddl.i]
    [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
    [metabase.driver.util :as driver.u]
    [metabase.models :refer [Field]]
@@ -99,6 +100,10 @@
   [& body]
   `(do-with-mysql-local-infile-off (fn [] ~@body)))
 
+(defn- format-schema [schema-name]
+  (when (driver/database-supports? driver/*driver* :schemas nil)
+    (ddl.i/format-name driver/*driver* schema-name)))
+
 (defn sync-upload-test-table!
   "Creates a table in the app db and syncs it synchronously, setting is_upload=true. Returns the table instance.
   The result is identical to if the table was synced with [[metabase.sync/sync-database!]], but faster because it skips
@@ -475,7 +480,7 @@
     (mt/with-current-user user-id
       (let [db                (t2/select-one :model/Database db-id)
             schema-name       (if (contains? args :schema-name)
-                                (:schema-name args)
+                                (format-schema (:schema-name args))
                                 (sql.tx/session-schema driver/*driver*))
             file              (csv-file-with
                                ["id, name"
@@ -654,8 +659,8 @@
     (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
       (with-mysql-local-infile-on-and-off
         (mt/with-dynamic-redefs [driver/db-default-timezone (constantly "Z")
-                      upload/current-database    (constantly (mt/db))]
-          (let [table-name (mt/random-name)
+                                 upload/current-database    (constantly (mt/db))]
+          (let [table-name     (mt/random-name)
                 datetime-pairs [["2022-01-01T12:00:00-07"    "2022-01-01T19:00:00Z"]
                                 ["2022-01-01T12:00:00-07:00" "2022-01-01T19:00:00Z"]
                                 ["2022-01-01T12:00:00-07:30" "2022-01-01T19:30:00Z"]
@@ -1153,9 +1158,8 @@
                 (:name new-field)
                 (:display_name new-field))))))))
 
-(deftest csv-upload-snowplow-test
-  ;; Just test with h2 because snowplow should be independent of the driver
-  (mt/test-driver :h2
+(deftest ^:mb/once csv-upload-snowplow-test
+  (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (snowplow-test/with-fake-snowplow-collector
       (with-upload-table! [_table (card->table (upload-example-csv!))]
         (testing "Successfully creating a CSV Upload publishes statistics to Snowplow"
@@ -1182,9 +1186,8 @@
                     :user-id (str (mt/user->id :rasta))}
                    (last (snowplow-test/pop-event-data-and-user-id!))))))))))
 
-(deftest csv-upload-audit-log-test
-  ;; Just test with h2 because these events are independent of the driver
-  (mt/test-driver :h2
+(deftest ^:mb/once csv-upload-audit-log-test
+  (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (mt/with-premium-features #{:audit-app}
       (with-upload-table!
        [_table (card->table (upload-example-csv!))]
@@ -1193,7 +1196,7 @@
                 :model    "Table"
                 :model_id pos?
                 :details  {:db-id       pos?
-                           :schema-name "PUBLIC"
+                           :schema-name (format-schema "public")
                            :table-name  string?
                            :model-id    pos?
                            :stats       {:num-rows          2
@@ -1203,9 +1206,8 @@
                                          :upload-seconds    pos?}}}
                (last-audit-event :upload-create)))))))
 
-(deftest create-csv-upload!-failure-test
-  ;; Just test with postgres because failure should be independent of the driver
-  (mt/test-driver :postgres
+(deftest ^:mb/once create-csv-upload!-failure-test
+  (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (mt/with-empty-db
       (testing "Uploads must be enabled"
         (doseq [uploads-enabled-value [false nil]]
@@ -1222,7 +1224,7 @@
         (mt/with-dynamic-redefs [driver/database-supports? (constantly false)]
           (is (thrown-with-msg?
                 java.lang.Exception
-                #"^Uploads are not supported on Postgres databases\."
+                #"^Uploads are not supported on \w+ databases\."
                 (upload-example-csv! :schema-name "public", :table-prefix "uploaded_magic_")))))
       (testing "User must have write permissions on the collection"
         (mt/with-non-admin-groups-no-root-collection-perms
@@ -1236,8 +1238,8 @@
                    (= :schema-filters (keyword (:type conn-prop))))
                  (driver/connection-properties driver))))
 
-(deftest create-csv-upload!-schema-does-not-sync-test
-  ;; Just test with postgres because failure should be independent of the driver
+(deftest ^:mb/once create-csv-upload!-schema-does-not-sync-test
+  ;; We only need to test this for a single driver, and the way this test has been written is coupled to Postgres
   (mt/test-driver :postgres
     (mt/with-empty-db
       (let [driver             (driver.u/database->driver (mt/db))
@@ -1250,6 +1252,7 @@
                                                                  patterns-type-prop "public"))})
         (testing "Upload should fail if table can't be found after sync, for example because of schema filters"
           (try (upload-example-csv! {:schema-name "public"})
+               (is (false? :should-not-be-reached))
                (catch Exception e
                  (is (= {:status-code 422}
                         (ex-data e)))
@@ -1259,7 +1262,7 @@
           (is (false? (let [details (mt/dbdef->connection-details driver/*driver* :db {:database-name (:name (mt/db))})]
                         (-> (jdbc/query (sql-jdbc.conn/connection-details->spec driver/*driver* details)
                                         ["SELECT EXISTS (SELECT 1 FROM information_schema.tables WHERE table_schema = 'public')"])
-                            first :exists)))))))))
+                            first vals first)))))))))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                           append-csv!                                                          |
@@ -1578,9 +1581,8 @@
                    (rows-for-table table))))
           (io/delete-file file))))))
 
-(deftest csv-append-snowplow-test
-  ;; Just test with h2 because snowplow should be independent of the driver
-  (mt/test-driver :h2
+(deftest ^:mb/once csv-append-snowplow-test
+  (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (snowplow-test/with-fake-snowplow-collector
 
      (with-upload-table! [table (create-upload-table!)]
@@ -1618,9 +1620,8 @@
                    :user-id (str (mt/user->id :crowberto))}
                   (last (snowplow-test/pop-event-data-and-user-id!))))))))))
 
-(deftest csv-append-audit-log-test
-  ;; Just test with h2 because these events are independent of the driver
-  (mt/test-driver :h2
+(deftest ^:mb/once csv-append-audit-log-test
+  (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (mt/with-premium-features #{:audit-app}
       (with-upload-table! [table (create-upload-table!)]
         (let [csv-rows ["name" "Luke Skywalker"]
@@ -1632,7 +1633,7 @@
                    :model    "Table"
                    :model_id (:id table)
                    :details  {:db-id       pos?
-                              :schema-name "PUBLIC"
+                              :schema-name (format-schema "public")
                               :table-name  string?
                               :stats       {:num-rows          1
                                             :num-columns       1