We have a couple of tests for Marionette which test closing a window (tab). We should port those to wdspec.

Please note that we cannot delete the existent Marionette tests yet, because it would mean that we would loose coverage for Windows, MacOS, and Android.

Comment on attachment 8969577 [details] Bug 1455282 - [wdspec] Add tests for Close Window command. https://reviewboard.mozilla.org/r/238336/#review244140 ::: testing/web-platform/meta/webdriver/tests/close_window/prompts.py.ini:1 (Diff revision 2) > +[prompts.py] I should rename this file to `user_prompts.py` to be in sync with other commands.

Comment on attachment 8969576 [details] Bug 1455282 - [wdclient] End session if no more windows are open. https://reviewboard.mozilla.org/r/238334/#review244440

Comment on attachment 8969577 [details] Bug 1455282 - [wdspec] Add tests for Close Window command. https://reviewboard.mozilla.org/r/238336/#review244442 ::: testing/web-platform/meta/webdriver/tests/close_window/user_prompts.py.ini:1 (Diff revision 3) > +[user_prompts.py] Doesn’t match name of the file. ::: testing/web-platform/tests/webdriver/tests/close_window/close.py:26 (Diff revision 3) > + value = assert_success(response) > + assert new_handle not in value > + assert session.handles == handles Nothing wrong with the way you have written this, but since the command returns the window handles, you could shorten this: > assert_success(response, original_handles) ::: testing/web-platform/tests/webdriver/tests/close_window/close.py:35 (Diff revision 3) > + value = assert_success(response) > + assert value == [] assert_success(response, []) ::: testing/web-platform/tests/webdriver/tests/close_window/close.py:38 (Diff revision 3) > + # With no more open top-level browsing contexts, the session is closed. > + session.session_id = None Does the client not do this for you? ::: testing/web-platform/tests/webdriver/tests/close_window/user_prompts.py:23 (Diff revision 3) > + """ > + 3. Handle any user prompts and return its value if it is an error. > + > + [...] > + > + In order to handle any user prompts a remote end must take the > + following steps: > + > + [...] > + > + 2. Perform the following substeps based on the current session's > + user prompt handler: > + > + [...] > + > + - accept state > + Accept the current user prompt. > + > + """ Can you dispose of these quotes when you make this change, please? ::: testing/web-platform/tests/webdriver/tests/support/fixtures.py:79 (Diff revision 3) > session.close() > > session.window_handle = current_window > > > +@ignore_exceptions This may warrant a separate commit.

Comment on attachment 8969577 [details] Bug 1455282 - [wdspec] Add tests for Close Window command. https://reviewboard.mozilla.org/r/238336/#review244442 > Doesn’t match name of the file. Maybe you looked at a former version of the patch? It has already been fixed. > Nothing wrong with the way you have written this, but since the > command returns the window handles, you could shorten this: > > > assert_success(response, original_handles) Oh, wow. That's nice! Thanks for pointing this out. I will add this but leave the other two asserts which are necessary to ensure that we really have the correct state, and `close` didn't just return the expected state without actually closing the window. > Does the client not do this for you? The operation here is not `session.close()` but our custom POST request. So the wdclient doesn't detect that, and we have to ensure to reset the session id instead. > Can you dispose of these quotes when you make this change, please? Sure, those are annoying and migth not reflect reality anymore.

Comment on attachment 8970362 [details] Bug 1455282 - [wdspec] Ignore exceptions when switching to top-level browsing context. https://reviewboard.mozilla.org/r/239148/#review244890 I thought I had reviewed this before…

