Merge branch 'main' into andrew/use_process_group

Author: ayjayt

.pre-commit-config.yaml

@@ -66,7 +66,7 @@ repos:
         language: python
         additional_dependencies: ["./gitlint-core[trusted-deps]"]
         entry: gitlint
-        args: [--staged, --msg-filename]
+        args: [--staged, -c, "general.ignore=B6,T3", --msg-filename]
         stages: [commit-msg]
   - repo: https://github.com/crate-ci/typos
     rev: v1

CHANGELOG.txt

@@ -1,4 +1,14 @@
 - Change to process group for better killing of multi-process chrome
+- Add argument to Session/Target send_command with_perf to return
+  timing information about browser write/read.
+- Update default chrome from 135.0.7011.0/1418433 to 144.0.7527.0/1544685
+- Fix: New chrome takes longer/doesn't populate targets right away, so add a
+  retry loop to populate targets
+- Alter `get_chrome` verbose to print whole JSON
+- Change chrome download path to use XDG cache dir
+- Don't download chrome if we already have that version: add force argument
+- Remove unused system inspection code
+- Add a set of helper functions to await for tab loading and send javascript
 v1.2.1
 - Use custom threadpool for functions that could be running during shutdown:
   Python's stdlib threadpool isn't available during interpreter shutdown, nor

ROADMAP.md

@@ -1,5 +1,8 @@
 # Roadmap
 
+- [ ] Fix up browser deps error (eliminate in-package analysis)
+- [ ] Switch to process group and kill that to catch all chromium children
+- [ ] Add helpers for running javascript
 - [ ] Working on better diagnostic information
   - [ ] Explain to user when their system has security restrictions
 - [ ] Eliminate synchronous API: it's unused, hard to maintain, and nearly
@@ -15,4 +18,3 @@
 - [ ] Do documentation
 - [ ] Browser Open/Close Status/PipeCheck status should happen at broker level
   - [ ] Broker should probably be opening browser and running watchdog...
-- [ ] Add a connect only for websockets

pyproject.toml

@@ -30,6 +30,7 @@ maintainers = [
 ]
 dependencies = [
   "logistro>=2.0.1",
+  "platformdirs>=4.3.6",
   "simplejson>=3.19.3",
 ]
 
@@ -43,17 +44,17 @@ choreo_get_chrome = "choreographer.cli._cli_utils:get_chrome_cli"
 
 [dependency-groups]
 dev = [
-  "pytest",
-  "pytest-asyncio; python_version < '3.14'",
-  "pytest-asyncio>=1.2.0; python_version >= '3.14'",
-  "pytest-xdist",
   "async-timeout",
   "numpy; python_version < '3.11'",
   "numpy>=2.3.3; python_version >= '3.11'",
   "mypy>=1.14.1",
   "types-simplejson>=3.19.0.20241221",
   "poethepoet>=0.30.0",
   "pyright>=1.1.406",
+  "pytest",
+  "pytest-asyncio; python_version < '3.14'",
+  "pytest-asyncio>=1.2.0; python_version >= '3.14'",
+  "pytest-xdist",
 ]
 
 # uv doens't allow dependency groups to have separate python requirements
@@ -105,12 +106,14 @@ ignore = [
             ]
 
 [tool.pytest.ini_options]
+asyncio_mode = "auto"
 asyncio_default_fixture_loop_scope = "function"
 log_cli = false
 addopts = "--import-mode=append"
 
+# tell poe to use the env we give it, otherwise it detects uv and overrides flags
 [tool.poe]
-executor.type = "virtualenv"
+executor.type = "simple"
 
 [tool.poe.tasks]
 test_proc = "pytest --log-level=1 -W error -n auto -v -rfE --capture=fd tests/test_process.py"
@@ -123,10 +126,12 @@ sequence = ["test_proc", "test_fn"]
 help = "Run all tests quickly"
 
 [tool.poe.tasks.debug-test]
+env = { CHOREO_ENABLE_DEBUG="1" }
 sequence = ["debug-test_proc", "debug-test_fn"]
 help = "Run test by test, slowly, quitting after first error"
 
 [tool.poe.tasks.filter-test]
