Closed Bug 1158925 Opened 7 years ago Closed 7 years ago

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


(Firefox for Android Graveyard :: Testing, defect)

35 Branch
Not set


(firefox42 fixed)

Firefox 42
Tracking Status
firefox42 --- fixed


(Reporter: Margaret, Assigned: capella, Mentored)



(1 file)

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

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:
The links are out of date -- try

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.
Flags: needinfo?(mking)
I'll take this one, go ahead and assign to me. Thanks :nalexander!
Flags: needinfo?(mking)
Just setting needinfo...
Flags: needinfo?(nalexander)
Helpers can go here:
Assignee: nobody → mking
Flags: needinfo?(nalexander)
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?
Flags: needinfo?(nalexander)
(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, and NI me.
Flags: needinfo?(nalexander)
(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?

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?
Flags: needinfo?(mking)
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.
Flags: needinfo?(mking)
Attached patch bug1158925.diffSplinter Review
Partial patch for the |ok()| and |is()| methods only ...
try push:
Attachment #8635900 - Flags: review?(margaret.leibovic)
Comment on attachment 8635900 [details] [diff] [review]

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]
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.