1063691 - Switch test_log_view_artifact_builder.py tests to be schema validation assertions instead

Status: RESOLVED DUPLICATE of bug 1397394

(Tree Management :: Treeherder: Data Ingestion, defect, P5)

Description

[Ed Morley [:emorley]]

(In reply to Jonathan Eads ( :jeads ) from bug 1043741comment #9)
> 
> (In reply to Ed Morley [:edmorley] from bug 1043741 comment #7)
> > * The tests were updated by uncommenting that block in
> > test_log_view_artifact_builder.py, but since the dicts are unordered, we're
> > at the mercy of whatever order Python decides to use, which is why the test
> > diffs are larger (and less readable) than I would have liked.
> 
> The testing strategy here,
> https://github.com/mozilla/treeherder-service/blob/master/tests/log_parser/
> test_log_view_artifact_builder.py#L48, is brittle and will likely keep
> biting us as we continue to modify the log view artifact in the future. A
> better strategy would be to maintain explicit json schemas for json
> artifacts when treeherder has hard structure requirements. The jsonschema
> module is already in our vendor lib
> https://github.com/mozilla/treeherder-service/tree/master/vendor/jsonschema
> so this is just a matter of defining the schema. 
> 
> If we do this, we can modify the test assertion to be schema validation
> results, which would be much easier to maintain going forward. Would also
> allow us to validate structures at run time in production so we can log
> errors when we hit them. This is out of the scope of this PR but should be
> added as a refactor task for treeherder.

Comment 1

[David Burns :automatedtester]

Can we validate the tests or if this is working as it is should be?

Comment 2

[Cameron Dawson [:camd]]

We actually don't store this artifact in the form that the artifact builder creates any longer.  We take that artifact and create entries in the TextLogStep and TextLogError models/tables.  And the entries in that model are used in the current Log Viewer to show steps.


So a better solution to this would be to remove all the "golden" files to compare against.  We could then validate that the right entries in TextLogStep and TextLogErrors were created.
https://github.com/mozilla/treeherder/blob/78dc024094a8d6b5c08fbb3e41b968d40aa0c16c/treeherder/etl/artifact.py#L50-L87

In conclusion.  These tests are still valid, but (as stated earlier) brittle.  We should refactor them to make them better.