Merge pull request #4944 from metabase/test-cleanup

Don't use expect-with-temp when temp objects aren't used in expected

test/metabase/api/card_test.clj

@@ -211,11 +211,11 @@
   ((user->client :rasta) :get 200 (str "card/" (u/get-id card))))
 
 ;; Check that a user without permissions isn't allowed to fetch the card
-(tt/expect-with-temp [Database  [{database-id :id}]
-                      Table     [{table-id :id}    {:db_id database-id}]
-                      Card      [card              {:dataset_query (mbql-count-query database-id table-id)}]]
+(expect
   "You don't have permissions to do that."
-  (do
+  (tt/with-temp* [Database [{database-id :id}]
+                  Table    [{table-id :id}    {:db_id database-id}]
+                  Card     [card              {:dataset_query (mbql-count-query database-id table-id)}]]
     ;; revoke permissions for default group to this database
     (perms/delete-related-permissions! (perms-group/all-users) (perms/object-path database-id))
     ;; now a non-admin user shouldn't be able to fetch this card
@@ -480,13 +480,14 @@
         (db/delete! Card :id card-id)))))
 
 ;; Make sure we card creation fails if we try to set a `collection_id` we don't have permissions for
-(tt/expect-with-temp [Collection [collection]]
+(expect
   "You don't have permissions to do that."
-  ((user->client :rasta) :post 403 "card" {:name                   "My Cool Card"
-                                           :display                "scalar"
-                                           :dataset_query          (mbql-count-query (id) (id :venues))
-                                           :visualization_settings {:global {:title nil}}
-                                           :collection_id          (u/get-id collection)}))
+  (tt/with-temp Collection [collection]
+    ((user->client :rasta) :post 403 "card" {:name                   "My Cool Card"
+                                             :display                "scalar"
+                                             :dataset_query          (mbql-count-query (id) (id :venues))
+                                             :visualization_settings {:global {:title nil}}
+                                             :collection_id          (u/get-id collection)})))
 
 ;; Make sure we can change the `collection_id` of a Card if it's not in any collection
 (tt/expect-with-temp [Card       [card]
@@ -497,26 +498,27 @@
     (db/select-one-field :collection_id Card :id (u/get-id card))))
 
 ;; Make sure we can still change *anything* for a Card if we don't have permissions for the Collection it belongs to
-(tt/expect-with-temp [Collection [collection]
-                      Card       [card       {:collection_id (u/get-id collection)}]]
+(expect
   "You don't have permissions to do that."
-  ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:name "Number of Blueberries Consumed Per Month"}))
+  (tt/with-temp* [Collection [collection]
+                  Card       [card       {:collection_id (u/get-id collection)}]]
+    ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:name "Number of Blueberries Consumed Per Month"})))
 
 ;; Make sure that we can't change the `collection_id` of a Card if we don't have write permissions for the new collection
-(tt/expect-with-temp [Collection [original-collection]
-                      Collection [new-collection]
-                      Card       [card                {:collection_id (u/get-id original-collection)}]]
+(expect
   "You don't have permissions to do that."
-  (do
+  (tt/with-temp* [Collection [original-collection]
+                  Collection [new-collection]
+                  Card       [card                {:collection_id (u/get-id original-collection)}]]
     (perms/grant-collection-readwrite-permissions! (perms-group/all-users) original-collection)
     ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})))
 
 ;; Make sure that we can't change the `collection_id` of a Card if we don't have write permissions for the current collection
-(tt/expect-with-temp [Collection [original-collection]
-                      Collection [new-collection]
-                      Card       [card                {:collection_id (u/get-id original-collection)}]]
+(expect
   "You don't have permissions to do that."
-  (do
+  (tt/with-temp* [Collection [original-collection]
+                  Collection [new-collection]
+                  Card       [card                {:collection_id (u/get-id original-collection)}]]
     (perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
     ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})))
 
@@ -606,20 +608,22 @@
   (POST-card-collections! :crowberto 200 new-collection [card-1 card-2]))
 
 ;; Test that we can bulk remove some Cards from a collection
