1468185 - Add support for allowed unhandledPromptBehavior options

Status: RESOLVED FIXED

(Testing :: geckodriver, enhancement, P1)

Description

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

To get bug 1264259 implemented geckodriver needs to support all the allowed values for unhandledPromptBehavior:

https://w3c.github.io/webdriver/#dfn-known-prompt-handling-approaches-table

I want to get this out with the 0.21.0 release to unblock myself from working on bug 1264259.

Comment 1

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

Attachment: Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

geckodriver currently only supports "accept" and "dismiss".
But to be compliant with the WebDriver specification all
allowed options have to be supported.

Review commit: https://reviewboard.mozilla.org/r/250646/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250646/

Comment 2

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

What I find strange is that the wdspec test `test_handle_prompt_dismiss_and_notify` is passing now, even with the appropriate end-point in Marionette not being handled. It means that the test is actually wrong, and doesn't check that an error is returned. I will have to update the patch.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0773ae1691c8514291b589b348a5b27a8b1b9870&filter-resultStatus=success&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable

Comment 3

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

Ok, so the before-mentioned wdspec tests were inappropriately marked as expected fail. The underlying problem here was that the various other options as added by this bug throw an `InvalidArgumentException` during session start:

> [task 2018-06-11T19:13:36.415Z] 19:13:36     INFO - STDOUT: E           InvalidArgumentException: invalid argument (400): Invalid unhandledPromptBehavior value: dismiss and notify

So allowing the validation of geckodriver to pass, we now correctly forward the values to Marionette, which for now simply ignores them.

Comment 4

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

Comment on attachment 8984845 [details]
Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250646/diff/1-2/

Comment 5

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

Also with the additional test executed now, the default timeout is not enough and needs to be extended to long.

Comment 6

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

Comment on attachment 8984845 [details]
Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250646/diff/2-3/

Comment 7

[Code Review Bot [:reviewbot]]

Comment on attachment 8984845 [details]
Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

https://reviewboard.mozilla.org/r/250646/#review256950


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/web-platform/tests/webdriver/tests/execute_script/user_prompts.py:1
(Diff revision 3)
> + # META: timeout=long

Error: Metadata comments should start the line [wpt: INDENTED-METADATA]

Comment 8

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

Comment on attachment 8984845 [details]
Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250646/diff/3-4/

Comment 9

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8984845 [details]
Bug 1468185 - [geckodriver] Add support for all unhandledPromptBehavior options.

https://reviewboard.mozilla.org/r/250646/#review256992

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cdcd92a013f
[geckodriver] Add support for all unhandledPromptBehavior options. r=ato

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11481 for changes under testing/web-platform/tests

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/11481
* continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/391457038?utm_source=github_status&utm_medium=notification)

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0cdcd92a013f