1130460 - Convert Mozmill js test testGoButton.js to a Firefox UI Test

Status: RESOLVED WONTFIX

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

Description

[Rebecca Billings [:rbillings]]

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

This test:
1) Clicks the location bar, types a URL, and clicks the GO button
2) It checks that the URL is opened in the current tab and that the GO button is hidden
3) It checks for the presence of the 'Organisation' element
4) It checks that the URL matches the expected domain name

Comment 1

[Rebecca Billings [:rbillings]]

Attachment: https://github.com/mozilla/firefox-ui-tests/pull/74

I changed the steps to remove "Organization", added the mozilla.org url, verified that the url displays after clicking the go button and that the go button no longer displays after being clicked.

Comment 2

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

Rebecca, is that patch ready for review? If yes, please update the attachment and ask for it. Thanks.

Comment 3

[Rebecca Billings [:rbillings]]

I'm unclear what needs to be updated in the attachment, but it is ready for review. Sorry I was unclear on the need to specify that it is ready to review.

Comment 4

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

Ok, then for the future please set the review flag. I will have a look at it tomorrow then.

Comment 5

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

(In reply to Rebecca Billings [:rbillings] from comment #0)
> 1) Clicks the location bar, types a URL, and clicks the GO button
> 2) It checks that the URL is opened in the current tab and that the GO
> button is hidden
> 3) It checks for the presence of the 'Organisation' element
> 4) It checks that the URL matches the expected domain name

All of those given steps have not been done yet in this test, so the coverage decreased a lot with the current state of the test. Please see my comments on the PR.

I don't think that you have to do check 3) when we have 4).

Comment 6

[Rebecca Billings [:rbillings]]

Based on your PR comments should I create both a very simple unit test and also the more extensive functional test? The unit test is blocked by bug 865232.

Comment 7

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

We might need our isDisplayed method here:
http://hg.mozilla.org/qa/mozmill-tests/file/99ae80464f46/lib/utils.js#l385

But you don't have to implement it right now. Just check which of those conditions are met by the go button, and use it directly. Just mark the line as "TODO: use isDisplayed" or such.

Yes, please do a unit test for the element type check of the go button. The functional test has to be equivalent to the Mozmill test then.

Comment 8

[Rebecca Billings [:rbillings]]

The PR https://github.com/mozilla/firefox-ui-tests/pull/74 is ready for review. Changes: added unit test, added more functional test steps and notes, added go button to locationbar class.

Comment 9

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

When an updated PR is ready for another review please set the review flag on the link attachment so I get the correct notification. Thanks.

Comment 10

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

The PR still shows 44 commits on this branch, which still contain all of our latest changes. Please fix that so it only contains commits you did personally. Not sure if you followed my advice I gave yesterday.

Comment 11

[Rebecca Billings [:rbillings]]

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

PR has been updated to fix issues and remove extra files

Comment 12

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

Rebecca, what you want is to ask for review and not to set r+ yourself. Maybe just wrongly picked the menu item. I will have a look later. Thanks for the update.

Comment 13

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

I made a couple of comments on the PR. It's good to see that you were able to fix the problems you had before, but as seen we are still in a feedback cycle due to all the remaining work to do. Please also note the Travis CI failure with exceeding line lengths. Make sure to wait with the review request until Travis reports green, or ask if you have trouble interpreting the results.

Comment 14

[Rebecca Billings [:rbillings]]

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

I have updated the PR with all of the fixes for each comment. The travis build failed due to blank lines containing whitespaces- there are no longer any lines which are too long.

Comment 15

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

Rebecca, please do not forget to set a proper person as reviewer. Otherwise your request can get lost.

Comment 16

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

Comment on attachment 8560729 [details]
https://github.com/mozilla/firefox-ui-tests/pull/74

I gave all my comments on Github, and it looks like there is still a bit of work to do.

Comment 17

[Rebecca Billings [:rbillings]]

I don't have time to complete this, so I am removing the "assigned" tag.

Comment 18

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

Rebecca, thank you for getting started on this test. Lets see that we can find someone else to continue with it. I will mark the PR as closed, but it will be easy for someone else to pick up the work when necessary.

Comment 19

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

Marco, is that something we would still need for Firefox UI tests? I believe that is already covered by a mochitest.

Comment 20

[Marco Bonardo [:mak]]

browser_locationBarCommand.js seems to cover the go button, I'm not sure it physically checks that the button is visible/hidden... 
I don't know what is the 'Organisation' element, the identity panel? I think it has its own tests already.
We could probably file a bug to improve browser_locationBarCommand.js to check for elements visibility, and wontfix this.

Comment 21

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

Yes, the `organization` is part of the identity panel, so we should keep it away from this test. I also had a look at the referred mochitest and it synthesizes a mouse click onto the Go button. As such it is an implicit visibility check, and we won't need something else.