(In reply to Andreas Tolfsen ‹:ato› from comment #13) > I thought I had reviewed this before… Ah, it was moved to a separate commit as requested. So I’m not going entirely insane. (-:

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/703161d003be [wdclient] End session if no more windows are open. r=ato https://hg.mozilla.org/integration/autoland/rev/843cb0cb835e [wdspec] Ignore exceptions when switching to top-level browsing context. r=ato https://hg.mozilla.org/integration/autoland/rev/295fd2df534f [wdspec] Add tests for Close Window command. r=ato

Backed out 3 changesets (bug 1455282) for linting failure in /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json:0 on a CLOSED TREE Problematic push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=295fd2df534f6a5f59f9d620be955a3e70ade74b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=175269249 Log: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c4027f932a5276de4d704103e0d0f0361b94d61a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c4027f932a5276de4d704103e0d0f0361b94d61a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified. Log: https://treeherder.mozilla.org/logviewer.html#?job_id=175269249&repo=autoland&lineNumber=268 [task 2018-04-24T06:40:22.159Z] No specific files specified, running the full wpt lint (this is slow) [task 2018-04-24T06:40:34.785Z] 0:12.63 INFO Diffing old and new manifests /builds/worker/checkouts/gecko/testing/web-platform/mozilla/meta/MANIFEST.json [task 2018-04-24T06:40:34.792Z] 0:12.63 INFO Diffing old and new manifests /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json [task 2018-04-24T06:40:36.936Z] 0:14.78 /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json 0 error webdriver/tests/fullscreen_window/user_prompts.py in manifest but removed from source. (wpt-manifest) [task 2018-04-24T06:40:36.936Z] 0:14.78 /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json 0 error webdriver/tests/get_active_element/user_prompts.py in manifest but removed from source. (wpt-manifest) [task 2018-04-24T06:40:37.110Z] 0:14.95 ERROR Manifest /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json is outdated, use |mach wpt-manifest-update| to fix. [task 2018-04-24T06:44:37.173Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json:0 | webdriver/tests/fullscreen_window/user_prompts.py in manifest but removed from source. (wpt-manifest) [task 2018-04-24T06:44:37.174Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/web-platform/meta/MANIFEST.json:0 | webdriver/tests/get_active_element/user_prompts.py in manifest but removed from source. (wpt-manifest) [taskcluster 2018-04-24 06:44:37.481Z] === Task Finished === [taskcluster 2018-04-24 06:44:37.481Z] Unsuccessful task run with exit code: 1 completed in 515.772 seconds

Sorry the last manifest update added accidentally two files which are targeted for a different bug locally. I will push a fix, and also check how often user_prompts.py fails.

The failure is actually again a shutdown hang of Firefox: [task 2018-04-24T07:00:31.762Z] 07:00:31 INFO - PID 1194 | 1524553231753 Marionette TRACE 0 -> [0,12,"Marionette:Quit",{"flags":["eForceQuit"]}] [task 2018-04-24T07:00:31.763Z] 07:00:31 INFO - PID 1194 | 1524553231754 Marionette TRACE 0 <- [1,12,{"error":"invalid session id","message":"","stacktrace":"WebDriverError@chrome://marionette/content/error.js:178:5\nInv ... et@chrome://marionette/content/server.js:245:8\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:500:9\n"},null] [task 2018-04-24T07:00:31.764Z] 07:00:31 INFO - PID 1194 | 1524553231754 geckodriver::marionette TRACE <- [1,12,{"error":"invalid session id","message":"","stacktrace":"WebDriverError@chrome://marionette/content/error.js:178:5\nInvalidSessionIDError@chrome://marionette/content/error.js:347:5\nassert.that/<@chrome://marionette/content/assert.js:405:13\nassert.session@chrome://marionette/content/assert.js:79:3\ndespatch@chrome://marionette/content/server.js:294:7\nasync*execute@chrome://marionette/content/server.js:271:11\nasync*onPacket/<@chrome://marionette/content/server.js:246:15\nasync*onPacket@chrome://marionette/content/server.js:245:8\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:500:9\n"},null] [task 2018-04-24T07:00:31.782Z] 07:00:31 INFO - PID 1194 | 1524553231771 Marionette DEBUG Closed connection 0 [task 2018-04-24T07:00:57.853Z] 07:00:57 INFO - TEST-UNEXPECTED-TIMEOUT | /webdriver/tests/close_window/user_prompts.py | expected OK So nothing we can do here right now. If it fails only intermittently we can land.

The latest try looks fine and shows the shutdown hang not that often. So I'm going to reland the patch. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e512bc224013bf7744b3661e61b12d2471cdb9a7

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e60e61507ef4 [wdclient] End session if no more windows are open. r=ato https://hg.mozilla.org/integration/autoland/rev/90908c3a6f9e [wdspec] Ignore exceptions when switching to top-level browsing context. r=ato https://hg.mozilla.org/integration/autoland/rev/2ec2ee71cc27 [wdspec] Add tests for Close Window command. r=ato

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

https://hg.mozilla.org/mozilla-central/rev/e60e61507ef4 https://hg.mozilla.org/mozilla-central/rev/90908c3a6f9e https://hg.mozilla.org/mozilla-central/rev/2ec2ee71cc27

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

Upstream PR merged