1251643 - Investigate validating "core" pings against schemas in tests

Status: RESOLVED WORKSFORME

(Toolkit :: Telemetry, defect, P3)

Description

[Georg Fritzsche [:gfritzsche]]

We are adding a "core" ping schema in bug 1249925.
We should see whether we can add validation against this in existing or new tests.

This helps us testing the client implementation and keeping the schema up to date.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Michael, do you know if we have existing tests for the "core" pings that we could add ping validation to?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> Michael, do you know if we have existing tests for the "core" pings that we
> could add ping validation to?

There are no existing tests for the "core" pings.

fwiw, my current implementation thoughts for bug 1243585 are to rework the TelemetryPingGenerator into the "Builder" pattern, which would throw if the core ping is constructed without necessary fields. We could write a test against this builder object to provide validation for the schema on the client side. As such, it may be pertinent to wait for bug 1243585 to land before we invest in writing a schema validation test on the client.

Comment 3

[Georg Fritzsche [:gfritzsche]]

Thanks, that sounds good.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Actually, the builder landed in bug 1247489 and does validation against the required fields [1][2]. We then have tests to make sure the mandatory fields functionality is correct. [3] Georg, do you see other opportunities for testing? This seems adequate to me.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/telemetry/pings/TelemetryCorePingBuilder.java#74
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/telemetry/pings/TelemetryPingBuilder.java#53
[3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pings/TestTelemetryPingBuilder.java#19

Comment 5

[Georg Fritzsche [:gfritzsche]]

I think the schema integration would be great to ensure that the client conforms to the schema and that we update the schema regularly (we currently don't since i pushed the first schema).

Would it be hard to integrate this?
If not this would probably be really useful, especially longer-term once we are integrating validation better in the data pipeline.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> I think the schema integration would be great to ensure that the client
> conforms to the schema and that we update the schema regularly (we currently
> don't since i pushed the first schema).
> 
> Would it be hard to integrate this?

Actually, I don't understand what we're looking for here – from "ensure... that we update the schema regularly", do you mean we should check the json output and make sure it contains all of the required fields, possibly optional fields, and nothing else? That way when we add a field and run the test on the client, it will contain the new field and break the test if the schema version is not updated?

Comment 7

[Georg Fritzsche [:gfritzsche]]

The schema versioning situation is a bit undefined on the pipeline side right now.
I'll see that i get the current schema updated, but this bug doesn't seem too useful right now.

Let's close this for now and revisit later when its needed.