Rename "waitFor" class to "waitForEvents"

VERIFIED FIXED

Status

Testing Graveyard
Mozmill
VERIFIED FIXED
8 years ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

Details

(Whiteboard: [mozmill-doc-needed][mozmill-1.2.2])

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
Created attachment 402072 [details] [diff] [review]
Patch v1

Mikeal has added the waitFor class because he used it in bug 489474 to wait for events to occur. The name itself doesn't tell anything about this function and should be renamed to better reflect the feature behind it.

This patch renames the class and makes it accessible via waitForEvents.
Attachment #402072 - Flags: review?(ctalbert)
(Assignee)

Updated

8 years ago
Blocks: 489474
(Assignee)

Updated

8 years ago
Whiteboard: [mozmill-doc-needed][mozmill-1.2.2?]
(Assignee)

Updated

8 years ago
Duplicate of this bug: 503487

Comment 2

8 years ago
I don't see you changing any of the consumers of this API, so this patch looks incomplete.  It looks like you are landing the tests in bug 489474 today...are you changing this API in those tests before landing?
(Assignee)

Comment 3

8 years ago
No, the video test will not land today. I have some problems with current regressions and have to do some updates. So I will update those later. AFAIK those are the only tests which make use of the waitFor call. Do you know others?
(Assignee)

Comment 4

8 years ago
Clint, ping?

Updated

8 years ago
Attachment #402072 - Flags: review?(ctalbert) → review+

Comment 5

8 years ago
Be sure to change the tests when they land. :)
(Assignee)

Comment 6

8 years ago
Landed as http://github.com/mikeal/mozmill/commit/a0f6b75af414071969c45f46b86fcc5221a56504
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Whiteboard: [mozmill-doc-needed][mozmill-1.2.2?] → [mozmill-doc-needed][mozmill-1.2.2]
(Assignee)

Comment 7

8 years ago
With the landing of this patch we have a regression which makes this object unavailable.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

8 years ago
Created attachment 404460 [details] [diff] [review]
Follow-up

Changes in this patch which we definitely need for the next release:

* Changed usage to waitForEvents.init() and waitForEvents.wait()
* We should use an instance of waitForEvents as a member of MozMillController as we do for all the other functions. That's more consistent
* Throw a real error instead of a string when timeout happens
Attachment #404460 - Flags: review?(ctalbert)
(Assignee)

Updated

8 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 9

8 years ago
Comment on attachment 404460 [details] [diff] [review]
Follow-up

Send a pull request to Mikeal with http://github.com/whimboo/mozmill/commit/2c9cbfeecd76659156a7836b50b7e68b8e7570fb
Attachment #404460 - Flags: review?(ctalbert)
merged to mikeal/master
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

8 years ago
Verified fixed based on changeset:
http://github.com/mikeal/mozmill/commit/2c9cbfeecd76659156a7836b50b7e68b8e7570fb
Status: RESOLVED → VERIFIED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.