Unify test application db environment variables [ci drivers]

This unifies the code and the environment variables for specifying
test database connection parameters across all supported
databases. Existing databases that supported environment variables for
tests have been refactored to use the new environment variable
names. All of the connection environment variables are named similar
to:

MB_<DBNAME>_TEST_<PARAM>

For user on MySQL, the variable would be named:

MB_MYSQL_TEST_USER

bin/ci

@@ -8,7 +8,7 @@ node-0() {
     if is_engine_enabled "mongo"; then
         run_step install-mongodb
     fi
-    run_step lein-test
+    MB_MYSQL_TEST_USER=ubuntu run_step lein-test
 }
 node-1() {
     is_enabled "drivers" && export ENGINES="h2,sqlserver,oracle" || export ENGINES="h2"
@@ -27,7 +27,7 @@ node-2() {
         run_step install-presto
     fi
     MB_ENCRYPTION_SECRET_KEY='Orw0AAyzkO/kPTLJRxiyKoBHXa/d6ZcO+p+gpZO/wSQ=' MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost \
-        MB_PRESTO_HOST=localhost MB_PRESTO_PORT=8080 \
+        MB_PRESTO_TEST_HOST=localhost MB_PRESTO_TEST_PORT=8080 MB_POSTGRESQL_TEST_USER=ubuntu \
         run_step lein-test
 }
 node-3() {

test/metabase/test/data/bigquery.clj

@@ -21,18 +21,11 @@
 
 (def ^:private ^String normalize-name (comp (u/rpartial s/replace #"-" "_") name))
 
-(defn- get-env-var [env-var]
-  (or (env (keyword (format "mb-bigquery-%s" (name env-var))))
-      (throw (Exception. (format "In order to test BigQuery, you must specify the env var MB_BIGQUERY_%s."
-                                 (s/upper-case (s/replace (name env-var) #"-" "_")))))))
-
 (def ^:private ^:const details
   (datasets/when-testing-engine :bigquery
-    {:project-id    (get-env-var :project-id)
-     :client-id     (get-env-var :client-id)
-     :client-secret (get-env-var :client-secret)
-     :access-token  (get-env-var :access-token)
-     :refresh-token (get-env-var :refresh-token)}))
+    (reduce (fn [acc env-var]
+              (assoc acc env-var (i/db-test-env-var-or-throw :bigquery env-var)))
+            {} [:project-id :client-id :client-secret :access-token :refresh-token])))
 
 (def ^:private ^:const ^String project-id (:project-id details))
 

test/metabase/test/data/druid.clj

 (ns metabase.test.data.druid
   (:require [cheshire.core :as json]
             [clojure.java.io :as io]
-            [environ.core :refer [env]]
             [metabase.test.data
              [dataset-definitions :as defs]
              [interface :as i]]
@@ -10,10 +9,8 @@
   (:import metabase.driver.druid.DruidDriver))
 
 (defn- database->connection-details [& _]
-  {:host (or (env :mb-druid-host)
-             (throw (Exception. "In order to test Druid, you must specify `MB_DRUID_HOST`.")))
-   :port (Integer/parseInt (or (env :mb-druid-port)
-                               (throw (Exception. "In order to test Druid, you must specify `MB_DRUID_PORT`."))))})
+  {:host (i/db-test-env-var-or-throw :druid :host)
+   :port (Integer/parseInt (i/db-test-env-var-or-throw :druid :port))})
 
 (u/strict-extend DruidDriver
   i/IDriverTestExtensions

test/metabase/test/data/interface.clj

@@ -4,6 +4,7 @@
    Objects that implement `IDriverTestExtensions` know how to load a `DatabaseDefinition` into an
    actual physical RDMS database. This functionality allows us to easily test with multiple datasets."
   (:require [clojure.string :as str]
+            [environ.core :refer [env]]
             [metabase
              [db :as db]
              [driver :as driver]
@@ -232,3 +233,36 @@
      (for [fielddef (nest-fielddefs dbdef table-name)]
        (update fielddef :field-name flatten-field-name))
      (flatten-rows dbdef table-name)]))
+
+(defn db-test-env-var
+  "Look up test environment var `:ENV-VAR` for the given `:DATABASE-NAME` containing connection related parameters.
+  If no `:default` param is specified and the var isn't found, throw.
+
+     (db-test-env-var :mysql :user) ; Look up `MB_MYSQL_TEST_USER`"
+  ([database-name env-var]
+   (db-test-env-var database-name env-var nil))
+  ([database-name env-var default]
+   (get env
+        (keyword (format "mb-%s-test-%s" (name database-name) (name env-var)))
+        default)))
+
+(defn- to-system-env-var-str
+  "Converts the clojure environment variable form (a keyword) to a
+  stringified version that will be specified at the system level
+
+  i.e. :foo-bar -> FOO_BAR"
+  [env-var-kwd]
+  (-> env-var-kwd
+      name
+      (str/replace "-" "_")
+      str/upper-case))
+
+(defn db-test-env-var-or-throw
+  "Same as `db-test-env-var` but will throw an exception if the variable is nil"
+  ([database-name env-var]
+   (db-test-env-var-or-throw database-name env-var nil))
+  ([database-name env-var default]
+   (or (db-test-env-var database-name env-var default)
+       (throw (Exception. (format "In order to test %s, you must specify the env var MB_%s_TEST_%s."
+                                  (name database-name)
+                                  (to-system-env-var-str env-var)))))))

test/metabase/test/data/mysql.clj

 (ns metabase.test.data.mysql
   "Code for creating / destroying a MySQL database from a `DatabaseDefinition`."
-  (:require [environ.core :refer [env]]
-            [metabase.test.data
+  (:require [metabase.test.data
              [generic-sql :as generic]
              [interface :as i]]
             [metabase.util :as u])
@@ -19,11 +18,11 @@
    :type/Time       "TIME"})
 
 (defn- database->connection-details [context {:keys [database-name]}]
-  (merge {:host         "localhost"
-          :port         3306
-          :timezone     :America/Los_Angeles
-          :user         (if (env :circleci) "ubuntu"
-                            "root")}
+  (merge {:host         (i/db-test-env-var-or-throw :mysql :host "localhost")
+          :port         (i/db-test-env-var-or-throw :mysql :port 3306)
+          :user         (i/db-test-env-var :mysql :user "root")
+          :password     (i/db-test-env-var :mysql :password)
+          :timezone     :America/Los_Angeles}
          (when (= context :db)
            {:db database-name})))
 

test/metabase/test/data/oracle.clj

@@ -9,16 +9,6 @@
             [metabase.util :as u])
   (:import metabase.driver.oracle.OracleDriver))
 
-(defn- get-db-env-var
-  " Look up the relevant connection param from corresponding env var or throw an exception if it's not set.
-
-     (get-db-env-var :user) ; Look up `MB_ORACLE_USER`"
-  [env-var & [default]]
-  (or (env (keyword (format "mb-oracle-%s" (name env-var))))
-      default
-      (throw (Exception. (format "In order to test Oracle, you must specify the env var MB_ORACLE_%s."
-                                 (s/upper-case (name env-var)))))))
-
 ;; Similar to SQL Server, Oracle on AWS doesn't let you create different databases;
 ;; We'll create a unique schema (the same as a "User" in Oracle-land) for each test run and use that to keep
 ;; tests from clobbering over one another; we'll also qualify the names of tables to include their DB name
@@ -35,11 +25,11 @@
 
 
 (def ^:private db-connection-details
-  (delay {:host     (get-db-env-var :host)
-          :port     (Integer/parseInt (get-db-env-var :port "1521"))
-          :user     (get-db-env-var :user)
-          :password (get-db-env-var :password)
-          :sid      (get-db-env-var :sid)}))
+  (delay {:host     (i/db-test-env-var-or-throw :oracle :host)
+          :port     (Integer/parseInt (i/db-test-env-var-or-throw :oracle :port "1521"))
+          :user     (i/db-test-env-var-or-throw :oracle :user)
+          :password (i/db-test-env-var-or-throw :oracle :password)
+          :sid      (i/db-test-env-var-or-throw :oracle :sid)}))
 
 
 (def ^:private ^:const field-base-type->sql-type

test/metabase/test/data/postgres.clj

 (ns metabase.test.data.postgres
   "Code for creating / destroying a Postgres database from a `DatabaseDefinition`."
-  (:require [environ.core :refer [env]]
-            [metabase.test.data
+  (:require [metabase.test.data
              [generic-sql :as generic]
              [interface :as i]]
             [metabase.util :as u])
@@ -21,11 +20,11 @@
    :type/UUID       "UUID"})
 
 (defn- database->connection-details [context {:keys [database-name]}]
-  (merge {:host     "localhost"
-          :port     5432
+  (merge {:host     (i/db-test-env-var-or-throw :postgresql :host "localhost")
+          :port     (i/db-test-env-var-or-throw :postgresql :port 5432)
+          :user     (i/db-test-env-var :postgresql :user)
+          :password (i/db-test-env-var :postgresql :password)
           :timezone :America/Los_Angeles}
-         (when (env :circleci)
-           {:user "ubuntu"})
          (when (= context :db)
            {:db database-name})))
 

test/metabase/test/data/presto.clj

@@ -13,20 +13,12 @@
 
 (resolve-private-vars metabase.driver.presto execute-presto-query! presto-type->base-type quote-name quote+combine-names)
 
-;;; Helpers
-
-(defn- get-env-var [env-var]
-  (or (env (keyword (format "mb-presto-%s" (name env-var))))
-      (throw (Exception. (format "In order to test Presto, you must specify the env var MB_PRESTO_%s."
-                                 (s/upper-case (s/replace (name env-var) #"-" "_")))))))
-
-
 ;;; IDriverTestExtensions implementation
 
 (defn- database->connection-details [context {:keys [database-name]}]
-  (merge {:host (get-env-var :host)
-          :port (get-env-var :port)
-          :user "metabase"
+  (merge {:host (i/db-test-env-var-or-throw :presto :host)
+          :port (i/db-test-env-var-or-throw :presto :port)
+          :user (i/db-test-env-var-or-throw :presto :user "metabase")
           :ssl  false}
          (when (= context :db)
            {:catalog database-name})))

test/metabase/test/data/redshift.clj

 (ns metabase.test.data.redshift
   (:require [clojure.string :as s]
-            [environ.core :refer [env]]
             [metabase.driver.generic-sql :as sql]
             [metabase.test.data
              [generic-sql :as generic]
@@ -19,22 +18,12 @@
    :type/Integer    "INTEGER"
    :type/Text       "TEXT"})
 
-(defn- get-db-env-var
-  "Look up the relevant env var for AWS connection details or throw an exception if it's not set.
-
-     (get-db-env-var :user) ; Look up `MB_REDSHIFT_USER`"
-  [env-var & [default]]
-  (or (env (keyword (format "mb-redshift-%s" (name env-var))))
-      default
-      (throw (Exception. (format "In order to test Redshift, you must specify the env var MB_REDSHIFT_%s."
-                                 (s/upper-case (name env-var)))))))
-
 (def ^:private db-connection-details
-  (delay {:host     (get-db-env-var :host)
-          :port     (Integer/parseInt (get-db-env-var :port "5439"))
-          :db       (get-db-env-var :db)
-          :user     (get-db-env-var :user)
-          :password (get-db-env-var :password)}))
+  (delay {:host     (i/db-test-env-var-or-throw :redshift :host)
+          :port     (Integer/parseInt (i/db-test-env-var-or-throw :redshift :port "5439"))
+          :db       (i/db-test-env-var-or-throw :redshift :db)
+          :user     (i/db-test-env-var-or-throw :redshift :user)
+          :password (i/db-test-env-var-or-throw :redshift :password)}))
 
 
 ;; Redshift is tested remotely, which means we need to support multiple tests happening against the same remote host at the same time.

test/metabase/test/data/sqlserver.clj

 (ns metabase.test.data.sqlserver
   "Code for creating / destroying a SQLServer database from a `DatabaseDefinition`."
   (:require [clojure.java.jdbc :as jdbc]
-            [clojure.string :as s]
-            [environ.core :refer [env]]
             [metabase.driver.generic-sql :as sql]
             [metabase.test.data
              [datasets :as datasets]
@@ -35,22 +33,11 @@
 (defn- +suffix [db-name]
   (str db-name \_ @db-name-suffix-number))
 
-(defn- get-db-env-var
-  "Since we run our tests on non-Windows machines, we need to connect to a remote server for running tests.
-   Look up the relevant env var or throw an exception if it's not set.
-
-     (get-db-env-var :user) ; Look up `MB_SQL_SERVER_USER`"
-  [env-var & [default]]
-  (or (env (keyword (format "mb-sql-server-%s" (name env-var))))
-      default
-      (throw (Exception. (format "In order to test SQL Server, you must specify the env var MB_SQL_SERVER_%s."
-                                 (s/upper-case (name env-var)))))))
-
 (defn- database->connection-details [context {:keys [database-name]}]
-  {:host         (get-db-env-var :host)
-   :port         (Integer/parseInt (get-db-env-var :port "1433"))
-   :user         (get-db-env-var :user)
-   :password     (get-db-env-var :password)
+  {:host         (i/db-test-env-var-or-throw :sqlserver :host)
+   :port         (Integer/parseInt (i/db-test-env-var-or-throw :sqlserver :port "1433"))
+   :user         (i/db-test-env-var-or-throw :sqlserver :user)
+   :password     (i/db-test-env-var-or-throw :sqlserver :password)
    :db           (when (= context :db)
                    (+suffix database-name))})
 

test/metabase/test/data/vertica.clj

 (ns metabase.test.data.vertica
   "Code for creating / destroying a Vertica database from a `DatabaseDefinition`."
-  (:require [environ.core :refer [env]]
-            [metabase.driver.generic-sql :as sql]
+  (:require [metabase.driver.generic-sql :as sql]
             [metabase.test.data
              [generic-sql :as generic]
              [interface :as i]]
@@ -22,16 +21,16 @@
 
 
 (defn- db-name []
-  (or (env :mb-vertica-db)
-      "docker"))
+  (i/db-test-env-var-or-throw :vertica :db "docker"))
 
 (def ^:private db-connection-details
-  (delay {:host     (or (env :mb-vertica-host) "localhost")
+  (delay {:host     (i/db-test-env-var-or-throw :vertica :host "localhost")
+          :port     (Integer/parseInt (i/db-test-env-var-or-throw :vertica :port "5433"))
+          :user     (i/db-test-env-var :vertica :user "dbadmin")
+          :password (i/db-test-env-var :vertica :password)
           :db       (db-name)
-          :port     5433
           :timezone :America/Los_Angeles ; why?
-          :user     (or (env :mb-vertica-user) "dbadmin")
-          :password (env :mb-vertica-password)}))
+          }))
 
 (defn- qualified-name-components
   ([_]                             [(db-name)])