Upgrade to Metabase 1.49.3 and fix the tests accordingly

Makefile

@@ -79,6 +79,8 @@ update_deps_files:
 test: start_trino_if_missing link_to_driver update_deps_files
 	@echo "Testing Starburst driver..."
 	cp driver_test.clj metabase/test/metabase/
+	cp dataset_definition_test.clj metabase/test/metabase/test/data/
+	cp connection_test.clj metabase/test/metabase/driver/sql_jdbc/connection_test.clj
 	cd $(makefile_dir)/metabase/; DRIVERS=starburst MB_STARBURST_TEST_PORT=$(trino_port) clojure -X:dev:drivers:drivers-dev:test
 
 testOptimized: start_trino_if_missing link_to_driver update_deps_files

README.md

@@ -64,9 +64,9 @@ Head to actions and run the `Release` workflow entering the same the same semant
 ### Update Metabase Version
 If needed, `make checkout_latest_metabase_tag` will update Metabase to its latest tagged release. 
 
-*CAUTION*: the Metabase test file `metabase/test/metabase/driver_test.clj` is overridden by a modified version on the root directory (see the `Makefile`). This is because two tests (`can-connect-with-destroy-db-test` and `check-can-connect-before-sync-test`) do not work with the Starburst driver as they're testing what happens when a database is deleted (which cannot happen with Starburst). So instead of adding some useless stuff to `can-connect?` for the sole purpose of satisfying tests, it was found preferable to just remove those two tests.
+*CAUTION*: several Metabase test files (`driver_test.clj`, `dataset_definition_test.clj` and `connection_test.clj`) are overridden by a modified version on the root directory (see the `Makefile`). This is because several tests (`can-connect-with-destroy-db-test`, `check-can-connect-before-sync-test`, `dataset-with-custom-pk-test`, `dataset-with-custom-composite-pk-test` and `test-ssh-tunnel-connection`) are not expected to work for any Presto/Trino driver. They are explicitly disabled for the Presto and Athena drivers but not the Starburst driver, leading to failures (most of these test foreign keys, something Trino doesn't support).
 
-Whenever upgrading the version of Metabase, `./driver_test.clj` should be replaced with `metabase/test/metabase/driver_test.clj` with the two offending tests removed (unless they pass or there is a clean way around them)
+Whenever upgrading the version of Metabase, compare `./driver_test.clj`, `./dataset_definition_test.clj` and `./connection_test.clj` with the latest Metabase versions and check if changes need to be added.
 
 ## References
 * [Encrypting Metabase Database Details](https://www.metabase.com/docs/latest/operations-guide/encrypting-database-details-at-rest.html)

app_versions.json

@@ -1,5 +1,5 @@
 {
     "trino": "431",
     "clojure": "1.11.1.1262",
-    "metabase": "v1.48.3"
+    "metabase": "v1.49.3"
 }

dataset_definition_test.clj

@@ -0,0 +1,27 @@
+(ns metabase.test.data.dataset-definition-test
+  (:require
+   [clojure.test :refer :all]
+   [metabase.driver :as driver]
+   [metabase.driver.ddl.interface :as ddl.i]
+   [metabase.test :as mt]
+   [toucan2.core :as t2]))
+
+(deftest dataset-with-custom-pk-test
+  (mt/test-drivers (disj (mt/sql-jdbc-drivers)
+                         ;; presto doesn't create PK for test data :) not sure why
+                         :presto-jdbc
+                         ;; creating db for athena is expensive and require some extra steps,
+                         ;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
+                         :athena
+                         ;; there is no PK in sparksql
+                         :sparksql)))
+
+(deftest dataset-with-custom-composite-pk-test
+  (mt/test-drivers (disj (mt/sql-jdbc-drivers)
+                         ;; presto doesn't create PK for test data :) not sure why
+                         :presto-jdbc
+                         ;; creating db for athena is expensive and require some extra steps,
+                         ;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
+                         :athena
+                         ;; there is no PK in sparksql
+                         :sparksql)))

driver_test.clj

@@ -1,8 +1,10 @@
 (ns metabase.driver-test
   (:require
    [cheshire.core :as json]
+   [clojure.set :as set]
    [clojure.test :refer :all]
    [metabase.driver :as driver]
+   [metabase.driver.ddl.interface :as ddl.i]
    [metabase.driver.h2 :as h2]
    [metabase.driver.impl :as driver.impl]
    [metabase.plugins.classloader :as classloader]
@@ -114,3 +116,99 @@
             ;; one it should be harmless but annoying
             (is (= query
                    (json/parse-string weird-formatted-query)))))))))
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;; Begin tests for `describe-*` methods used in sync
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(defn- describe-fields-for-table [db table]
+  (sort-by :database-position
+           (if (driver/database-supports? driver/*driver* :describe-fields db)
+             (vec (driver/describe-fields driver/*driver* db
+                                          :schema-names [(:schema table)]
+                                          :table-names [(:name table)]))
+             (:fields (driver/describe-table driver/*driver* db table)))))
+
+(deftest ^:parallel describe-fields-or-table-test
+  (testing "test `describe-fields` or `describe-table` returns some basic metadata"
+    (mt/test-drivers (mt/normal-drivers)
+      (mt/dataset daily-bird-counts
+        (let [table (t2/select-one :model/Table :id (mt/id :bird-count))
+              fmt   #(ddl.i/format-name driver/*driver* %)]
+          (is (=? [{:name              (fmt "id")
+                    :database-type     string?
+                    :database-position 0
+                    :base-type         #(isa? % :type/Number)}
+                   {:name              (fmt "date")
+                    :database-type     string?
+                    :database-position 1
+                    :base-type         #(isa? % :type/Temporal)}
+                   {:name              (fmt "count")
+                    :database-type     string?
+                    :database-position 2
+                    :base-type         #(isa? % :type/Number)}]
+                  (describe-fields-for-table (mt/db) table))))))))
+
+(deftest ^:parallel describe-table-fks-test
+  (testing "`describe-table-fks` should work for drivers that do not support `describe-fks`"
+    (mt/test-drivers (set/difference (mt/normal-drivers-with-feature :foreign-keys)
+                                     (mt/normal-drivers-with-feature :describe-fks))
+      (let [orders   (t2/select-one :model/Table (mt/id :orders))
+            products (t2/select-one :model/Table (mt/id :products))
+            people   (t2/select-one :model/Table (mt/id :people))
+            fmt      (partial ddl.i/format-name driver/*driver*)]
+        (is (= #{{:fk-column-name   (fmt "user_id")
+                  :dest-table       (select-keys people [:name :schema])
+                  :dest-column-name (fmt "id")}
+                 {:fk-column-name   (fmt "product_id")
+                  :dest-table       (select-keys products [:name :schema])
+                  :dest-column-name (fmt "id")}}
+               #_{:clj-kondo/ignore [:deprecated-var]}
+               (driver/describe-table-fks driver/*driver* (mt/db) orders)))))))
+
+(deftest ^:parallel describe-fks-test
+  (testing "`describe-fks` works for drivers that support `describe-fks`"
+    (mt/test-drivers (mt/normal-drivers-with-feature :foreign-keys :describe-fks)
+      (let [fmt           (partial ddl.i/format-name driver/*driver*)
+            orders        (t2/select-one :model/Table (mt/id :orders))
+            products      (t2/select-one :model/Table (mt/id :products))
+            people        (t2/select-one :model/Table (mt/id :people))
+            entire-db-fks (driver/describe-fks driver/*driver* (mt/db))
+            schema-db-fks (driver/describe-fks driver/*driver* (mt/db)
+                                               :schema-names (when (:schema orders) [(:schema orders)]))
+            table-db-fks  (driver/describe-fks driver/*driver* (mt/db)
+                                               :schema-names (when (:schema orders) [(:schema orders)])
+                                               :table-names [(:name orders)])]
+        (doseq [[description orders-fks]
+                {"describe-fks results for entire DB includes the orders table FKs"
+                 (into #{}
+                       (filter (fn [x]
+                                 (and (= (:fk-table-name x) (:name orders))
+                                      (= (:fk-table-schema x) (:schema orders)))))
+                       entire-db-fks)
+                 "describe-fks results for a schema includes the orders table FKs"
+                 (into #{}
+                       (filter (fn [x]
+                                 (and (= (:fk-table-name x) (:name orders))
+                                      (= (:fk-table-schema x) (:schema orders)))))
+                       schema-db-fks)
+                 "describe-fks results for a table includes the orders table FKs"
+                 (into #{} table-db-fks)}]
+          (testing description
+            (is (= #{{:fk-column-name  (fmt "user_id")
+                      :fk-table-name   (:name orders)
+                      :fk-table-schema (:schema orders)
+                      :pk-column-name  (fmt "id")
+                      :pk-table-name   (:name people)
+                      :pk-table-schema (:schema people)}
+                     {:fk-column-name  (fmt "product_id")
+                      :fk-table-name   (:name orders)
+                      :fk-table-schema (:schema orders)
+                      :pk-column-name  (fmt "id")
+                      :pk-table-name   (:name products)
+                      :pk-table-schema (:schema products)}}
+                   orders-fks))))))))
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;; End tests for `describe-*` methods used in sync
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

drivers/starburst/src/metabase/driver/implementation/driver_helpers.clj

@@ -22,11 +22,6 @@
   [_]
   :monday)
 
-(defmethod driver/describe-table-fks :starburst [_ _ _]
-  ;; Trino does not support finding foreign key metadata tables, but some connectors support foreign keys.
-  ;; We have this return nil to avoid running unnecessary queries during fks sync.
-  nil)
-
 (doseq [[feature supported?] {:set-timezone                    true
                               :basic-aggregations              true
                               :standard-deviation-aggregations true

drivers/starburst/src/metabase/driver/starburst.clj

@@ -58,6 +58,7 @@
 
 ;;; The Starburst JDBC driver DOES NOT support the `.getImportedKeys` method so just return `nil` here so the
 ;;; implementation doesn't try to use it.
+#_{:clj-kondo/ignore [:deprecated-var]}
 (defmethod driver/describe-table-fks :starburst
   [_driver _database _table]
   nil)

drivers/starburst/test/metabase/driver/starburst_test.clj

@@ -245,7 +245,7 @@
 
 (deftest datetime-diff-base-test
   (mt/test-drivers (mt/normal-drivers-with-feature :datetime-diff)
-    (mt/dataset sample-dataset
+    (mt/dataset test-data
       (letfn [(query [x y unit]
                 (->> (mt/run-mbql-query orders
                        {:limit 1