1132936 - Create unit tests for IdentityPopup class in toolbars.py

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, defect)

Description

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

We do not have any unit tests for the IdentityPopup class located at:

https://github.com/mozilla/firefox-ui-tests/blob/master/firefox_puppeteer/ui/toolbars.py

https://github.com/rbillings/firefox-ui-tests/blob/gobutton/firefox_puppeteer/tests/test_toolbars.py

We need a new set of tests under their own class, which cover:
* Checks that the elements are present and have the correct type (see the location bar tests as example)
* opening and closing the identity popup

Comment 1

[Ankit Goyal]

Attachment: Bug 1132936 - Create unit tests for IdentityPopup class in toolbars.py. r=whimboo

Comment 2

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

Hi Ankit, thanks for your interest to work on this bug! I was away last week and had a couple of other reviews to do first. I will have a look at your request in a bit.

Comment 3

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

Comment on attachment 8565375 [details] [review]
Bug 1132936 - Create unit tests for IdentityPopup class in toolbars.py. r=whimboo

This PR looks good. But there are some changes necessary for a full review. I commented everything on the PR itself.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Hi Ankit,

I see there is a pull request for this:
https://github.com/mozilla/firefox-ui-tests/pull/86

It has some additional work to get r+, can you address this?  If not, let us know and we will find someone else to address the review comments!

Comment 5

[Ankit Goyal]

Joel, I am in the middle of something. So I can't contribute for a while.

Comment 6

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

Thank you Ankit for letting us know. I will put this bug back into the general bucket, so we can hopefully find someone else to finish it off. Thanks for the work so far.

Comment 7

[Barbara Miller (:galgeek)]

Attachment: github_pull_request.txt

It took a few tries to get to something that succeeds on all platforms, but it looks like this does, using hide.Popup() to force close the identity popup, until its own close method lands.

I also have a draft of a unit test for the close method that I could add here or to the close method bug, depending on which lands first.

Comment 8

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

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

Looks good. Lets continue once the close() method has been landed.

Comment 9

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

The close() method has been landed. Barbara, could you get this PR for the unit tests updated? Thanks.

Comment 10

[Barbara Miller (:galgeek)]

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

Thanks for your feedback, Henrik!

I've updated the PR, including adding a test_force_close.

With the expected.element_present check in the close method, the try block in the tearDown needs to except NoSuchElementException.

Comment 11

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

https://github.com/mozilla/firefox-ui-tests/commit/9505b0e96c5cae85814862d959aa1e3631b31ff0