Skip to content

Commit

Permalink
PR feedback:
Browse files Browse the repository at this point in the history
Move bulk of logic from with-env-keys-renamed-by to new do-with-env-keys-renamed-by fn instead, calling that from macro

Change logic of do-with-env-keys-renamed-by to first build the map of renames, then call set/rename-keys with that (for easier capturing of the testing context information)

Switch to u/format-color in tests

Reference new test macro via metabase.test (which imports it)

Use namespace prefixes for the delegated test calls
  • Loading branch information
jeff303 committed Mar 24, 2021
1 parent ae2232d commit b5995d0
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 20 deletions.
13 changes: 7 additions & 6 deletions modules/drivers/oracle/test/metabase/driver/oracle_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
[metabase.models.table :refer [Table]]
[metabase.query-processor :as qp]
[metabase.query-processor-test :as qp.test]
[metabase.query-processor-test.order-by-test] ; used for one SSL connectivity test
[metabase.query-processor-test.order-by-test :as qp-test.order-by-test] ; used for one SSL connectivity test
[metabase.test :as mt]
[metabase.test.data.oracle :as oracle.tx]
[metabase.test.data.sql :as sql.tx]
Expand Down Expand Up @@ -238,8 +238,9 @@
(mt/test-driver :oracle
(if (System/getenv "MB_ORACLE_SSL_TEST_SSL")
(testing "Oracle with SSL connectivity"
(tu/with-env-keys-renamed-by #(str/replace-first % "mb-oracle-ssl-test" "mb-oracle-test")
(metabase.query-processor-test.order-by-test/order-by-aggregate-fields-test)))
(println (u/colorize 'yellow (format "Skipping %s because %s env var is not set\n"
"oracle-connect-with-ssl-test"
"MB_ORACLE_SSL_TEST_SSL"))))))
(mt/with-env-keys-renamed-by #(str/replace-first % "mb-oracle-ssl-test" "mb-oracle-test")
(qp-test.order-by-test/order-by-aggregate-fields-test)))
(println (u/format-color 'yellow
"Skipping %s because %s env var is not set"
"oracle-connect-with-ssl-test"
"MB_ORACLE_SSL_TEST_SSL")))))
15 changes: 8 additions & 7 deletions test/metabase/driver/mysql_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
[metabase.models.field :refer [Field]]
[metabase.models.table :refer [Table]]
[metabase.query-processor :as qp]
[metabase.query-processor-test.string-extracts-test] ; used for one SSL with PEM connectivity test
;; used for one SSL with PEM connectivity test
[metabase.query-processor-test.string-extracts-test :as string-extracts-test]
[metabase.sync :as sync]
[metabase.sync.analyze.fingerprint :as fingerprint]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
[metabase.test.util :as tu]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.honeysql-extensions :as hx]
Expand Down Expand Up @@ -337,8 +337,9 @@
(mt/test-driver :mysql
(if (System/getenv "MB_MYSQL_SSL_TEST_SSL_CERT")
(testing "MySQL with SSL connectivity using PEM certificate"
(tu/with-env-keys-renamed-by #(str/replace-first % "mb-mysql-ssl-test" "mb-mysql-test")
(metabase.query-processor-test.string-extracts-test/test-breakout)))
(println (u/colorize 'yellow (format "Skipping %s because %s env var is not set\n"
"mysql-connect-with-ssl-and-pem-cert-test"
"MB_MYSQL_SSL_TEST_SSL_CERT"))))))
(mt/with-env-keys-renamed-by #(str/replace-first % "mb-mysql-ssl-test" "mb-mysql-test")
(string-extracts-test/test-breakout)))
(println (u/format-color 'yellow
"Skipping %s because %s env var is not set"
"mysql-connect-with-ssl-and-pem-cert-test"
"MB_MYSQL_SSL_TEST_SSL_CERT")))))
1 change: 1 addition & 0 deletions test/metabase/test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@
throw-if-called
with-column-remappings
with-discarded-collections-perms-changes
with-env-keys-renamed-by
with-locale
with-log-level
with-log-messages
Expand Down
26 changes: 19 additions & 7 deletions test/metabase/test/util.clj
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
(ns metabase.test.util
"Helper functions and macros for writing unit tests."
(:require [cheshire.core :as json]
[clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer :all]
[clojure.walk :as walk]
[clojurewerkz.quartzite.scheduler :as qs]
[colorize.core :as colorize]
[environ.core :as env]
[java-time :as t]
[medley.core :as m]
[metabase.driver :as driver]
[metabase.models :refer [Card Collection Dashboard DashboardCardSeries Database Dimension Field FieldValues
Metric NativeQuerySnippet Permissions PermissionsGroup Pulse PulseCard PulseChannel
Expand Down Expand Up @@ -797,14 +797,26 @@
(with-open [socket (ServerSocket. 0)]
(.getLocalPort socket)))

(defn do-with-env-keys-renamed-by
"Evaluates the thunk with the current core.environ/env being redefined, its keys having been renamed by the given
rename-fn. Prefer to use the with-env-keys-renamed-by macro version instead."
[rename-fn thunk]
(let [orig-e env/env
renames-fn (fn [m k _]
(let [k-str (name k)
new-k (rename-fn k-str)]
(if (not= k-str new-k)
(assoc m k (keyword new-k))
m)))
renames (reduce-kv renames-fn {} orig-e)
new-e (set/rename-keys orig-e renames)]
(testing (colorize/blue (format "\nRenaming env vars by map: %s\n" (u/pprint-to-str renames)))
(with-redefs [env/env new-e]
(thunk)))))

(defmacro with-env-keys-renamed-by
"Evaluates body with the current core.environ/env being redefined, its keys having been renamed by the given
rename-fn."
{:arglists '([rename-fn & body])}
[rename-fn & body]
`(let [e# env/env
key-fn# (fn [k#]
(keyword (~rename-fn (name k#))))
new-e# (m/map-keys key-fn# e#)]
(with-redefs [env/env new-e#]
~@body)))
`(do-with-env-keys-renamed-by ~rename-fn (fn [] ~@body)))

0 comments on commit b5995d0

Please sign in to comment.