diff --git a/docs/superpowers/plans/2026-07-07-wabot-modernization.md b/docs/superpowers/plans/2026-07-07-wabot-modernization.md index 8a5a3cc..6c37d8c 100644 --- a/docs/superpowers/plans/2026-07-07-wabot-modernization.md +++ b/docs/superpowers/plans/2026-07-07-wabot-modernization.md @@ -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"" + 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** diff --git a/src/wabot/browser.py b/src/wabot/browser.py index 17f47aa..8a62c0a 100644 --- a/src/wabot/browser.py +++ b/src/wabot/browser.py @@ -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"" + 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) diff --git a/tests/unit/test_browser.py b/tests/unit/test_browser.py index 31cc48d..7b71428 100644 --- a/tests/unit/test_browser.py +++ b/tests/unit/test_browser.py @@ -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