Closed
Description
There appear to be certain cases where we create a new WebDriver session just to run our teardown code. This seems… bad.
This is, as far as I can tell, caused by a combination of our behaviour implicitly creating sessions when trying to run commands and our teardown code running commands.
Sometimes it even appears like we're restarting the browser, via creating a new session, while we're trying to terminate the runner — I think as a result of this.
This behaviour has always seemed slightly scary, because it carries the risk of something ignoring an exception during a test, and then when another WebDriver command is run then creating an entirely new session in the middle of a test — and I hope we all agree that that is clearly incorrect behaviour.
I don't know why we have the implicit session creation — it does appear to go all the way back to the original implementation in w3c/wptrunner#109 (cc @jgraham).
Changing command to instead assert session_id is not None leads to the following:
ERROR test_pointer_down_closes_browsing_context - teardown error:
+ Exception Group Traceback (most recent call last):
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 341, in from_call
| result: Optional[TResult] = func()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 241, in <lambda>
| lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_hooks.py", line 513, in __call__
| return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_manager.py", line 120, in _hookexec
| return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 139, in _multicall
| raise exception.with_traceback(exception.__traceback__)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 122, in _multicall
| teardown.throw(exception) # type: ignore[union-attr]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/threadexception.py", line 92, in pytest_runtest_teardown
| yield from thread_exception_runtest_hook()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/threadexception.py", line 63, in thread_exception_runtest_hook
| yield
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 122, in _multicall
| teardown.throw(exception) # type: ignore[union-attr]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/unraisableexception.py", line 95, in pytest_runtest_teardown
| yield from unraisable_exception_runtest_hook()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/unraisableexception.py", line 65, in unraisable_exception_runtest_hook
| yield
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 122, in _multicall
| teardown.throw(exception) # type: ignore[union-attr]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/logging.py", line 857, in pytest_runtest_teardown
| yield from self._runtest_for(item, "teardown")
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/logging.py", line 833, in _runtest_for
| yield
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 122, in _multicall
| teardown.throw(exception) # type: ignore[union-attr]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/capture.py", line 883, in pytest_runtest_teardown
| return (yield)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pluggy/src/pluggy/_callers.py", line 103, in _multicall
| res = hook_impl.function(*args)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 188, in pytest_runtest_teardown
| item.session._setupstate.teardown_exact(nextitem)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 557, in teardown_exact
| raise exceptions[0]
| exceptiongroup.ExceptionGroup: errors while tearing down <Function test_pointer_down_closes_browsing_context> (3 sub-exceptions)
+-+---------------- 1 ----------------
| Traceback (most recent call last):
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 546, in teardown_exact
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1020, in finish
| raise exceptions[0]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1009, in finish
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest-asyncio/pytest_asyncio/plugin.py", line 291, in finalizer
| event_loop.run_until_complete(async_finalizer())
| File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
| return future.result()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest-asyncio/pytest_asyncio/plugin.py", line 283, in async_finalizer
| await gen_obj.__anext__()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/support/fixtures.py", line 197, in session
| cleanup_session(_current_session)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/support/helpers.py", line 84, in cleanup_session
| _ensure_valid_window(session)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/support/helpers.py", line 17, in inner
| return f(*args, **kwargs)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/support/helpers.py", line 44, in _ensure_valid_window
| session.window_handle
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 19, in inner
| assert session.session_id is not None
| AssertionError
+---------------- 2 ----------------
| Traceback (most recent call last):
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 546, in teardown_exact
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1020, in finish
| raise exceptions[0]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1009, in finish
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 893, in _teardown_yield_fixture
| next(it)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/classic/perform_actions/conftest.py", line 78, in release_actions
| session.actions.release()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 19, in inner
| assert session.session_id is not None
| AssertionError
+---------------- 3 ----------------
| Traceback (most recent call last):
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/runner.py", line 546, in teardown_exact
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1020, in finish
| raise exceptions[0]
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 1009, in finish
| fin()
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/third_party/pytest/src/_pytest/fixtures.py", line 893, in _teardown_yield_fixture
| next(it)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/webdriver/tests/support/fixtures_http.py", line 170, in http_new_tab
| session.window_handle = new_handle
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 21, in inner
| return func(self, *args, **kwargs)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 655, in window_handle
| return self.send_session_command("POST", "window", body=body)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 602, in send_session_command
| return self.send_command(method, url, body, timeout)
| File "/Volumes/gsnedders/projects/wpt/web-platform-tests/tools/webdriver/webdriver/client.py", line 566, in send_command
| raise err
| webdriver.error.InvalidSessionIdException: invalid session id (404)
+------------------------------------
And that shows we do indeed just end up implicitly creating a new session while trying to teardown our fixtures.
Can we just… get rid of this behaviour?
See also: #51816