Use features instead of hard-coded driver names in tests (#52758)

* use features instead of hard coding drivers in tests

* fix formatting

* fix details-name-is-db feature

* bad conn details helper instead of features

* address pr comments

* exclude vertica from materialized view tests

* fix whitespace

docs/developers-guide/driver-changelog.md

@@ -4,6 +4,10 @@ title: Driver interface changelog
 
 # Driver Interface Changelog
 
+## Metabase 0.53.0 
+
+- Added the multi-method `bad-connection-details` to allow mocking bad connection parameters for tests.
+
 ## Metabase 0.52.0
 
 - The Docker image for Metabase 0.52.0 now uses Java 21 instead of Java 11. Please make sure to test your driver

src/metabase/driver.clj

@@ -717,7 +717,15 @@
     ;; Whether the driver supports loading dynamic test datasets on each test run. Eg. datasets with names like
     ;; `checkins:4-per-minute` are created dynamically in each test run. This should be truthy for every driver we test
     ;; against except for Athena and Databricks which currently require test data to be loaded separately.
-    :test/dynamic-dataset-loading})
+    :test/dynamic-dataset-loading
+
+    ;; Some DBs allow you to connect to a DB that doesn't exist by creating it for you.
+    ;; This is to allow such DBs to opt out of tests that rely on not being able to connect to non-existent DBs.
+    :test/creates-db-on-connect
+
+    ;; For some cloud DBs the test database is never created, and can't or shouldn't be destroyed.
+    ;; This is to allow avoiding destroying the test DBs of such cloud DBs.
+    :test/cannot-destroy-db})
 
 (defmulti database-supports?
   "Does this driver and specific instance of a database support a certain `feature`?

test/metabase/driver/sql_jdbc/sync/describe_table_test.clj

@@ -756,7 +756,7 @@
          (is (= #{{:type :normal-column-index :value "id"}}
                 (describe-table-indexes (t2/select-one :model/Table (mt/id :conditional_index))))))))))
 
-(defmethod driver/database-supports? [::driver/driver ::materialized-view-fields]
+(defmethod driver/database-supports? [::driver/driver ::describe-materialized-view-fields]
   [_driver _feature _database]
   true)
 
@@ -783,7 +783,8 @@
                 :mongo
                 :sparksql
                 :sqlite
-                :athena]]
+                :athena
+                :vertica]]
   (defmethod driver/database-supports? [driver ::describe-materialized-view-fields]
     [_driver _feature _database]
     false))

test/metabase/driver_test.clj

@@ -84,9 +84,35 @@
                          :field-definitions [{:field-name "foo", :base-type :type/Text}]
                          :rows              [["bar"]]}]}))
 
+(defmulti bad-connection-details
+  {:arglists '([driver])}
+  tx/dispatch-on-driver-with-test-extensions
+  :hierarchy #'driver/hierarchy)
+
+(doseq [driver [:redshift :snowflake :vertica]]
+  (defmethod bad-connection-details driver
+    [_driver]
+    {:db (mt/random-name)}))
+
+(doseq [driver [:oracle]]
+  (defmethod bad-connection-details driver
+    [_driver]
+    {:service-name (mt/random-name)}))
+
+(doseq [driver [:presto-jdbc]]
+  (defmethod bad-connection-details driver
+    [_driver]
+    {:catalog (mt/random-name)}))
+
+(doseq [driver [:redshift :snowflake :vertica :presto-jdbc :oracle]]
+  (defmethod driver/database-supports? [driver :test/cannot-destroy-db]
+    [_driver _feature _database]
+    true))
+
 (deftest can-connect-with-destroy-db-test
   (testing "driver/can-connect? should fail or throw after destroying a database"
-    (mt/test-drivers (mt/normal-drivers-with-feature :test/dynamic-dataset-loading)
+    (mt/test-drivers (set/difference (mt/normal-drivers-with-feature :test/dynamic-dataset-loading)
+                                     (mt/normal-drivers-with-feature :test/creates-db-on-connect))
       (let [database-name (mt/random-name)
             dbdef         (basic-db-definition database-name)]
         (mt/dataset dbdef
@@ -100,10 +126,8 @@
             (testing "after deleting a database, can-connect? should return false or throw an exception"
               (let [;; in the case of some cloud databases, the test database is never created, and can't or shouldn't be destroyed.
                     ;; so fake it by changing the database details
-                    details (case driver/*driver*
-                              (:redshift :snowfake :vertica) (assoc details :db (mt/random-name))
-                              :oracle                        (assoc details :service-name (mt/random-name))
-                              :presto-jdbc                   (assoc details :catalog (mt/random-name))
+                    details (if (driver/database-supports? driver/*driver* :test/cannot-destroy-db (mt/db))
+                              (merge details (bad-connection-details driver/*driver*))
                               ;; otherwise destroy the db and use the original details
                               (do
                                 (tx/destroy-db! driver/*driver* dbdef)
@@ -118,7 +142,8 @@
 
 (deftest check-can-connect-before-sync-test
   (testing "Database sync should short-circuit and fail if the database at the connection has been deleted (metabase#7526)"
-    (mt/test-drivers (mt/normal-drivers-with-feature :test/dynamic-dataset-loading)
+    (mt/test-drivers (set/difference (mt/normal-drivers-with-feature :test/dynamic-dataset-loading)
+                                     (mt/normal-drivers-with-feature :test/creates-db-on-connect))
       (let [database-name (mt/random-name)
             dbdef         (basic-db-definition database-name)]
         (mt/dataset dbdef
@@ -142,14 +167,11 @@
             ;; release db resources like connection pools so we don't have to wait to finish syncing before destroying the db
             (driver/notify-database-updated driver/*driver* db)
             ;; destroy the db
-            (if (contains? #{:redshift :snowflake :vertica :presto-jdbc :oracle} driver/*driver*)
+            (if (driver/database-supports? driver/*driver* :test/cannot-destroy-db (mt/db))
               ;; in the case of some cloud databases, the test database is never created, and can't or shouldn't be destroyed.
               ;; so fake it by changing the database details
               (let [details     (:details (mt/db))
-                    new-details (case driver/*driver*
-                                  (:redshift :snowflake :vertica) (assoc details :db (mt/random-name))
-                                  :oracle                         (assoc details :service-name (mt/random-name))
-                                  :presto-jdbc                    (assoc details :catalog (mt/random-name)))]
+                    new-details (merge details (bad-connection-details driver/*driver*))]
                 (t2/update! :model/Database (u/the-id db) {:details new-details}))
               ;; otherwise destroy the db and use the original details
               (tx/destroy-db! driver/*driver* dbdef))