Closed
Bug 855427
Opened 12 years ago
Closed 11 years ago
Refactor tests which use getNotification() method
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(firefox29 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox29 | --- | fixed |
People
(Reporter: jfrench, Assigned: jfrench)
References
()
Details
(Whiteboard: [mentor= andreea.matei][lang=js][good first bug])
Attachments
(1 file, 2 obsolete files)
5.11 KB,
patch
|
AndreeaMatei
:
review+
|
Details | Diff | Splinter Review |
Just entering a marker bug, begat from bug 758187, for the potential refactoring of tests which use getNotification() and the related method changes in that bug.
At present, we think changes may involve these tests
testPreferences/testPasswordSavedAndDeleted
testPasswordManager/testPasswordNotificationBar
restartTests/testPreferences_masterPassword/test1
I searched for this bug beforehand, so hopefully I'm not entering a duplicate bug. :) Apologies if so.
Updated•12 years ago
|
Severity: minor → normal
Whiteboard: [mentor= andreea.matei][lang=js][good first bug]
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•12 years ago
|
Assignee: nobody → tojonmz
Assignee | ||
Comment 1•12 years ago
|
||
I updated the tests in a local branch last night, they all appear to work fine with the new method provided in lib/toolbars.js in patchv6 provided in bug 758187.
I will hold off on uploading any patches until that bug is landed on default.
I also flipped the boolean intentionally to true in the tests (notification popup expected open) for a test run, just to confirm that the method then identifies a simulated failure condition.
Tested with Default 22.0a1 20130329030904. Tests pass as expected.
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
Patch "updates for waitForNotification (default) rev1.0" for Default branch. Three files modified.
The work represents updates to tests, in harmony with the latest patch(v6.1) for waitForNotification() used in bug 758187.
It's been quite awhile since I looked at this patch locally, please give it a careful look, I could well have misinterpreted the change.
It seemed timely to provide an initial patch for review, as it seems we are getting close to landing the change to toolbars.js for testGeolocation. Maybe it would make sense for this patch to be merged with that patch before landing, I'm not sure. Since this patch requires that one's toolbars.js.
Tested with Default Nightly 24.0a1 20130528030942 using Andreea's latest lib/toolbars.js from patch rev6.1. All tests pass as expected.
Attachment #755136 -
Flags: review?(andreea.matei)
Comment 3•12 years ago
|
||
Comment on attachment 755136 [details] [diff] [review]
updates to waitForNotification (default) rev1.0
Review of attachment 755136 [details] [diff] [review]:
-----------------------------------------------------------------
Sorry I didn't get to this earlier, but I was really waiting for a review on my patch. There was no point in checking this if those changes there weren't going to get through. And that is the case now.
Please wait for a landing there, so you have the clear steps on what needs to be changed and you don't work in vain.
Attachment #755136 -
Flags: review?(andreea.matei) → review-
Assignee | ||
Comment 4•12 years ago
|
||
Yup, thanks Andreea, I just saw that in the other bug. I will wait for that to land.
Assignee | ||
Comment 5•12 years ago
|
||
I see Andreea has a new patch up for review for bug 758187. I retested her latest patch 2013-08-01 + this patch for the refactoring. These three tests with both patches appear to be working and passing as expected. The syntax used for the watiForNotification method doesn't appear to have changed so far.
I also simulated a failure case with the one restart test and it reported the failure as expected.
Tested in:
Windows XP32 SP3
mozmill-env w/mozmill 2.0rc4 latest + patch rev1.0
latest Nightly 25.0a1 20130729030214 140267
Assignee | ||
Comment 6•11 years ago
|
||
The patch still appears valid for landing on default, run with the work landed last week in bug 758187.
Re-tested and passed in:
Windows XP32 SP3
mozmill-env w/mozmill 2.0rc6 latest + patch rev1.0
latest Nightly 27.0a1 20131001030204 149422
Assignee | ||
Comment 7•11 years ago
|
||
The mozmill-tests repo has since been restructured in bug 927937. This is a revised patch to represent the new firefox/ metrofirefox/ structure, and another merge for the latest state of the tests in default.
Re-tested and passed in:
Windows XP32 SP3
mozmill-env w/mozmill 2.1-dev latest + patch rev1.1
latest Nightly 28.0a1 20131128030201 157937
Attachment #755136 -
Attachment is obsolete: true
Attachment #8339988 -
Flags: review?(andreea.matei)
Comment 8•11 years ago
|
||
Comment on attachment 8339988 [details] [diff] [review]
updates to waitForNotification (default) rev1.1
Review of attachment 8339988 [details] [diff] [review]:
-----------------------------------------------------------------
There's one more instance here:
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/restartTests/testAddons_installMultipleExtensions/test1.js#l75
Attachment #8339988 -
Flags: review?(andreea.matei) → review-
Assignee | ||
Comment 9•11 years ago
|
||
Thanks Andreea, will add it in.
Assignee | ||
Comment 10•11 years ago
|
||
Additional test added as indicated. I also tried another failure-case (switching a boolean in the test) and the test harness does indicate a failure as expected. As per below:
ERROR | Test Failure | {
"exception": {
"message": "Modal dialog has been found and processed",
"lineNumber": 27,
"name": "AssertionError",
"fileName": "resource://mozmill/modules/errors.js"
}
Assuming that is our expected assert message.
Re-tested and passed in:
Windows XP32 SP3
mozmill-env w/mozmill 2.1-dev latest + patch rev1.2
latest Nightly 28.0a1 20131205030207 158806
Attachment #8339988 -
Attachment is obsolete: true
Attachment #8343258 -
Flags: review?(andreea.matei)
Comment 11•11 years ago
|
||
Comment on attachment 8343258 [details] [diff] [review]
updates to waitForNotification (default) rev1.2
Review of attachment 8343258 [details] [diff] [review]:
-----------------------------------------------------------------
Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/adb6928641c7 (default)
Thanks Jonathan!
Attachment #8343258 -
Flags: review?(andreea.matei) → review+
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox29:
--- → fixed
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•