Closed Bug 1158925 Opened 7 years ago Closed 7 years ago
Browser Event (and related similar helper functions) in robocop _head .js
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
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.
I'll take this one, go ahead and assign to me. Thanks :nalexander!
Just setting needinfo...
Helpers can go here: http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/robocop_head.js#1232
Assignee: nobody → mking
Status: NEW → ASSIGNED
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?
(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.
(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:  function is(lhs, rhs, text)  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?  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 http://mxr.mozilla.org/mozilla-central/search?string=ok%28passed%2C+text%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
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?
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.
Partial patch for the |ok()| and |is()| methods only ... try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=acb2b578b7fd
Attachment #8635900 - Flags: review?(margaret.leibovic)
Comment on attachment 8635900 [details] [diff] [review] bug1158925.diff Review of attachment 8635900 [details] [diff] [review]: ----------------------------------------------------------------- Love it.
Attachment #8635900 - Flags: review?(margaret.leibovic) → review+
Whiteboard: [lang=js][good next bug] → [keep open]
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.
We can do that ! see: bug 1185783 ... and, pardon the interruption :-)
Assignee: mking → markcapella
Whiteboard: [keep open]
You need to log in before you can comment on or make changes to this bug.