1379025 - Sync ping "firm up" follow-up tests

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Metrics, defect)

Description

[:Grisha Kruglov]

I somehow entirely missed test changes in Bug 1374758.

Comment 1

[:Grisha Kruglov]

Attachment: Bug 1379025 - Update sync ping tests for the "firm up" changes

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

Comment 2

[:Grisha Kruglov]

So far so good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=37333cec3bd8

Comment 3

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8884115 [details]
Bug 1379025 - Update sync ping tests for the "firm up" changes

https://reviewboard.mozilla.org/r/155042/#review160310

Works for me!

::: mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java:143
(Diff revision 1)
> +
> +        // Test general shape of payload. Expecting {"syncs":[],"why":"schedule", "version": 1}.
>          // NB that even though we set an empty sync event store, it's not in the json string.
>          // That's because sync events are not yet instrumented.
>          ExtendedJSONObject payload = outgoingPing.getPayload().getObject("payload");
> -        assertEquals("{\"syncs\":[],\"why\":\"schedule\"}", payload.toJSONString());
> +        assertEquals(3, payload.keySet().size());

Again -- you can `.equals` two EJO's.  But I'm fine with this for now.

::: mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java:194
(Diff revision 1)
>  
> -    private void assertSync(ExtendedJSONObject sync, String uid, long took, String deviceID, int version, boolean restarted) throws JSONException {
> +    private void assertSync(ExtendedJSONObject sync, String uid, long took, String deviceID, boolean restarted) throws JSONException {
>          assertEquals(uid, sync.getString("uid"));
>          assertEquals(Long.valueOf(took), sync.getLong("took"));
>          assertEquals(deviceID, sync.getString("deviceID"));
> -        assertEquals(Integer.valueOf(version), sync.getIntegerSafely("version"));
> +        assertTrue(sync.getLong("when") != null);

Consider testing that `when` is larger than some fixed timestamp (say from a year ago) and also smaller than the _current_ timestamp.

Comment 4

[:Grisha Kruglov]

Comment on attachment 8884115 [details]
Bug 1379025 - Update sync ping tests for the "firm up" changes

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/407df61a6300
Update sync ping tests for the "firm up" changes r=nalexander

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/407df61a6300

Comment 7

[Intermittent Failures Robot]

4 failures in 656 pushes (0.006 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 4

Platform breakdown:
* android-4-0-armv7-api15: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1379025&startday=2017-07-03&endday=2017-07-09&tree=all

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift here too (from https://bugzilla.mozilla.org/show_bug.cgi?id=1374758#c22) Thanks!

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/109e1b9998b3