-(tt/expect-with-temp [Collection [collection]
-                      Card       [card-1     {:collection_id (u/get-id collection)}]
-                      Card       [card-2     {:collection_id (u/get-id collection)}]]
+(expect
   [{:status "ok"}
    [nil nil]]
-  (POST-card-collections! :crowberto 200 nil [card-1 card-2]))
+  (tt/with-temp* [Collection [collection]
+                  Card       [card-1     {:collection_id (u/get-id collection)}]
+                  Card       [card-2     {:collection_id (u/get-id collection)}]]
+    (POST-card-collections! :crowberto 200 nil [card-1 card-2])))
 
 ;; Check that we aren't allowed to move Cards if we don't have permissions for destination collection
-(tt/expect-with-temp [Collection [collection]
-                      Card       [card-1]
-                      Card       [card-2]]
+(expect
   ["You don't have permissions to do that."
    [nil nil]]
-  (POST-card-collections! :rasta 403 collection [card-1 card-2]))
+  (tt/with-temp* [Collection [collection]
+                  Card       [card-1]
+                  Card       [card-2]]
+    (POST-card-collections! :rasta 403 collection [card-1 card-2])))
 
 ;; Check that we aren't allowed to move Cards if we don't have permissions for source collection
 (tt/expect-with-temp [Collection [collection]
@@ -630,14 +634,14 @@
   (POST-card-collections! :rasta 403 nil [card-1 card-2]))
 
 ;; Check that we aren't allowed to move Cards if we don't have permissions for the Card
-(tt/expect-with-temp [Collection [collection]
-                      Database   [database]
-                      Table      [table      {:db_id (u/get-id database)}]
-                      Card       [card-1     {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]
-                      Card       [card-2     {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]]
+(expect
   ["You don't have permissions to do that."
    [nil nil]]
-  (do
+  (tt/with-temp* [Collection [collection]
+                  Database   [database]
+                  Table      [table      {:db_id (u/get-id database)}]
+                  Card       [card-1     {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]
+                  Card       [card-2     {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]]
     (perms/revoke-permissions! (perms-group/all-users) (u/get-id database))
     (perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
     (POST-card-collections! :rasta 403 collection [card-1 card-2])))

test/metabase/api/collection_test.clj

@@ -18,27 +18,27 @@
   ((user->client :crowberto) :get 200 "collection"))
 
 ;; check that we don't see collections if we don't have permissions for them
-(tt/expect-with-temp [Collection [collection-1 {:name "Collection 1"}]
-                      Collection [collection-2 {:name "Collection 2"}]]
+(expect
   ["Collection 1"]
-  (do
+  (tt/with-temp* [Collection [collection-1 {:name "Collection 1"}]
+                  Collection [collection-2 {:name "Collection 2"}]]
     (perms/grant-collection-read-permissions! (group/all-users) collection-1)
     (map :name ((user->client :rasta) :get 200 "collection"))))
 
 ;; check that we don't see collections if they're archived
-(tt/expect-with-temp [Collection [collection-1 {:name "Archived Collection", :archived true}]
-                      Collection [collection-2 {:name "Regular Collection"}]]
+(expect
   ["Regular Collection"]
-  (do
+  (tt/with-temp* [Collection [collection-1 {:name "Archived Collection", :archived true}]
+                  Collection [collection-2 {:name "Regular Collection"}]]
     (perms/grant-collection-read-permissions! (group/all-users) collection-1)
     (perms/grant-collection-read-permissions! (group/all-users) collection-2)
     (map :name ((user->client :rasta) :get 200 "collection"))))
 
 ;; Check that if we pass `?archived=true` we instead see archived cards
-(tt/expect-with-temp [Collection [collection-1 {:name "Archived Collection", :archived true}]
-                      Collection [collection-2 {:name "Regular Collection"}]]
+(expect
   ["Archived Collection"]
-  (do
+  (tt/with-temp* [Collection [collection-1 {:name "Archived Collection", :archived true}]
+                  Collection [collection-2 {:name "Regular Collection"}]]
     (perms/grant-collection-read-permissions! (group/all-users) collection-1)
     (perms/grant-collection-read-permissions! (group/all-users) collection-2)
     (map :name ((user->client :rasta) :get 200 "collection" :archived :true))))
@@ -100,7 +100,8 @@
    {:name "My Beautiful Collection", :color "#ABCDEF"}))
 
 ;; check that non-admins aren't allowed to update a collection
-(tt/expect-with-temp [Collection [collection]]
+(expect
   "You don't have permissions to do that."
-  ((user->client :rasta) :put 403 (str "collection/" (u/get-id collection))
-   {:name "My Beautiful Collection", :color "#ABCDEF"}))
+  (tt/with-temp Collection [collection]
+    ((user->client :rasta) :put 403 (str "collection/" (u/get-id collection))
+     {:name "My Beautiful Collection", :color "#ABCDEF"})))

test/metabase/api/label_test.clj

@@ -9,8 +9,8 @@
 
 ;;; GET /api/label -- list all labels
 (tt/expect-with-temp [Label [{label-1-id :id} {:name "Toucan-Approved"}]
-                   Label [{label-2-id :id} {:name "non-Toucan-Approved"}]]
-  [{:id label-2-id, :name "non-Toucan-Approved", :slug "non_toucan_approved", :icon nil}  ; should be sorted by name, case-insensitive
+                      Label [{label-2-id :id} {:name "non-Toucan-Approved"}]]
+  [{:id label-2-id, :name "non-Toucan-Approved", :slug "non_toucan_approved", :icon nil} ; should be sorted by name, case-insensitive
    {:id label-1-id, :name "Toucan-Approved",     :slug "toucan_approved",     :icon nil}]
   ((user->client :rasta) :get 200, "label"))
 
@@ -27,7 +27,8 @@
   ((user->client :rasta) :put 200, (str "label/" label-id) {:name "Bird-Friendly"}))
 
 ;;; DELETE /api/label/:id -- delete a label
-(tt/expect-with-temp [Label [{label-id :id} {:name "This will make the toucan very cross!"}]]
+(expect
   nil
-  (do ((user->client :rasta) :delete 204, (str "label/" label-id))
-      (Label label-id)))
+  (tt/with-temp Label [{label-id :id} {:name "This will make the toucan very cross!"}]
+    ((user->client :rasta) :delete 204, (str "label/" label-id))
+    (Label label-id)))

test/metabase/api/pulse_test.clj

@@ -174,9 +174,9 @@
 
 
 ;; ## DELETE /api/pulse/:id
-(tt/expect-with-temp [Pulse [pulse]]
+(expect
   nil
-  (do
+  (tt/with-temp Pulse [pulse]
     ((user->client :rasta) :delete 204 (format "pulse/%d" (:id pulse)))
     (pulse/retrieve-pulse (:id pulse))))
 

test/metabase/api/segment_test.clj

@@ -372,10 +372,11 @@
 
 
 ;;; PUT /api/segment/id. Can I update a segment's name without specifying `:points_of_interest` and `:show_in_getting_started`?
-(tt/expect-with-temp [Segment [segment]]
-  :ok
-  (do ((user->client :crowberto) :put 200 (str "segment/" (u/get-id segment))
-       {:name             "Cool name"
-        :revision_message "WOW HOW COOL"
-        :definition       {}})
-      :ok))
+(expect
+  (tt/with-temp Segment [segment]
+    ;; just make sure API call doesn't barf
+    ((user->client :crowberto) :put 200 (str "segment/" (u/get-id segment))
+     {:name             "Cool name"
+      :revision_message "WOW HOW COOL"
+      :definition       {}})
+    true))