855427 - Refactor tests which use getNotification() method

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Jonathan French (:jfrench)]

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.

Comment 1

[Jonathan French (:jfrench)]

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.

Comment 2

[Jonathan French (:jfrench)]

Attachment: updates to waitForNotification (default) rev1.0

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.

Comment 3

[Andreea Matei [:AndreeaMatei]]

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.

Comment 4

[Jonathan French (:jfrench)]

Yup, thanks Andreea, I just saw that in the other bug. I will wait for that to land.

Comment 5

[Jonathan French (:jfrench)]

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

Comment 6

[Jonathan French (:jfrench)]

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

Comment 7

[Jonathan French (:jfrench)]

Attachment: updates to waitForNotification (default) rev1.1

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

Comment 8

[Andreea Matei [:AndreeaMatei]]

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

Comment 9

[Jonathan French (:jfrench)]

Thanks Andreea, will add it in.

Comment 10

[Jonathan French (:jfrench)]

Attachment: updates to waitForNotification (default) rev1.2

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

Comment 11

[Andreea Matei [:AndreeaMatei]]

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!