Move Android sync ping "os" field to payload.

RESOLVED FIXED in Firefox 58

Status

()

Firefox for Android
Android Sync
RESOLVED FIXED
a month ago
a month ago

People

(Reporter: tcsc, Assigned: tcsc)

Tracking

unspecified
Firefox 58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a month ago
It's currently in the top level, which is wrong.
(Assignee)

Updated

a month ago
See Also: → bug 1409861
(Assignee)

Comment 1

a month ago
Already in this file and the patch is trivial, might as well just do it.
Assignee: nobody → tchiovoloni
Comment hidden (mozreview-request)

Updated

a month ago
See Also: → bug 1410145

Comment 3

a month ago
mozreview-review
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?
Attachment #8920281 - Flags: review?(gkruglov) → review+
Comment hidden (mozreview-request)

Comment 5

a month ago
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2cb1033ae11
Move os to correct place in android sync ping r=Grisha
https://hg.mozilla.org/mozilla-central/rev/b2cb1033ae11
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.