1158925 - Move promiseBrowserEvent (and related similar helper functions) in robocop_head.js

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[:Margaret Leibovic]

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

[Nick Alexander :nalexander [he/him]]

The links are out of date -- try

http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/testFindInPage.js#15

etc, but this would be a nice clean-up.

Matt, are you interested?  This is smaller than the Synced Tabs ticket, so just to tide you over until we find a more interesting one.

Comment 2

[u535116]

I'll take this one, go ahead and assign to me. Thanks :nalexander!

Comment 3

[u535116]

Just setting needinfo...

Comment 4

[Nick Alexander :nalexander [he/him]]

Helpers can go here:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/robocop_head.js#1232

Comment 5

[u535116]

I'm having a hard time running all the robocop tests locally, as it takes a long time to run on my device. Any tips on speeding it up?

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Matt King from comment #5)
> I'm having a hard time running all the robocop tests locally, as it takes a
> long time to run on my device. Any tips on speeding it up?

Not really.  I'd run a representative test or two locally -- use |mach robocop SPECIFIC_TEST| -- and then push a try build for the lot.  Do you have try access?  I'll vouch for you!  File a ticket, like https://bugzilla.mozilla.org/show_bug.cgi?id=1039210, and NI me.

Comment 7

[Mark Capella [:capella]]

(drive-by)   :-)

In bug 1157637 comment #7 -ish margaret and I discussed setting up common functions for these copy/pastedsetting up common functions for copy/pasta functions, possibly as part of this bug:
   [0] function is(lhs, rhs, text)
   [1] ok(passed, text)

I'm looking at writing a new test soon that may use these again, so I'm wondering if you plan to provide patches for these?

[0] http://mxr.mozilla.org/mozilla-central/search?string=function+is%28lhs%2C+rhs%2C+text%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
[1]http://mxr.mozilla.org/mozilla-central/search?string=ok%28passed%2C+text%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 8

[Mark Capella [:capella]]

Hey mat, not sure what stage you have this work in, but I wonder, do you mind if I attach a partial patch here that completes the above for the |ok()| and |is()| methods?

Comment 9

[u535116]

Sorry about that, I have made some changes but have still been unable to test reliably...please do submit the patch. I will try to work on getting testing going this week.

Comment 10

[Mark Capella [:capella]]

Attachment: bug1158925.diff

Partial patch for the |ok()| and |is()| methods only ...
try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=acb2b578b7fd

Comment 11

[:Margaret Leibovic]

Comment on attachment 8635900 [details] [diff] [review]
bug1158925.diff

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

Love it.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0a74192ff490

Comment 13

[:Margaret Leibovic]

I am not a fan of keeping bugs open. I think we should close this and file a separate bug for the remaining work. It will make it much easier to track what's going on if any of these patches cause problems down the road.

Comment 14

[Mark Capella [:capella]]

We can do that ! see: bug 1185783 ... and, pardon the interruption :-)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0a74192ff490