1129159 - Convert Mozmill testEscapeAutocomplete to Firefox UI Test

Status: RESOLVED FIXED

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

Description

[Barbara Miller (:galgeek)]

This is a replacement for http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/testAwesomeBar/testEscapeAutocomplete.js

The Mozmill test does this:

1. Clear complete history so there's no interference from previous entries.
2. Open some local pages to set up the test environment.
3. Focus the locationbar and delete any locationbar content.
4. Type a test string in the locationbar.
5. Check that the test string matches locationbar value.
6. Check that the autocomplete list has opened.
7. Press the escape key.
8. Check that the test string matches locationbar value.
9. Check that the autocomplete list has closed.
10. Press the escape key again.
11. Check that the locationbar returns to the current page url.

Comment 1

[Barbara Miller (:galgeek)]

Attachment: github_pull_request.txt

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

It's going the right way. Left some comments on the PR.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I will add the places library as dependency here. Given that I have that one ready and it will land in a bit via bug 1121731, it would be good when you can your test updated to make use of the places methods similar to the mozmill test.

Comment 4

[Barbara Miller (:galgeek)]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

I've updated the github pr, referencing the places library appropriately, I hope!

In the meantime, I haven't tested this test.

Comment 5

[Barbara Miller (:galgeek)]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

I've tested, updated again, and tested anew, successfully.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

The PR looks pretty good. I added a couple of comments, but as it looks like its close for landing. Please re-request review once the update is done. Thanks Barbara.

Comment 7

[Barbara Miller (:galgeek)]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

I've updated the github PR per your feedback there.

Thank you!

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

Logic-wise this test looks good. Lets fix the remaining issues as pointed out.

Comment 9

[Barbara Miller (:galgeek)]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

Thanks for your feedback!

I've updated the github PR.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

Looks great! Just a minor nit to do. Let me know when you did the update and I will get it merged. Thanks

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

I missed one thing here, which is the missing manifest entry. Right now the test would never be run.

Comment 12

[Barbara Miller (:galgeek)]

Comment on attachment 8559397 [details] [review]
github_pull_request.txt

I've updated the github PR, incorporating a workaround for the Marionette bug leaving us in 'content' context in tearDown, and causing an error when run under xvfb.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

https://github.com/mozilla/firefox-ui-tests/commit/77e6205e2f8ee0c42ae241b16a141561c099389c