1185783 - Move promiseBrowserEvent (and related helpers) in robocop_head.js - Part II

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Mark Capella [:capella]]

I accidentally 'subverted' bug 1158925 with a partial patch that I should have just openned on it's own :-/

Let's go ahead with a new bug, with the intent of the original bug 1158925 ...

"Copy pasta all over the place in our JS tests! We should move some of these common helper functions to a shared place."

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testFindInPage.js#15
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testTrackingProtection.js#18
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testHistoryService.js#32

To fix this bug, you should audit our JS tests to find the places we do things like this, and make shared helper methods in here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/robocop_head.js

Comment 1

[Geoff Brown [:gbrown]]

In bug 1184186, many of the robocop tests that implement promiseBrowserEvent have been converted to mochitest-chrome tests and moved to mobile/android/tests/browser/chrome. The mobile chrome tests do not currently share any code across tests, but I imagine that a head.js could be introduced and the chrome tests re-factored.

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r?margaret

Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r?margaret

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1185783 - Part 2: Move promiseNotification to head.js. r?margaret

Bug 1185783 - Part 2: Move promiseNotification to head.js. r?margaret

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8669225 [details]
MozReview Request: Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r?margaret

Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r?margaret

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8669226 [details]
MozReview Request: Bug 1185783 - Part 2: Move promiseNotification to head.js. r?margaret

Bug 1185783 - Part 2: Move promiseNotification to head.js. r?margaret

Comment 6

[:Margaret Leibovic]

Comment on attachment 8669225 [details]
MozReview Request: Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r?margaret

https://reviewboard.mozilla.org/r/21147/#review19067

Nice, thanks for picking this up.

Comment 7

[:Margaret Leibovic]

Comment on attachment 8669226 [details]
MozReview Request: Bug 1185783 - Part 2: Move promiseNotification to head.js. r?margaret

https://reviewboard.mozilla.org/r/21149/#review19069

Comment 8

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/c68d71e149200d32b2d66eab23afe651ef0d2aca
Bug 1185783 - Part 1: Add head.js with promiseBrowserEvent. r=margaret

https://hg.mozilla.org/integration/fx-team/rev/6b7da36dcf08a7bf461ecc642a072a74a38da329
Bug 1185783 - Part 2: Move promiseNotification to head.js. r=margaret

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c68d71e14920
https://hg.mozilla.org/mozilla-central/rev/6b7da36dcf08