Thom suggested that if TPS validated any sync pings it generates against the schema we would be more likely to see issues which cause us to generate an invalid ping (eg, bug 1313495).

Comment on attachment 8808331 [details] Bug 1313573 - Validate the sync ping during TPS test runs https://reviewboard.mozilla.org/r/91160/#review90934 There is some substantially dubious parts of this patch. If you have an idea how to write it in a better way, LMK. ::: services/sync/tests/unit/sync_ping_schema.json:31 (Diff revision 1) > "type": "string", > "pattern": "^[0-9a-f]{32}$" > }, > "devices": { > "type": "array", > - "items": { "$ref": "#/definitions/engine" } > + "items": { "$ref": "#/definitions/device" } First bug caught by this feature. Let me know if I should move this into it's own patch. ::: services/sync/tps/extensions/tps/resource/tps.jsm:910 (Diff revision 1) > * > * This is called by RunTestPhase() after the environment is validated. > */ > _executeTestPhase: function _executeTestPhase(file, phase, settings) { > try { > + this.config = JSON.parse(prefs.getCharPref('tps.config')); This used to be written into the test file by the python code, which meant we used a tmpfile. Which of course, breaks the hacky things I do above to find the source root. While its not ideal to need to do those things, passing this via a pref is much cleaner IMO than having a implicitly added variable that's created via python. ::: services/sync/tps/extensions/tps/resource/tps.jsm (Diff revision 1) > Logger.logInfo("Starting phase " + this._currentPhase); > > Logger.logInfo("setting client.name to " + this.phases[this._currentPhase]); > Weave.Svc.Prefs.set("client.name", this.phases[this._currentPhase]); > > - // If a custom server was specified, set it now I removed this as part an initial attempt to get rid of the config variable (which it turns out we do need, hence adding it via a pref). That said, this code is either unused or broken, depending on which part, so it seems better to remove it alltogether. ::: services/sync/tps/extensions/tps/resource/tps.jsm:985 (Diff revision 1) > }; > SyncTelemetry.submit = record => { > Logger.logInfo("Intercepted sync telemetry submission: " + JSON.stringify(record)); > this._syncsReportedViaTelemetry += record.syncs.length + (record.discarded || 0); > if (record.discarded) { > - Logger.AssertTrue(record.syncs.length == SyncTelemetry.maxPayloadCount, > + if (record.syncs.length != SyncTelemetry.maxPayloadCount) { These wouldn't actually cause TPS to fail.

Comment on attachment 8808331 [details] Bug 1313573 - Validate the sync ping during TPS test runs https://reviewboard.mozilla.org/r/91160/#review90980 Looks great - a bit of a shame we don't have testing-common etc, but as we discussed, it's not worth spending a week on just for TPS.

Pushed by tchiovoloni@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c05a3ad32d21 Validate the sync ping during TPS test runs r=markh