+env = { CHOREO_ENABLE_DEBUG="1" }
 cmd = "pytest --log-level=1 -W error -vvvx -rA --capture=no --show-capture=no"
 help = "Run any/all tests one by one with basic settings: can include filename and -k filters"
 

src/choreographer/_brokers/_async.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import asyncio
+import time
 import warnings
 from functools import partial
 from typing import TYPE_CHECKING
@@ -22,6 +23,9 @@
 
 _logger = logistro.getLogger(__name__)
 
+PERFS_MAX = 5000  # maximum number of entries in the perf dicts
+TRIM_SIZE = 500  # what to save after trimming it
+
 
 class UnhandledMessageWarning(UserWarning):
     pass
@@ -49,6 +53,9 @@ class Broker:
     ]
     """A mapping of session id: subscription: list[futures]"""
 
+    write_perfs: MutableMapping[protocol.MessageKey, tuple[float, float]]
+    read_perfs: MutableMapping[protocol.MessageKey, float]
+
     def __init__(self, browser: Browser, channel: ChannelInterface) -> None:
         """
         Construct a broker for a synchronous arragenment w/ both ends.
@@ -66,6 +73,8 @@ def __init__(self, browser: Browser, channel: ChannelInterface) -> None:
         # if its a user task, can cancel
         self._current_read_task: asyncio.Task[Any] | None = None
         self.futures = {}
+        self.write_perfs = {}
+        self.read_perfs = {}
         self._subscriptions_futures = {}
 
         self._write_lock = asyncio.Lock()
@@ -223,6 +232,14 @@ async def read_loop() -> None:  # noqa: PLR0912, PLR0915, C901
                         raise RuntimeError(f"Couldn't find a future for key: {key}")
                     if not future.done():
                         future.set_result(response)
+                        self.read_perfs[key] = time.perf_counter()
+                        if len(self.write_perfs) > PERFS_MAX:
+                            self.write_perfs = dict(
+                                list(self.write_perfs.items())[TRIM_SIZE:],
+                            )
+                            self.read_perfs = dict(
+                                list(self.read_perfs.items())[TRIM_SIZE:],
+                            )
                 else:
                     warnings.warn(
                         f"Unhandled message type:{response!s}",
@@ -237,6 +254,16 @@ async def read_loop() -> None:  # noqa: PLR0912, PLR0915, C901
         read_task.add_done_callback(check_read_loop_error)
         self._current_read_task = read_task
 
+    def get_perf(
+        self,
+        obj: protocol.BrowserCommand,
+    ) -> tuple[float, float, float]:
+        """Get the performance tuple for a certain BrowserCommand."""
+        key = protocol.calculate_message_key(obj)
+        if not key:
+            return (0, 0, 0)
+        return (*self.write_perfs[key], self.read_perfs[key])
+
     async def write_json(
         self,
         obj: protocol.BrowserCommand,
@@ -254,13 +281,15 @@ async def write_json(
         self.futures[key] = future
         _logger.debug(f"Created future: {key} {future}")
         try:
+            perf_start = time.perf_counter()
             async with self._write_lock:  # this should be a queue not a lock
                 loop = asyncio.get_running_loop()
                 await loop.run_in_executor(
                     self._executor,
                     self._channel.write_json,
                     obj,
                 )
+            self.write_perfs[key] = (perf_start, time.perf_counter())
         except (_manual_thread_pool.ExecutorClosedError, asyncio.CancelledError) as e:
             if not future.cancel() or not future.cancelled():
                 await future  # it wasn't canceled, so listen to it before raising

src/choreographer/browser_async.py

@@ -16,7 +16,7 @@
 from choreographer import protocol
 
 from ._brokers import Broker
-from .browsers import BrowserClosedError, BrowserDepsError, BrowserFailedError, Chromium
+from .browsers import BrowserClosedError, BrowserFailedError, Chromium
 from .channels import ChannelClosedError, Pipe
 from .protocol.devtools_async import Session, Target
 from .utils import TmpDirWarning, _manual_thread_pool
@@ -32,6 +32,9 @@
     from .browsers._interface_type import BrowserImplInterface
     from .channels._interface_type import ChannelInterface
 
+_N = MAX_POPULATE_LOOPS = 20
+
+
 _logger = logistro.getLogger(__name__)
 
 # Since I added locks to pipes, do we need locks here?
@@ -164,15 +167,16 @@ def run() -> subprocess.Popen[bytes] | subprocess.Popen[str]:  # depends on args
             self._channel.open()  # should this and below be in a broker run
             _logger.debug("Running read loop")
             self._broker.run_read_loop()
-            _logger.debug("Populating Targets")
-            await asyncio.sleep(0)  # let watchdog start
-            await self.populate_targets()
+            await asyncio.sleep(0)  # let watchdog start before populate
+            counter = 0
+            while not self.get_tab():
+                _logger.debug("Populating Targets")
+                await self.populate_targets()
+                await asyncio.sleep(0.1)
+                counter += 1
+                if counter == MAX_POPULATE_LOOPS:
+                    break
         except (BrowserClosedError, BrowserFailedError, asyncio.CancelledError) as e:
-            if (
-                hasattr(self._browser_impl, "missing_libs")
-                and self._browser_impl.missing_libs  # type: ignore[reportAttributeAccessIssue]
-            ):
-                raise BrowserDepsError from e
             raise BrowserFailedError(
                 "The browser seemed to close immediately after starting.",
                 "You can set the `logging.Logger` level lower to see more output.",
@@ -352,7 +356,6 @@ async def populate_targets(self) -> None:
             raise RuntimeError("Could not get targets") from Exception(
                 response["error"],
             )
-
         for json_response in response["result"]["targetInfos"]:
             if (
                 json_response["type"] == "page"

src/choreographer/browsers/_errors.py

@@ -6,6 +6,7 @@ class BrowserFailedError(RuntimeError):
     """An error for when the browser fails to launch."""
 
 
+# not currently used but keeping copy + not breaking API
 class BrowserDepsError(BrowserFailedError):
     """An error for when the browser is closed because of missing libs."""
 

src/choreographer/browsers/chromium.py

@@ -115,46 +115,6 @@ def logger_parser(
 
         return True
 
-    def _libs_ok(self) -> bool:
-        """Return true if libs ok."""
-        if self.skip_local:
-            _logger.debug(
-                "If we HAVE to skip local.",
-            )
-            return True
-        _logger.debug("Checking for libs needed.")
-        if platform.system() != "Linux":
-            _logger.debug("We're not in linux, so no need for check.")
-            return True
-        p = None
-        try:
-            _logger.debug(f"Trying ldd {self.path}")
-            p = subprocess.run(  # noqa: S603, validating run with variables
-                [  # noqa: S607 path is all we have
-                    "ldd",
-                    str(self.path),
-                ],
-                capture_output=True,
-                timeout=5,
-                check=True,
-            )
-        except Exception as e:  # noqa: BLE001
-            msg = "ldd failed."
-            stderr = p.stderr.decode() if p and p.stderr else None
-            # Log failure as INFO rather than WARNING so that it's hidden by default,
-            # since browser may succeed even if ldd fails
-            _logger.info(
-                msg  # noqa: G003 + in log
-                + f" e: {e}, stderr: {stderr}",
-            )
-            return False
-        if b"not found" in p.stdout:
-            msg = "Found deps missing in chrome"
-            _logger.debug2(msg + f" {p.stdout.decode()}")
-            return False
-        _logger.debug("No problems found with dependencies")
-        return True
-
     def __init__(
         self,
         channel: ChannelInterface,
@@ -220,7 +180,6 @@ def pre_open(self) -> None:
             path=self._tmp_dir_path,
             sneak=self._is_isolated,
         )
-        self.missing_libs = not self._libs_ok()
         _logger.info(f"Temporary directory at: {self.tmp_dir.path}")
 
     def is_isolated(self) -> bool: