1409860 - Move Android sync ping "os" field to payload.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, enhancement)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

It's currently in the top level, which is wrong.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Already in this file and the patch is trivial, might as well just do it.

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1409860 - Move os to correct place in android sync ping

Review commit: https://reviewboard.mozilla.org/r/191308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/191308/

Comment 3

[:Grisha Kruglov]

Comment on attachment 8920281 [details]
Bug 1409860 - Move os to correct place in android sync ping

https://reviewboard.mozilla.org/r/191308/#review196462

::: mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java:130
(Diff revision 1)
>          assertTrue(application.containsKey("channel"));
>          assertTrue(application.containsKey("buildId"));
>          assertTrue(application.containsKey("xpcomAbi"));
>  
>          // Test os key.
> -        ExtendedJSONObject os = outgoingPing.getPayload().getObject("os");
> +        ExtendedJSONObject os = outgoingPing.getPayload().getObject("payload").getObject("os");

Could "merge" this test with the code just below.

::: mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java:139
(Diff revision 1)
>          // Test for presence and type to void random failures.
>          assertTrue(os.getIntegerSafely("version") != null);
>          // Likely "en-US" in tests, but let's test for presence and type to avoid random failures.
>          assertTrue(os.getString("locale") != null);
>  
>          // Test general shape of payload. Expecting {"syncs":[],"why":"schedule", "version": 1}.

Perhaps change this comment too?

Comment 4

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8920281 [details]
Bug 1409860 - Move os to correct place in android sync ping

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/191308/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2cb1033ae11
Move os to correct place in android sync ping r=Grisha

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b2cb1033ae11