From 4472564b0a7bcdbe09d65796a9edf3e1ce0ee2ef Mon Sep 17 00:00:00 2001
From: Arthur Ulfeldt <arthur@ulfeldt.com>
Date: Tue, 2 May 2017 17:56:31 -0700
Subject: [PATCH] more code review improvements
---
test/metabase/sync_database/analyze_test.clj | 90 +++++++++++---------
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/test/metabase/sync_database/analyze_test.clj b/test/metabase/sync_database/analyze_test.clj
index c5273444712..b6da17af15c 100644
--- a/test/metabase/sync_database/analyze_test.clj
+++ b/test/metabase/sync_database/analyze_test.clj
@@ -88,8 +88,8 @@
(db/count Field :table_id table-id)))
(db/count Field :last_analyzed nil :table_id table-id)))
-(defn latest-sync-time [db_id table-name]
- (let [table-id (db/select-one-id Table, :db_id db_id, :active true :name table-name)]
+(defn latest-sync-time [table]
+ (let [table-id (db/select-one-id Table :db_id (:db_id table) :active true :name (:name table))]
(db/select-field :last_analyzed Field :table_id table-id)))
(defn set-table-visibility-type! [table visibility-type]
@@ -106,52 +106,58 @@
(analyze-data-shape-for-tables! (driver/database-id->driver db-id) {:id db-id})))
;; expect all the kinds of hidden tables to stay un-analyzed through transitions and repeated syncing
-(tt/expect-with-temp [Table [table {:rows 15}]
- Field [field {:table_id (:id table)}]]
+(expect
1
- (do (set-table-visibility-type! table "hidden")
- (api-sync-call table)
- (set-table-visibility-type! table "cruft")
- (set-table-visibility-type! table "cruft")
- (api-sync-call table)
- (set-table-visibility-type! table "technical")
- (api-sync-call table)
- (set-table-visibility-type! table "technical")
- (api-sync-call table)
- (api-sync-call table)
- (count-unanalyzed-fields (:db_id table) (:name table))))
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+
+ (do (set-table-visibility-type! table "hidden")
+ (api-sync-call table)
+ (set-table-visibility-type! table "cruft")
+ (set-table-visibility-type! table "cruft")
+ (api-sync-call table)
+ (set-table-visibility-type! table "technical")
+ (api-sync-call table)
+ (set-table-visibility-type! table "technical")
+ (api-sync-call table)
+ (api-sync-call table)
+ (count-unanalyzed-fields (:db_id table) (:name table)))))
;; same test not coming through the api
-(tt/expect-with-temp [Table [table {:rows 15}]
- Field [field {:table_id (:id table)}]]
+(expect
1
- (do (set-table-visibility-type! table "hidden")
- (sync-call table)
- (set-table-visibility-type! table "cruft")
- (set-table-visibility-type! table "cruft")
- (sync-call table)
- (set-table-visibility-type! table "technical")
- (sync-call table)
- (set-table-visibility-type! table "technical")
- (sync-call table)
- (sync-call table)
- (count-unanalyzed-fields (:db_id table) (:name table))))
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+ (do (set-table-visibility-type! table "hidden")
+ (sync-call table)
+ (set-table-visibility-type! table "cruft")
+ (set-table-visibility-type! table "cruft")
+ (sync-call table)
+ (set-table-visibility-type! table "technical")
+ (sync-call table)
+ (set-table-visibility-type! table "technical")
+ (sync-call table)
+ (sync-call table)
+ (count-unanalyzed-fields (:db_id table) (:name table)))))
;; un-hiding a table should cause it to be analyzed
-(tt/expect-with-temp [Table [table {:rows 15}]
- Field [field {:table_id (:id table)}]]
+(expect
0
- (do (set-table-visibility-type! table "hidden")
- (set-table-visibility-type! table nil)
- (count-unanalyzed-fields (:db_id table) (:name table))))
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+ (do (set-table-visibility-type! table "hidden")
+ (set-table-visibility-type! table nil)
+ (count-unanalyzed-fields (:db_id table) (:name table)))))
;; re-hiding a table should not cause it to be analyzed
-(tt/expect-with-temp [Table [table {:rows 15}]
- Field [field {:table_id (:id table)}]]
- true
- (do (set-table-visibility-type! table "hidden") ;; start with it not visible
- (set-table-visibility-type! table nil) ;; make it visible to trigger analysis
- (let [initial-sync-time (latest-sync-time (:db_id table) (:name table))]
- ;; make it invisible which should not trigger analysis
- (set-table-visibility-type! table "hidden")
- (= initial-sync-time (latest-sync-time (:db_id table) (:name table))))))
+(expect
+ ;; create an initially hidden table
+ (tt/with-temp* [Table [table {:rows 15, :visibility_type "hidden"}]
+ Field [field {:table_id (:id table)}]]
+ ;; switch the table to visible (triggering a sync) and get the last sync time
+ (let [last-sync-time (do (set-table-visibility-type! table nil)
+ (latest-sync-time table))]
+ ;; now make it hidden again
+ (set-table-visibility-type! table "hidden")
+ ;; sync time shouldn't change
+ (= last-sync-time (latest-sync-time table)))))
--
GitLab