Closed
Bug 1251643
Opened 9 years ago
Closed 8 years ago
Investigate validating "core" pings against schemas in tests
Categories
(Toolkit :: Telemetry, defect, P3)
Toolkit
Telemetry
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
firefox47 | --- | affected |
People
(Reporter: gfritzsche, Assigned: mcomella)
References
Details
(Whiteboard: [measurement:client])
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.
Reporter | ||
Comment 1•9 years ago
|
||
Michael, do you know if we have existing tests for the "core" pings that we could add ping validation to?
Flags: needinfo?(michael.l.comella)
Reporter | ||
Updated•9 years ago
|
Priority: P2 → P3
Assignee | ||
Comment 2•9 years ago
|
||
(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.
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 4•9 years ago
|
||
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
Reporter | ||
Comment 5•9 years ago
|
||
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.
Flags: needinfo?(gfritzsche)
Assignee | ||
Comment 6•9 years ago
|
||
(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?
Flags: needinfo?(gfritzsche)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → michael.l.comella
Reporter | ||
Comment 7•8 years ago
|
||
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.
Flags: needinfo?(gfritzsche)
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•