fix: Browser.quit() stops managed driver service; document facade semantics

This commit is contained in:
Mathew Sir Guest the best 2026-07-08 22:08:56 -06:00
parent 4734915475
commit 3d0e00e88f
3 changed files with 179 additions and 9 deletions

@ -2455,6 +2455,7 @@ Naming note: the spec says "`reset_good_status()` retained"; under the approved
`tests/unit/test_browser.py`:
```python
from types import SimpleNamespace
from unittest.mock import MagicMock
import pytest
@ -2516,7 +2517,23 @@ class TestDelegation:
def test_getattr_without_page_raises(self, bot):
with pytest.raises(AttributeError):
bot.do_login
_ = bot.do_login
def test_direct_delegation_does_not_flip_good(self, bot):
# only perform() traps+flips; direct bot.x() is the raw path
bot.set_page(Login)
with pytest.raises(WebDriverException):
bot.explode()
assert bot.good is True
def test_getitem_without_page_raises_friendly(self, bot):
with pytest.raises(RuntimeError, match="no current page"):
bot["username"]
def test_repr_shows_page_and_good(self, bot):
assert "page=None" in repr(bot) and "good=True" in repr(bot)
bot.set_page(Login)
assert "page=Login" in repr(bot)
class TestPerform:
@ -2541,10 +2558,18 @@ class TestPerform:
bot.reset()
assert bot.perform("do_login") == "logged-in"
def test_broken_state_attribute_read_returns_noop_callable(self, bot):
# documents the footgun: while broken, an attribute read yields the
# no-op refusal callable, not the underlying value
bot.set_page(Login)
bot.perform("explode") # flips good=False
assert callable(bot.anything)
class TestQuit:
def test_quit_quits_driver_and_removes_session(self):
driver, store = MagicMock(), MagicMock()
store.get.return_value = SimpleNamespace(service_pid=None) # external: nothing to stop
bot = Browser(driver, session_name="s1", store=store)
bot.quit()
driver.quit.assert_called_once_with()
@ -2554,6 +2579,30 @@ class TestQuit:
driver = MagicMock()
Browser(driver).quit()
driver.quit.assert_called_once_with()
def test_quit_stops_managed_service(self, monkeypatch):
import wabot.browser as browser_mod
stopped = []
monkeypatch.setattr(browser_mod, "stop_service", lambda pid: stopped.append(pid))
driver, store = MagicMock(), MagicMock()
store.get.return_value = SimpleNamespace(service_pid=4321) # managed session
bot = Browser(driver, session_name="s1", store=store)
bot.quit()
assert stopped == [4321]
store.remove.assert_called_once_with("s1")
def test_quit_removes_record_even_if_driver_quit_raises(self, monkeypatch):
import wabot.browser as browser_mod
monkeypatch.setattr(browser_mod, "stop_service", lambda pid: None)
driver, store = MagicMock(), MagicMock()
driver.quit.side_effect = WebDriverException("boom")
store.get.return_value = SimpleNamespace(service_pid=None)
bot = Browser(driver, session_name="s1", store=store)
with pytest.raises(WebDriverException):
bot.quit()
store.remove.assert_called_once_with("s1") # finally block still ran
```
- [ ] **Step 2: Run tests to verify they fail**
@ -2573,6 +2622,18 @@ access is delegated to the current page (``bot.login()`` invokes
``bot.page.login()``); ``bot[key]`` resolves elements on it. Any
``perform()`` failure flips ``good = False``, after which page actions
are refused until ``reset()`` — "log and refuse, don't crash".
Two ways to drive the current page:
* Direct delegation — ``bot.login()`` calls ``bot.page.login()``. This is
the raw path: it raises on error and does NOT touch ``good``.
* ``bot.perform("login")`` — traps driver failures, flips ``good`` to
False on error, and refuses further page actions until ``reset()``.
Route anything that can fail through ``perform()`` for the guard. While
``good`` is False, refused delegation returns a no-op callable, so
``bot.x()`` yields None; reading a page *attribute* in that state is not
meaningful.
"""
from __future__ import annotations
@ -2581,6 +2642,7 @@ import logging
from selenium.common.exceptions import WebDriverException
from .hosts import stop_service
from .pacing import HumanPacing
LOGGER = logging.getLogger("wabot")
@ -2604,8 +2666,15 @@ class Browser:
self.good = True
self.page = None
def __repr__(self) -> str:
page = type(self.page).__name__ if self.page is not None else None
return f"<Browser page={page} good={self.good}>"
def set_page(self, page_cls) -> bool:
"""Instantiate ``page_cls`` and make it current if its verify() passes."""
"""Instantiate ``page_cls`` and make it current if its verify() passes.
A consumer ``verify()`` that raises propagates (it is not trapped).
"""
page = page_cls(self)
if not page.verify():
LOGGER.error("failed to verify page: %s", page_cls.__name__)
@ -2626,10 +2695,14 @@ class Browser:
return getattr(self.page, name)
def __getitem__(self, key):
if self.page is None:
raise RuntimeError(
f"cannot resolve element {key!r}: no current page — call set_page() first"
)
return self.page[key]
def perform(self, method: str, *args, **kwargs):
"""Invoke a page method, trapping failures instead of raising."""
"""Invoke a page method, trapping driver failures instead of raising."""
if not self.good:
LOGGER.warning("broken state — refusing %r (call reset())", method)
return None
@ -2650,10 +2723,22 @@ class Browser:
self.good = True
def quit(self) -> None:
"""Quit the browser; forget the saved session if there is one."""
"""Quit the browser and forget the saved session.
For a wabot-managed driver service, its detached process is also
stopped — a plain ``driver.quit()`` leaves that service running.
External servers (records without a ``service_pid``) are left alone.
Use this for full teardown; to keep a session alive for another
process, let this process exit WITHOUT calling ``quit()``.
"""
record = None
if self._store is not None and self.session_name:
record = self._store.get(self.session_name)
try:
self.driver.quit()
finally:
if record is not None and record.service_pid:
stop_service(record.service_pid)
if self._store is not None and self.session_name:
self._store.remove(self.session_name)
```
@ -2661,7 +2746,7 @@ class Browser:
- [ ] **Step 4: Run tests to verify they pass**
Run: `uv run pytest tests/unit/test_browser.py -v`
Expected: 13 passed
Expected: 19 passed
- [ ] **Step 5: Commit**
@ -2993,7 +3078,7 @@ Run: `uv run pytest tests/unit/test_api.py -v`
Expected: 11 passed
Run: `uv run pytest`
Expected: full unit suite passes (128 tests: pacing 7, sessions 14, hosts 19, reattach 6, fields 17, page 36, screenshot 5, browser 13, api 11)
Expected: full unit suite passes (134 tests: pacing 7, sessions 14, hosts 19, reattach 6, fields 17, page 36, screenshot 5, browser 19, api 11)
- [ ] **Step 5: Commit**

@ -5,6 +5,18 @@ access is delegated to the current page (``bot.login()`` invokes
``bot.page.login()``); ``bot[key]`` resolves elements on it. Any
``perform()`` failure flips ``good = False``, after which page actions
are refused until ``reset()`` "log and refuse, don't crash".
Two ways to drive the current page:
* Direct delegation ``bot.login()`` calls ``bot.page.login()``. This is
the raw path: it raises on error and does NOT touch ``good``.
* ``bot.perform("login")`` traps driver failures, flips ``good`` to
False on error, and refuses further page actions until ``reset()``.
Route anything that can fail through ``perform()`` for the guard. While
``good`` is False, refused delegation returns a no-op callable, so
``bot.x()`` yields None; reading a page *attribute* in that state is not
meaningful.
"""
from __future__ import annotations
@ -13,6 +25,7 @@ import logging
from selenium.common.exceptions import WebDriverException
from .hosts import stop_service
from .pacing import HumanPacing
LOGGER = logging.getLogger("wabot")
@ -36,8 +49,15 @@ class Browser:
self.good = True
self.page = None
def __repr__(self) -> str:
page = type(self.page).__name__ if self.page is not None else None
return f"<Browser page={page} good={self.good}>"
def set_page(self, page_cls) -> bool:
"""Instantiate ``page_cls`` and make it current if its verify() passes."""
"""Instantiate ``page_cls`` and make it current if its verify() passes.
A consumer ``verify()`` that raises propagates (it is not trapped).
"""
page = page_cls(self)
if not page.verify():
LOGGER.error("failed to verify page: %s", page_cls.__name__)
@ -58,10 +78,14 @@ class Browser:
return getattr(self.page, name)
def __getitem__(self, key):
if self.page is None:
raise RuntimeError(
f"cannot resolve element {key!r}: no current page — call set_page() first"
)
return self.page[key]
def perform(self, method: str, *args, **kwargs):
"""Invoke a page method, trapping failures instead of raising."""
"""Invoke a page method, trapping driver failures instead of raising."""
if not self.good:
LOGGER.warning("broken state — refusing %r (call reset())", method)
return None
@ -82,9 +106,21 @@ class Browser:
self.good = True
def quit(self) -> None:
"""Quit the browser; forget the saved session if there is one."""
"""Quit the browser and forget the saved session.
For a wabot-managed driver service, its detached process is also
stopped a plain ``driver.quit()`` leaves that service running.
External servers (records without a ``service_pid``) are left alone.
Use this for full teardown; to keep a session alive for another
process, let this process exit WITHOUT calling ``quit()``.
"""
record = None
if self._store is not None and self.session_name:
record = self._store.get(self.session_name)
try:
self.driver.quit()
finally:
if record is not None and record.service_pid:
stop_service(record.service_pid)
if self._store is not None and self.session_name:
self._store.remove(self.session_name)

