As filed as https://github.com/mozilla/geckodriver/issues/1375 the pause action primitive currently doesn't support `undefined` as value for duration. Per the spec it should be allowed: https://w3c.github.io/webdriver/#dfn-process-a-pause-action

Differential Revision: https://phabricator.services.mozilla.com/D6133

Comment on attachment 9009887 [details] [diff] [review] [geckodriver] "Pause" action primitive has to support undefined duration Review of attachment 9009887 [details] [diff] [review]: ----------------------------------------------------------------- r+wc If you could update the commit message to not refer to JavaScript concepts, that would be great. The "duration" field is _optional_, not "supported undefined".

Comment on attachment 9009890 [details] [diff] [review] [wdspec] Add tests for null input source Review of attachment 9009890 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/perform_actions/validity.py @@ +56,5 @@ > + actions = [{ > + "type": action_type, > + "id": "foobar", > + "actions": [{ > + "type": "pause", Can we test for a pause action with the duration set to null as well, please?

Comment on attachment 9009889 [details] [diff] [review] [wdspec] Create tests for "Release Actions" command Review of attachment 9009889 [details] [diff] [review]: ----------------------------------------------------------------- Is this change relevant to fixing the optional pause duration? Just re-flag me if you think it is.

Comment on attachment 9009888 [details] [diff] [review] [wdspec] Relocate "Perform Actions" tests, and separate tests by input source Review of attachment 9009888 [details] [diff] [review]: ----------------------------------------------------------------- I have the same question about this patch.

Comment on attachment 9009890 [details] [diff] [review] [wdspec] Add tests for null input source Review of attachment 9009890 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/perform_actions/validity.py @@ +56,5 @@ > + actions = [{ > + "type": action_type, > + "id": "foobar", > + "actions": [{ > + "type": "pause", This is already done in the test `test_pause_invalid_types` you wrote not that long ago.

(In reply to Andreas Tolfsen ❲:ato❳ from comment #10) > Is this change relevant to fixing the optional pause duration? It's not, and I can move this out to a separate patch if that is what you want to hear.

Comment on attachment 9009890 [details] [diff] [review] [wdspec] Add tests for null input source Review of attachment 9009890 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/web-platform/tests/webdriver/tests/perform_actions/validity.py @@ +56,5 @@ > + actions = [{ > + "type": action_type, > + "id": "foobar", > + "actions": [{ > + "type": "pause", Perfect. What I wanted to avoid here is someone implementing the spec thinking {type: "pause"} and {type: "pause", duration: null} are equivalent.

(In reply to Henrik Skupin (:whimboo) from comment #13) > (In reply to Andreas Tolfsen ❲:ato❳ from comment #10) > > > Is this change relevant to fixing the optional pause duration? > > It's not, and I can move this out to a separate patch if that is > what you want to hear. Sorry, I didn’t mean any offence. I was just trying to ascertain if the patches were somehow related to the optional pause duration change, or if they were preparation work for the fourth patch to add tests for the missing duration parameter. The patches themselves look good, but if they are indeed unrelated then splitting that work off to a separate bug makes sense.

No problem at all. I was somehow in such a refactoring mode that I wanted to make it look nice together with this patch. I will move the commits to bug 1492469.

Comment on attachment 9010280 [details] [diff] [review] [wdspec] Add tests for null input source and missing duration for pause primitive Rebased the patch after removing the refactoring patches. Taking over r+ from ato.

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/57bb41e8a02a [geckodriver] "Pause" action primitive should allow duration to be optional. r=ato https://hg.mozilla.org/integration/mozilla-inbound/rev/35e63e22c367 [wdspec] Add tests for null input source and missing duration for pause primitive. r=ato

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

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

https://hg.mozilla.org/mozilla-central/rev/57bb41e8a02a https://hg.mozilla.org/mozilla-central/rev/35e63e22c367

Upstream PR merged

No need to uplift given that this didn't affect our tests in CI, and a new geckodriver release is already out.