Closed Bug 1461652 Opened 2 years ago Closed 2 years ago

Reenable big ping test on Android

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: janerik, Assigned: janerik)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug 1331445 disabled the big ping test on Android, as it added about 50 seconds of runtime on an android/debug build.

We should find a way to re-enable the test in an Android-friendly manner.
Depends on: 1331445
Maybe mock the gzip stream using Policy to return a known large/small string so we can avoid the whole "use a bunch of random data so it's hard to compress" part?
Assignee: nobody → jrediger
Priority: -- → P1
A first rough patch to mock the gzip compression as :chutten mentioned.
With a (hopefully) quickfix in bug 1331445, this patch would reenable the big ping test on Android by mocking the gzip compression using a not-so-random long string.
I am currently not able to push to try, so this will be hold off until I can do so anyway.
Attachment #8975837 - Flags: review?(chutten)
Comment on attachment 8975837 [details] [diff] [review]
Mock gzip compression in tests to simulate large payloads. r?chutten

Review of attachment 8975837 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: toolkit/components/telemetry/tests/unit/head.js
@@ +319,5 @@
> +  while (string.length < length) {
> +    string += data
> +  }
> +
> +  return string.substring(0, length);

not `new Array(length + 1).join("a")`? That's a trick I've seen elsewhere for generating a string of a given length.
Attachment #8975837 - Flags: review?(chutten) → review+
Thanks for pointing out the one-liner to create a string of a certain length. Added that.

I decided to cancel the patch in bug 1331445 and instead immediately apply this change.
No need to land something that's going away in the next commit anyway.
Attachment #8976063 - Flags: review?(chutten)
Attachment #8975837 - Attachment is obsolete: true
No longer depends on: 1331445
See Also: → 1331445
Comment on attachment 8976063 [details] [diff] [review]
Mock gzip compression in tests to simulate large payloads. r?chutten

Review of attachment 8976063 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8976063 - Flags: review?(chutten) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5edae29aca32
Mock gzip compression in tests to simulate large payloads. r=chutten
Keywords: checkin-needed
MozReview-Commit-ID: Gct9oVfPVou
Attachment #8976448 - Flags: review?(chutten)
Attachment #8976063 - Attachment is obsolete: true
Flags: needinfo?(jrediger)
Attachment #8976448 - Flags: review?(chutten) → review+
Fixed the linting issue in the updated patch.
Keywords: checkin-needed
Hello,

this patch failed when applied:

applying Bug-1461652---Mock-gzip-compression-in-tests-to-si.patch
patching file toolkit/components/telemetry/TelemetrySend.jsm
Hunk #1 FAILED at 103
Hunk #2 FAILED at 1208
2 out of 2 hunks FAILED -- saving rejects to file toolkit/components/telemetry/TelemetrySend.jsm.rej
patching file toolkit/components/telemetry/tests/unit/head.js
Hunk #1 FAILED at 269
Hunk #2 FAILED at 295
2 out of 2 hunks FAILED -- saving rejects to file toolkit/components/telemetry/tests/unit/head.js.rej
patching file toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
Hunk #1 FAILED at 328
Hunk #2 FAILED at 370
2 out of 2 hunks FAILED -- saving rejects to file toolkit/components/telemetry/tests/unit/test_TelemetrySend.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh Bug-1461652---Mock-gzip-compression-in-tests-to-si.patch

can you please take a look?
Flags: needinfo?(jrediger)
Flags: needinfo?(jrediger)
Keywords: checkin-needed
Patch is already in inbound and pushed by :Dexter. We forgot to remove ckeckin-needed.
https://hg.mozilla.org/mozilla-central/rev/87690ba7227e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.