Closed Bug 1132710 Opened 9 years ago Closed 9 years ago

Convert Mozmill test 'remote/testSecurity/testSubmitUnencryptedInfoWarning.js' to Marionette

Categories

(Testing :: Firefox UI Tests, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla39

People

(Reporter: galgeek, Assigned: galgeek)

References

Details

Attachments

(1 file)

52 bytes, text/x-github-pull-request
whimboo
: review+
whimboo
: feedback+
chmanchester
: feedback+
whimboo
: feedback+
whimboo
: feedback+
whimboo
: feedback+
Details | Review
This bug will cover the necessary work to convert the following test into Marionette.

http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/testSecurity/testSubmitUnencryptedInfoWarning.js
Blocks: 1132629
Priority: -- → P1
This bug is blocked by the modal dialog handling which needs to be implemented in bug 887274.
Depends on: 887274
Whiteboard: [blocked]
Whiteboard: [blocked]
Assignee: nobody → galgeek
QA Contact: hskupin
I wonder if this is the best way to handle this dialog.

I've omitted here the mozmill version's subroutine for handling the dialog to keep things a bit simpler.
Attachment #8571567 - Flags: feedback?(hskupin)
Attachment #8571567 - Flags: feedback?(cmanchester)
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

This looks reasonable to me -- does the dialog handling work as expected?
Attachment #8571567 - Flags: feedback?(cmanchester) → feedback+
The dialog handling looks right to me.

I copied the code more or less from test_modal_dialogs.py in the marionette client's unit tests.
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Looks good. It would be great if it would work all the way with Alert(). But lets see later what other modals request from us to do. But this patch as is looks good.
Attachment #8571567 - Flags: feedback?(hskupin) → feedback+
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Thanks for your feedback, Chris and Henrik!

I've updated the PR, and made a couple of additional comments on github.

This is probably about ready for review, but I know it may need a rebase once pageinfo lands.
Attachment #8571567 - Flags: feedback?(hskupin)
Attachment #8571567 - Flags: feedback?(hskupin) → feedback+
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Thanks for your feedback!

I've updated the PR.
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Don't miss the conversation in line notes on this PR!
Attachment #8571567 - Flags: feedback?(hskupin)
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

I'm missing feedback from Chris on this PR. Please talk with him if he hasn't seen my comments.
Attachment #8571567 - Flags: feedback?(hskupin) → feedback+
Attachment #8571567 - Flags: feedback?(hskupin)
Attachment #8571567 - Flags: feedback?(cmanchester)
I haven't had a chance to chat with Chris, but I've requested his feedback here, and added the relevant github line notes to a comment to make them easier to find.
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Please request review once Chris answered the questions and you updated the code if necessary.
Attachment #8571567 - Flags: feedback?(hskupin)
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Thanks for your comments, Chris!

I've revised the PR using the code you suggested, and added a self.assertRaises to test specifically that there's no alert present after the warning is accepted.
Attachment #8571567 - Flags: feedback?(hskupin)
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Better but we would still fail in cleaning up correctly. Please see my comments on the PR for fixing that and other stuff.
Attachment #8571567 - Flags: feedback?(hskupin) → feedback+
Comment on attachment 8571567 [details] [review]
github_pull_request.txt

Thanks for your comments! I've updated the PR.

Maybe this is ready for review?
Attachment #8571567 - Flags: review?(hskupin)
Attachment #8571567 - Flags: review?(hskupin) → review+
https://github.com/mozilla/firefox-ui-tests/commit/36ed081c78c95412e9f9f408dd7eed0c9a6ba7d8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Product: Mozilla QA → Testing
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: