Follow-up from "Add spec to test import->export equivalence"
The following points from !23338 (merged) should be addressed:
We had to ignore several attributes from the project tree when performing comparison checks between imports and exports for the test to pass. The more fields we exclude, the less useful this test is. Ideally we would include all of these things in the comparison, but it was not possible in the original MR to do so without either a lot of added complexity, or further investigation into domain behavior, or both. Some of the fields listed below might be OK to exclude, but someone with more experience in the import/export product areas should verify.
(Note: I'm not listing fields that are currently excluded but were OK'ed to be excluded in the original MR, such as row IDs.)
The goal of this issue is to:
- Go through the fields below and decide whether they are OK to be ignored in comparisons or not.
- If they should be included in the comparison, decide for each field:
- whether it's a problem with the app; create a follow-up issue and fix it, then remove the
:ignored
for this field - whether it's just a test issue; consider including a special case in the test for it (e.g. a custom matcher) instead of ignoring it altogether
- whether it's a problem with the app; create a follow-up issue and fix it, then remove the
Below is a list of what is currently ignored and why:
-
Array order. We currently perform
Set
based equality checks on lists of equal length so that we can compare orderless at every level of the tree for equal content. We did this because some relation lists would come back in a different order and I don't know whether these have a well defined order or not. - Unaccounted list elements. In some sense we have false negatives in our checks currently, because we do not catch inequalities where the re-exported project tree contains extra elements that weren't in the initial tree (we ignore those currently.) Ideally this shouldn't happen, but because we know the exporter to produce lots of duplicate data, these can appear as additional items and hence make arrays incomparable (due to different length) unless we scan into them element by element based on the left hand side of the comparison.
-
notes. For some reason we do not import all entities from an exported JSON, but instead (re)create some of them on-the-fly, particularly user related data. This failed the comparison test, since global auto-generated test data using monotonic IDs were used, and which differ between every test run. Affected fields are
username
,name
,email
,email_alias
. See alsospec/factories/sequences.rb
. -
next_run_at
. This pipelines related field appears to change between test runs based on wall clock time and was therefore neutralized in the test.
Other questions:
- In the MR, the question was raised that if we ignore
token
s, should we also ignorerunner_token
,rss-token
and others?