@ -1,3 +1,4 @@
from types import SimpleNamespace
from unittest.mock import MagicMock
import pytest
@ -61,6 +62,22 @@ class TestDelegation:
with pytest.raises(AttributeError):
_ = bot.do_login
def test_direct_delegation_does_not_flip_good(self, bot):
# only perform() traps+flips; direct bot.x() is the raw path
bot.set_page(Login)
with pytest.raises(WebDriverException):
bot.explode()
assert bot.good is True
def test_getitem_without_page_raises_friendly(self, bot):
with pytest.raises(RuntimeError, match="no current page"):
bot["username"]
def test_repr_shows_page_and_good(self, bot):
assert "page=None" in repr(bot) and "good=True" in repr(bot)
bot.set_page(Login)
assert "page=Login" in repr(bot)
class TestPerform:
def test_returns_method_result(self, bot):
@ -84,10 +101,18 @@ class TestPerform:
bot.reset()
assert bot.perform("do_login") == "logged-in"
def test_broken_state_attribute_read_returns_noop_callable(self, bot):
# documents the footgun: while broken, an attribute read yields the
# no-op refusal callable, not the underlying value
bot.set_page(Login)
bot.perform("explode") # flips good=False
assert callable(bot.anything)
class TestQuit:
def test_quit_quits_driver_and_removes_session(self):
driver, store = MagicMock(), MagicMock()
store.get.return_value = SimpleNamespace(service_pid=None) # external: nothing to stop
bot = Browser(driver, session_name="s1", store=store)
bot.quit()
driver.quit.assert_called_once_with()
@ -97,3 +122,27 @@ class TestQuit:
driver = MagicMock()
Browser(driver).quit()
driver.quit.assert_called_once_with()
def test_quit_stops_managed_service(self, monkeypatch):
import wabot.browser as browser_mod
stopped = []
monkeypatch.setattr(browser_mod, "stop_service", lambda pid: stopped.append(pid))
driver, store = MagicMock(), MagicMock()
store.get.return_value = SimpleNamespace(service_pid=4321) # managed session
bot = Browser(driver, session_name="s1", store=store)
bot.quit()
assert stopped == [4321]
store.remove.assert_called_once_with("s1")
def test_quit_removes_record_even_if_driver_quit_raises(self, monkeypatch):
import wabot.browser as browser_mod
monkeypatch.setattr(browser_mod, "stop_service", lambda pid: None)
driver, store = MagicMock(), MagicMock()
driver.quit.side_effect = WebDriverException("boom")
store.get.return_value = SimpleNamespace(service_pid=None)
bot = Browser(driver, session_name="s1", store=store)
with pytest.raises(WebDriverException):
bot.quit()
store.remove.assert_called_once_with("s1") # finally block still ran