Skip to content
Snippets Groups Projects
Unverified Commit 312a46bd authored by Chris Truter's avatar Chris Truter Committed by GitHub
Browse files

Add test against serialization YAML format changing (#47349)

parent 1e78f8bd
No related branches found
No related tags found
No related merge requests found
Showing
with 669 additions and 5 deletions
...@@ -77,6 +77,7 @@ target/checksum.txt ...@@ -77,6 +77,7 @@ target/checksum.txt
xcshareddata xcshareddata
xcuserdata xcuserdata
dev/src/dev/nocommit/ dev/src/dev/nocommit/
dev/serialization_deltas/
*~ *~
**/cypress_sample_database.json **/cypress_sample_database.json
**/cypress_sample_instance_data.json **/cypress_sample_instance_data.json
......
...@@ -91,11 +91,14 @@ ...@@ -91,11 +91,14 @@
kw-id (keyword id)] kw-id (keyword id)]
(if (= ["Setting"] (mapv :model abs-path)) (if (= ["Setting"] (mapv :model abs-path))
{:serdes/meta abs-path :key kw-id :value (get settings kw-id)} {:serdes/meta abs-path :key kw-id :value (get settings kw-id)}
(->> abs-path (try
strip-labels (->> abs-path
(get @cache) strip-labels
second (get @cache)
ingest-file))))) second
ingest-file)
(catch Exception e
(throw (ex-info "Unable to ingest file" {:abs-path abs-path} e))))))))
(defn ingest-yaml (defn ingest-yaml
"Creates a new Ingestable on a directory of YAML files, as created by "Creates a new Ingestable on a directory of YAML files, as created by
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
[java-time.api :as t] [java-time.api :as t]
[metabase-enterprise.serialization.test-util :as ts] [metabase-enterprise.serialization.test-util :as ts]
[metabase-enterprise.serialization.v2.extract :as extract] [metabase-enterprise.serialization.v2.extract :as extract]
[metabase-enterprise.serialization.v2.round-trip-test :as round-trip-test]
[metabase.audit :as audit] [metabase.audit :as audit]
[metabase.core :as mbc] [metabase.core :as mbc]
[metabase.models [metabase.models
...@@ -32,6 +33,10 @@ ...@@ -32,6 +33,10 @@
[metabase.util :as u] [metabase.util :as u]
[toucan2.core :as t2])) [toucan2.core :as t2]))
(comment
;; Use this spell in your test body to add the given fixtures to the round trip baseline.
(round-trip-test/add-to-baseline!))
(defn- by-model [model-name extraction] (defn- by-model [model-name extraction]
(->> extraction (->> extraction
(into []) (into [])
...@@ -330,6 +335,7 @@ ...@@ -330,6 +335,7 @@
_ _
{:action_id action-id {:action_id action-id
:dashboard_id other-dash-id}] :dashboard_id other-dash-id}]
(testing "table and database are extracted as [db schema table] triples" (testing "table and database are extracted as [db schema table] triples"
(let [ser (serdes/extract-one "Card" {} (t2/select-one Card :id c1-id))] (let [ser (serdes/extract-one "Card" {} (t2/select-one Card :id c1-id))]
(is (=? {:serdes/meta [{:model "Card" :id c1-eid :label "some_question"}] (is (=? {:serdes/meta [{:model "Card" :id c1-eid :label "some_question"}]
......
;; This main test here is like an inside-out e2e test.
;;
;; It starts with an export, which it loads and then exports again.
;; By comparing the export both before and after, we can find unwanted divergences.
;; If the tests fail, and the in-memory data diffs are unclear, the relevant YAML files are also written
;; to dev/serialization_deltas, where you can diff them using the tool of your choice.
;;
;; There is also a coverage test, which checks if our fixture is missing any models, or have old cruft.
;;
;; If you've been sent here by the coverage test, here are some pointers:
;;
;; If there are models missing, go to theirs corresponding tests in [[v2.extract-test]], which you may still
;; need to create, and add a call to `round-trip-test/add-to-baseline!` immediately inside the database
;; initialization body; Run each test, then remove the calls again.
;; This will dump additional files into the baseline fixture folder, which you'll need to commit.
;; Its important that you first run [[baseline-completeness-test]] though to check that the dump is
;; consistent with the existing contents.
;; You may need to do some minor cleanup, things like the database sync status may have changed.
;; You can do this cleanup by hand, or run `(update-baseline!)` to normalize the discrepancies.
;;
;; To remove old cruft simply run `(update-baseline!)` (see Rich comment at the bottom)
(ns metabase-enterprise.serialization.v2.round-trip-test
(:require
[clojure.data :as data]
[clojure.java.io :as io]
[clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer [deftest is testing]]
[clojure.walk :as walk]
[metabase-enterprise.serialization.v2.extract :as extract]
[metabase-enterprise.serialization.v2.ingest :as ingest]
[metabase-enterprise.serialization.v2.load :as load]
[metabase-enterprise.serialization.v2.models :as serdes.models]
[metabase-enterprise.serialization.v2.storage :as storage]
[metabase.models.serialization :as serdes]
[metabase.test :as mt]
[metabase.util.log :as log]
[metabase.util.yaml :as yaml])
(:import
(java.io File)
(java.nio.file Files Path StandardCopyOption)
(java.nio.file.attribute FileAttribute)))
(set! *warn-on-reflection* true)
(def ^File source-dir (io/as-file (io/resource "serialization_baseline")))
(def ^File dev-inspect-dir (io/as-file (File. "dev/serialization_deltas")))
(def ignored-fields
;; Is worth considering when adding entries here, whether they shouldn't just be skipped in extraction.
#{:cache_field_values_schedule
:metadata_sync_schedule
:metabase_version})
(defn- strip-base-path [base file]
(str/replace-first file (str base File/separator) ""))
(defn fileset
"Return a set of all the nested filenames within a directory."
[dir]
(->> (file-seq dir)
(filter #(.isFile ^File %))
(map #(.getPath ^File %))
(map (partial strip-base-path dir))
(into (sorted-set))))
(defn read-yaml
"Reads a YAML file and returns Clojure data, with ignored fields removed."
[file]
(walk/postwalk
(fn [x]
(if-not (map? x)
x
(reduce dissoc x ignored-fields)))
(yaml/parse-string (slurp file))))
(defn non-empty-diff [diff]
(when (some identity diff)
diff))
(defn compare-files
"Test for differences between two YAML files. Ignores some keys and some key ordering."
[file1 file2]
(->> (data/diff (read-yaml file1)
(read-yaml file2))
(take 2)
non-empty-diff))
(defn load-extract!
"Perform a round-trip of loading an existing serialization, then serializing it again."
[input-dir output-dir]
(serdes/with-cache
(load/load-metabase! (ingest/ingest-yaml input-dir)))
;; Use a separate cache to make sure there is no cross-contamination.
(serdes/with-cache
(-> (extract/extract {:include-field-values true})
(storage/store! output-dir))))
(defn- delete-dir-contents! [^File dir]
(when (and dir (.exists dir))
(->> (file-seq dir)
(remove #{dir})
reverse
(run! #(.delete ^File %)))))
(defn- create-files-to-diff!
"Leave behind artifacts for closer inspection, e.g. via a CLI diff tool"
[ref-file out-file]
(let [base-filename (str dev-inspect-dir File/separator (strip-base-path source-dir ref-file))]
(let [file-path (File. base-filename)
parent (.getParentFile file-path)]
(when parent
(Files/createDirectories (.toPath parent) (into-array FileAttribute []))))
(spit (str base-filename ".baseline") (slurp ref-file))
(spit (str base-filename ".actual") (slurp out-file))))
(defn- temp-dir ^Path [base-path]
(Files/createTempDirectory base-path (make-array FileAttribute 0)))
(def source-dir-path (strip-base-path (File. (System/getProperty "user.dir")) source-dir))
(def ^:private internal-model?
#{"Schema"})
(defn add-to-baseline!
"Use this within v2.extract-test where relevant to add their fixtures to the baseline."
[]
(storage/store! (into [] (extract/extract {:include-field-values true})) source-dir))
;; If this test is failing, read the docstring at the top of this namespace for what to do B-)
(deftest baseline-completeness-test
(let [ingestable (ingest/ingest-yaml source-dir)
resources (ingest/ingest-list ingestable)
baselined (into #{} (map :model) (apply concat resources))
necessary? (set serdes.models/exported-models)]
(doseq [m serdes.models/exported-models]
(is (baselined m) (format "We need to add %s entries to %s" m source-dir-path)))
(doseq [b baselined :when (not (internal-model? b))]
(is (necessary? b) (format "We can remove %s files from %s" b source-dir-path)))))
(deftest baseline-comparison-test
(let [wrote-files? (volatile! false)
temp-dir (temp-dir "serialization_test")
output-dir (.toFile (.resolve temp-dir "serialization_output"))]
(try
(mt/with-empty-h2-app-db
(delete-dir-contents! dev-inspect-dir)
(load-extract! source-dir output-dir)
(let [source-files (fileset source-dir)
output-files (fileset output-dir)
missing-files (set/difference source-files output-files)
added-files (set/difference output-files source-files)]
(testing "No files are missing"
(is (empty? missing-files)))
(testing "No files have been added"
(is (empty? added-files)))
(testing "File contents\n"
(doseq [file source-files
:let [ref-file (io/file source-dir file)
out-file (io/file output-dir file)]
:when (.exists out-file)
:let [delta (compare-files ref-file out-file)]]
(is (nil? delta)
(str "Content mismatch for file: " (strip-base-path source-dir file)))
;; Leave behind files for developers to inspect
(when (and (.exists dev-inspect-dir) delta)
(vreset! wrote-files? true)
(create-files-to-diff! ref-file out-file))))
(when @wrote-files?
(log/warn "Mismatching files have been written to /dev/serialization_deltas"))))
(finally
(delete-dir-contents! output-dir)))))
(defn- update-baseline!
"Run this if you've examined the output of [[directory-comparison-test]], and are happy to accept the changes."
[]
(let [temp-dir (temp-dir "serialization_test")
output-dir (.toFile (.resolve temp-dir "serialization_output"))]
(mt/with-empty-h2-app-db
(load-extract! source-dir output-dir))
(delete-dir-contents! source-dir)
(Files/move (.toPath output-dir)
(.toPath source-dir)
(into-array [StandardCopyOption/REPLACE_EXISTING]))))
(comment
(delete-dir-contents! source-dir)
(update-baseline!))
archived: false
created_at: '2024-08-28T14:29:47.386823Z'
creator_id: ann@heart.band
description: null
entity_id: hYWoE-l1BpOqmvYedqP2g
http: []
implicit:
- kind: row/update
made_public_by_id: crowberto@metabase.com
model_id: aqpA3mIKUnfzYUlwjuGwT
name: My Action
parameter_mappings: []
parameters: []
public_uuid: 107f3af5-e51d-49aa-b332-97b1d61e52ad
query: []
type: implicit
visualization_settings: null
serdes/meta:
- id: hYWoE-l1BpOqmvYedqP2g
label: my_action
model: Action
name: Some Collection
description: null
entity_id: M-Q4pcV0qkiyJ0kiSWECl
slug: some_collection
created_at: '2024-08-28T09:46:18.671622Z'
archived: false
type: null
parent_id: null
personal_owner_id: null
namespace: null
authority_level: null
serdes/meta:
- id: M-Q4pcV0qkiyJ0kiSWECl
label: some_collection
model: Collection
archive_operation_id: null
archived_directly: null
is_sample: false
name: Series Question A
description: null
entity_id: OMuZ0wHe2O5Z_59-cLmn4
created_at: '2024-08-28T09:46:24.719281Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: M-Q4pcV0qkiyJ0kiSWECl
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: OMuZ0wHe2O5Z_59-cLmn4
label: series_question_a
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: Series Question B
description: null
entity_id: XsxiHuzwlGIFNq245HdZC
created_at: '2024-08-28T09:46:24.72343Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: M-Q4pcV0qkiyJ0kiSWECl
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: XsxiHuzwlGIFNq245HdZC
label: series_question_b
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: Some Question
description: null
entity_id: f1C68pznmrpN1F5xFDj6d
created_at: '2024-08-28T09:46:24.692002Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: M-Q4pcV0qkiyJ0kiSWECl
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: f1C68pznmrpN1F5xFDj6d
label: some_question
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: Shared Dashboard
description: null
entity_id: Q_jD-f-9clKLFZ2TfUG2h
created_at: '2024-08-28T09:46:24.726993Z'
creator_id: rasta@metabase.com
archived: false
collection_id: M-Q4pcV0qkiyJ0kiSWECl
auto_apply_filters: true
collection_position: null
position: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
show_in_getting_started: false
caveats: null
points_of_interest: null
parameters: []
serdes/meta:
- id: Q_jD-f-9clKLFZ2TfUG2h
label: shared_dashboard
model: Dashboard
archived_directly: false
dashcards:
- entity_id: UkpFcfUZMZt9ehChwnrAO
card_id: f1C68pznmrpN1F5xFDj6d
created_at: '2024-08-28T09:46:24.733016Z'
row: 0
col: 0
size_x: 4
size_y: 4
action_id: null
dashboard_tab_id: null
parameter_mappings: []
series:
- card_id: OMuZ0wHe2O5Z_59-cLmn4
position: 0
- card_id: XsxiHuzwlGIFNq245HdZC
position: 1
visualization_settings:
column_settings: null
serdes/meta:
- id: Q_jD-f-9clKLFZ2TfUG2h
model: Dashboard
- id: UkpFcfUZMZt9ehChwnrAO
model: DashboardCard
- entity_id: AlYMOYAhCXhr1VIiH5umt
card_id: OMuZ0wHe2O5Z_59-cLmn4
created_at: '2024-08-28T09:46:24.737505Z'
row: 0
col: 0
size_x: 4
size_y: 4
action_id: null
dashboard_tab_id: null
parameter_mappings: []
series: []
visualization_settings:
column_settings: null
serdes/meta:
- id: Q_jD-f-9clKLFZ2TfUG2h
model: Dashboard
- id: AlYMOYAhCXhr1VIiH5umt
model: DashboardCard
initially_published_at: null
tabs: []
width: fixed
name: Shared Collection
description: null
entity_id: Y6d4QwJgGKw-X1tRh3ir2
slug: shared_collection
created_at: '2024-08-28T14:36:15.259947Z'
archived: false
type: null
parent_id: null
personal_owner_id: null
namespace: snippets
authority_level: null
serdes/meta:
- id: Y6d4QwJgGKw-X1tRh3ir2
label: shared_collection
model: Collection
archive_operation_id: null
archived_directly: null
is_sample: false
name: Source question
description: null
entity_id: aqpA3mIKUnfzYUlwjuGwT
created_at: '2024-08-28T14:29:47.378643Z'
creator_id: ann@heart.band
display: table
archived: false
collection_id: null
collection_preview: true
collection_position: null
query_type: native
database_id: My Database
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query:
database: My Database
native:
native: select 1
type: native
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: aqpA3mIKUnfzYUlwjuGwT
label: source_question
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: model
name: root card
description: null
entity_id: ze7O4-8qRFH1v2Ho2apT1
created_at: '2024-08-28T12:50:14.404226Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: null
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: ze7O4-8qRFH1v2Ho2apT1
label: root_card
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: grandparent card
description: null
entity_id: m98z04nYGCF5n7cvOTfbj
created_at: '2024-08-28T12:50:14.408123Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: olgKH56lYOjakuobH4zPz
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: m98z04nYGCF5n7cvOTfbj
label: grandparent_card
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: Grandparent Collection
description: null
entity_id: olgKH56lYOjakuobH4zPz
slug: grandparent_collection
created_at: '2024-08-28T12:50:13.265991Z'
archived: false
type: null
parent_id: null
personal_owner_id: null
namespace: null
authority_level: null
serdes/meta:
- id: olgKH56lYOjakuobH4zPz
label: grandparent_collection
model: Collection
archive_operation_id: null
archived_directly: null
is_sample: false
name: parent card
description: null
entity_id: IBZSKSt2-QOMviIaGcQE6
created_at: '2024-08-28T12:50:14.411803Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: qPZhJMPNGkfd3sq8jWiZS
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: IBZSKSt2-QOMviIaGcQE6
label: parent_card
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: parent dash
description: null
entity_id: JT3BKJwNCfhUHnd9oqQ_1
created_at: '2024-08-28T12:50:14.419489Z'
creator_id: rasta@metabase.com
archived: false
collection_id: qPZhJMPNGkfd3sq8jWiZS
auto_apply_filters: true
collection_position: null
position: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
show_in_getting_started: false
caveats: null
points_of_interest: null
parameters: []
serdes/meta:
- id: JT3BKJwNCfhUHnd9oqQ_1
label: parent_dash
model: Dashboard
archived_directly: false
dashcards: []
initially_published_at: null
tabs: []
width: fixed
name: child card
description: null
entity_id: Ra0JVOCXKTxH5TPbwZm0x
created_at: '2024-08-28T12:50:14.415343Z'
creator_id: rasta@metabase.com
display: table
archived: false
collection_id: fJVsXIDzzipSZsaro9Pvu
collection_preview: true
collection_position: null
query_type: null
database_id: test-data (h2)
table_id: null
enable_embedding: false
embedding_params: null
made_public_by_id: null
public_uuid: null
parameters: []
parameter_mappings: []
dataset_query: {}
result_metadata: null
visualization_settings:
column_settings: null
serdes/meta:
- id: Ra0JVOCXKTxH5TPbwZm0x
label: child_card
model: Card
archived_directly: false
metabase_version: v1.48.1-SNAPSHOT (53cf083)
source_card_id: null
type: question
name: Child Collection
description: null
entity_id: fJVsXIDzzipSZsaro9Pvu
slug: child_collection
created_at: '2024-08-28T12:50:13.31301Z'
archived: false
type: null
parent_id: qPZhJMPNGkfd3sq8jWiZS
personal_owner_id: null
namespace: null
authority_level: null
serdes/meta:
- id: fJVsXIDzzipSZsaro9Pvu
label: child_collection
model: Collection
archive_operation_id: null
archived_directly: null
is_sample: false
name: Parent Collection
description: null
entity_id: qPZhJMPNGkfd3sq8jWiZS
slug: parent_collection
created_at: '2024-08-28T12:50:13.270754Z'
archived: false
type: null
parent_id: olgKH56lYOjakuobH4zPz
personal_owner_id: null
namespace: null
authority_level: null
serdes/meta:
- id: qPZhJMPNGkfd3sq8jWiZS
label: parent_collection
model: Collection
archive_operation_id: null
archived_directly: null
is_sample: false
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment