diff --git a/test/metabase/models/user_test.clj b/test/metabase/models/user_test.clj index 008ac841f0ef7286675b0332125a3cd5f0afe11f..9e79aa6fc38a1fdbe76abfbd8995eb145187b13a 100644 --- a/test/metabase/models/user_test.clj +++ b/test/metabase/models/user_test.clj @@ -415,8 +415,9 @@ (is (= "en_US" (db/select-one-field :locale User :id user-id))))) (testing "invalid locale" - (is (thrown? - AssertionError + (is (thrown-with-msg? + Exception + #"Assert failed: \(i18n/available-locale\? locale\)" (mt/with-temp User [{user-id :id} {:locale "en_XX"}]))))) (testing "updating a User" @@ -426,8 +427,9 @@ (is (= "en_GB" (db/select-one-field :locale User :id user-id)))) (testing "invalid locale" - (is (thrown? + (is (thrown-with-msg? AssertionError + #"Assert failed: \(i18n/available-locale\? locale\)" (db/update! User user-id :locale "en_XX")))))))) (deftest normalize-locale-test diff --git a/test/metabase/pulse_test.clj b/test/metabase/pulse_test.clj index 24fe5f993f30b04f9fe1417fd3c37b646ecf09fa..b03647724bbc94395d9f67ed0eade66ffb8aa731 100644 --- a/test/metabase/pulse_test.clj +++ b/test/metabase/pulse_test.clj @@ -754,9 +754,8 @@ (send-pulse-created-by-user! user-kw card)))] (is (= [[1 "2014-04-07T00:00:00Z" 5 12]] (send-pulse-created-by-user!* :crowberto))) - (is (thrown-with-msg? - clojure.lang.ExceptionInfo - #"^You do not have permissions to view Card [\d,]+." - (mt/suppress-output - (send-pulse-created-by-user!* :rasta))) - "If the current user doesn't have permissions to execute the Card for a Pulse, an Exception should be thrown.")))) + (testing "If the current user doesn't have permissions to execute the Card for a Pulse, an Exception should be thrown." + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"You do not have permissions to view Card [\d,]+." + (send-pulse-created-by-user!* :rasta))))))) diff --git a/test/metabase/query_processor/middleware/permissions_test.clj b/test/metabase/query_processor/middleware/permissions_test.clj index 9eba1d41ea6fc1bc0859b3d4e20d9d337e169aae..621c49e939a776a94f388fb62f33fe2b8772422c 100644 --- a/test/metabase/query_processor/middleware/permissions_test.clj +++ b/test/metabase/query_processor/middleware/permissions_test.clj @@ -24,19 +24,20 @@ (defn- check-perms-for-rasta "Check permissions for `query` with rasta as the current user." - {:style/indent 0} [query] (do-with-rasta (fn [] (check-perms query)))) -(def ^:private perms-error-msg #"^You do not have permissions to run this query\.") +(def ^:private perms-error-msg #"You do not have permissions to run this query\.") (deftest native-query-perms-test (testing "Make sure the NATIVE query fails to run if current user doesn't have perms" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (check-perms-for-rasta - {:database 1000 - :type :native - :native {:query "SELECT * FROM VENUES"}})))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (check-perms-for-rasta + {:database 1000 + :type :native + :native {:query "SELECT * FROM VENUES"}})))) (testing "...but it should work if user has perms" (mt/with-temp Database [db] @@ -51,15 +52,17 @@ (deftest mbql-query-perms-test (testing "Make sure the MBQL query fails to run if current user doesn't have perms" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (mt/with-temp* [Database [db] - Table [table {:db_id (u/the-id db)}]] - ;; All users get perms for all new DBs by default - (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) - (check-perms-for-rasta - {:database (u/the-id db) - :type :query - :query {:source-table (u/the-id table)}}))))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (mt/with-temp* [Database [db] + Table [table {:db_id (u/the-id db)}]] + ;; All users get perms for all new DBs by default + (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) + (check-perms-for-rasta + {:database (u/the-id db) + :type :query + :query {:source-table (u/the-id table)}}))))) (testing "...but it should work if user has perms [MBQL]" (mt/with-temp* [Database [db] @@ -75,11 +78,13 @@ (deftest nested-native-query-test (testing "Make sure nested native query fails to run if current user doesn't have perms" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (check-perms-for-rasta - {:database 1000 - :type :query - :query {:source-query {:native "SELECT * FROM VENUES"}}})))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (check-perms-for-rasta + {:database 1000 + :type :query + :query {:source-query {:native "SELECT * FROM VENUES"}}})))) (testing "...but it should work if user has perms [nested native queries]" (mt/with-temp Database [db] @@ -94,15 +99,17 @@ (deftest nested-mbql-query-test (testing "Make sure nested MBQL query fails to run if current user doesn't have perms" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (mt/with-temp* [Database [db] - Table [table {:db_id (u/the-id db)}]] - ;; All users get perms for all new DBs by default - (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) - (check-perms-for-rasta - {:database (u/the-id db) - :type :query - :query {:source-query {:source-table (u/the-id table)}}}))))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (mt/with-temp* [Database [db] + Table [table {:db_id (u/the-id db)}]] + ;; All users get perms for all new DBs by default + (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) + (check-perms-for-rasta + {:database (u/the-id db) + :type :query + :query {:source-query {:source-table (u/the-id table)}}}))))) (testing "...but it should work if user has perms [nested MBQL queries]" (mt/with-temp* [Database [db] @@ -117,23 +124,25 @@ (deftest template-tags-referenced-queries-test (testing "Fails for MBQL query referenced in template tag, when user has no perms to referenced query" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (mt/with-temp* [Database [db] - Table [table-1 {:db_id (u/the-id db)}] - Table [table-2 {:db_id (u/the-id db)}] - Card [card {:dataset_query {:database (u/the-id db), :type :query, - :query {:source-table (u/the-id table-2)}}}]] - ;; All users get perms for all new DBs by default - (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db) nil (u/the-id table-2)) - (let [card-id (:id card) - tag-name (str "#" card-id)] - (check-perms-for-rasta - {:database (u/the-id db) - :type :native - :native {:query (format "SELECT * FROM {{%s}} AS x" tag-name) - :template-tags {tag-name - {:id tag-name, :name tag-name, :display-name tag-name, - :type "card", :card card-id}}}})))))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (mt/with-temp* [Database [db] + Table [table-1 {:db_id (u/the-id db)}] + Table [table-2 {:db_id (u/the-id db)}] + Card [card {:dataset_query {:database (u/the-id db), :type :query, + :query {:source-table (u/the-id table-2)}}}]] + ;; All users get perms for all new DBs by default + (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db) nil (u/the-id table-2)) + (let [card-id (:id card) + tag-name (str "#" card-id)] + (check-perms-for-rasta + {:database (u/the-id db) + :type :native + :native {:query (format "SELECT * FROM {{%s}} AS x" tag-name) + :template-tags {tag-name + {:id tag-name, :name tag-name, :display-name tag-name, + :type "card", :card card-id}}}})))))) (testing "...but it should work if user has perms [template tag referenced query]" (mt/with-temp* [Database [db] @@ -159,22 +168,24 @@ :type "card", :card card-id}}}})))))) (testing "Fails for native query referenced in template tag, when user has no perms to referenced query" - (is (thrown-with-msg? ExceptionInfo perms-error-msg - (mt/with-temp* [Database [db] - Card [card {:dataset_query - {:database (u/the-id db), :type :native, - :native {:query "SELECT 1 AS \"foo\", 2 AS \"bar\", 3 AS \"baz\""}}}]] - ;; All users get perms for all new DBs by default - (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) - (let [card-id (:id card) - tag-name (str "#" card-id)] - (check-perms-for-rasta - {:database (u/the-id db) - :type :native - :native {:query (format "SELECT * FROM {{%s}} AS x" tag-name) - :template-tags {tag-name - {:id tag-name, :name tag-name, :display-name tag-name, - :type "card", :card card-id}}}})))))) + (is (thrown-with-msg? + ExceptionInfo + perms-error-msg + (mt/with-temp* [Database [db] + Card [card {:dataset_query + {:database (u/the-id db), :type :native, + :native {:query "SELECT 1 AS \"foo\", 2 AS \"bar\", 3 AS \"baz\""}}}]] + ;; All users get perms for all new DBs by default + (perms/revoke-data-perms! (perms-group/all-users) (u/the-id db)) + (let [card-id (:id card) + tag-name (str "#" card-id)] + (check-perms-for-rasta + {:database (u/the-id db) + :type :native + :native {:query (format "SELECT * FROM {{%s}} AS x" tag-name) + :template-tags {tag-name + {:id tag-name, :name tag-name, :display-name tag-name, + :type "card", :card card-id}}}})))))) (testing "...but it should work if user has perms [template tag referenced query]" (mt/with-temp* [Database [db] diff --git a/test/metabase/test/redefs.clj b/test/metabase/test/redefs.clj index 2621b130cf35b07957c79e70c028c6009f8c886e..e0d93d004a2b2aa5693739a160a9269d7a5fda3c 100644 --- a/test/metabase/test/redefs.clj +++ b/test/metabase/test/redefs.clj @@ -19,7 +19,19 @@ ;; TODO -- there's not really a reason that we can't use with-temp in parallel tests -- it depends on the test -- so ;; once we're a little more comfortable with the current parallel stuff we should remove this restriction. (test-runner.parallel/assert-test-is-not-parallel "with-temp") - (apply orig-do-with-temp args)) + ;; catch any Exceptions thrown by the original call and rethrow them with some extra context to make them a little + ;; easier to debug. + (try + (apply orig-do-with-temp args) + (catch Throwable e + ;; only wrap the Exception with additional context if it's not already wrapped. We don't need a bunch of nested + ;; `do-with-temp` calls that didn't fail wrapping the Exception from the one that did and adding a bunch of + ;; noise + (if (::with-temp-error (ex-data e)) + (throw e) + (throw (ex-info (str "with-temp error: " (ex-message e)) + {:args args, ::with-temp-error true} + e)))))) (alter-var-root #'tt/do-with-temp (constantly do-with-temp))