964165 - [Test] Test window.alert in marionette python under new window management system

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::UI Tests, defect, P1)

Description

[(inactive after 6/18) Alive Kuo [:alive]]

See relavant bug for marionette js: bug 964157

Comment 1

[Zac C (:zac)]

For this our test case will be: 
1. open UITests app
2. go to the 'UI/Alert' page 
3. Tap 'alert' 
4. Assert modal dialog

Is that suitable Alive?

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Zac C (:zac) from comment #1)
> For this our test case will be: 
> 1. open UITests app
> 2. go to the 'UI/Alert' page 
> 3. Tap 'alert' 
> 4. Assert modal dialog
> 
> Is that suitable Alive?

Great :)

It'd be nice if we have some shared alert/confirm/prompt assertion and actions helper (click/cancel...) so all apps has alert/modal/dialog could reuse.

Comment 3

[Zac C (:zac)]

(In reply to Alive Kuo [:alive][NEEDINFO!][:艾莉芙] from comment #2)
> Great :)
> 
> It'd be nice if we have some shared alert/confirm/prompt assertion and
> actions helper (click/cancel...) so all apps has alert/modal/dialog could
> reuse.

We mostly do already for prompts like this!

Comment 4

[Robert Chira [:RobertC]]

Attachment: Pull request

Comment 5

[Robert Chira [:RobertC]]

The test from the pull request taps on the first alert button.
Should we add a test that taps on the second one, which opens the alert in the iframe?

Comment 6

[[:AndreiH]]

Comment on attachment 8365973 [details] [review]
Pull request

Comments that need to be addressed

Comment 7

[Bob Silverberg [:bsilverberg]]

In comment #1, zac says "4. Assert modal dialog", which to me suggests that we should be asserting that we are in fact seeing a modal dialog. Is there something we should be doing in the test to assert this? Currently it's just checking the text and then tapping the ok button. This may be enough, but I just wanted to check against the requirements.

Comment 8

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8365973 [details] [review]
Pull request

See the PR for a question and a comment.

Comment 9

[Zac C (:zac)]

Sure, the test should assert that the dialog is visible. Reviewers and test coder can decide what's valid.

Comment 10

[Zac C (:zac)]

Comment on attachment 8365973 [details] [review]
Pull request

very close, just a couple of small nits!

Comment 11

[Florin Strugariu [:Bebe]]

Comment on attachment 8365973 [details] [review]
Pull request

this failed on travis

Comment 12

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8365973 [details] [review]
Pull request

This is failing on desktop. See comments in the PR.

Comment 13

[Robert Chira [:RobertC]]

Skipped the test on desktop because of Bug 979220

Comment 14

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8365973 [details] [review]
Pull request

I believe that the new test needs to be skipped on tbpl as well.

Comment 15

[Florin Strugariu [:Bebe]]

Comment on attachment 8365973 [details] [review]
Pull request

With Bob's updates it looks OK

Comment 16

[Robert Chira [:RobertC]]

Comment on attachment 8365973 [details] [review]
Pull request

Disabled the test on tbpl.

Comment 17

[Florin Strugariu [:Bebe]]

Zac is this test in our Smoke test plan?

Comment 18

[Zac C (:zac)]

(In reply to Florin Strugariu [:Bebe] from comment #17)
> Zac is this test in our Smoke test plan?

No I'm not and not really sure this is a high value test case.

It seems like the kind of thing that the marionettejs tests will have good coverage for.

Comment 19

[Zac C (:zac)]

This is a good candidate for marionette-js tests on CI

There's no real device-specific functionality here.