From 13326080a5160b2605f3b92dd0301e07f2d81bec Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Thu, 13 Nov 2025 19:14:14 -0500 Subject: [PATCH 01/15] Check for existing chrome in get_chrome and add force option --- src/choreographer/cli/_cli_utils.py | 79 +++++++++++++++++----------- tests/test_get_chrome.py | 81 +++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 30 deletions(-) create mode 100644 tests/test_get_chrome.py diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index 8a1ce57e..bc1ae168 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -23,7 +23,7 @@ supported_platform_strings = ["linux64", "win32", "win64", "mac-x64", "mac-arm64"] -def get_google_supported_platform_string() -> tuple[str, str, str, str]: +def get_google_supported_platform_string() -> str | None: arch_size_detected = "64" if sys.maxsize > 2**32 else "32" arch_detected = "arm" if platform.processor() == "arm" else "x" @@ -39,11 +39,11 @@ def get_google_supported_platform_string() -> tuple[str, str, str, str]: if chrome_platform_detected in supported_platform_strings: platform_string = chrome_platform_detected - return platform_string, arch_size_detected, platform.processor(), platform.system() + return platform_string def get_chrome_download_path() -> Path | None: - _chrome_platform_detected, _, _, _ = get_google_supported_platform_string() + _chrome_platform_detected = get_google_supported_platform_string() if not _chrome_platform_detected: return None @@ -85,17 +85,19 @@ def _extract_member(self, member, targetpath, pwd): # type: ignore [no-untyped- return path -def get_chrome_sync( # noqa: PLR0912, C901 +def get_chrome_sync( # noqa: C901, PLR0912 arch: str | None = None, i: int | None = None, path: str | Path = default_download_path, *, verbose: bool = False, + force: bool = False, ) -> Path | str: """Download chrome synchronously: see `get_chrome()`.""" - if not arch: - arch, _, _, _ = get_google_supported_platform_string() + if isinstance(path, str): + path = Path(path) + arch = arch or get_google_supported_platform_string() if not arch: raise RuntimeError( "You must specify an arch, one of: " @@ -103,8 +105,6 @@ def get_chrome_sync( # noqa: PLR0912, C901 f"Detected {arch} is not supported.", ) - if isinstance(path, str): - path = Path(path) if i: _logger.info("Loading chrome from list") browser_list = json.loads( @@ -115,17 +115,16 @@ def get_chrome_sync( # noqa: PLR0912, C901 version_obj = browser_list["versions"][i] else: _logger.info("Using last known good version of chrome") - with ( - Path(__file__).resolve().parent.parent - / "resources" - / "last_known_good_chrome.json" - ).open() as f: - version_obj = json.load(f) - if verbose: - print(version_obj["version"]) # noqa: T201 allow print in cli - print(version_obj["revision"]) # noqa: T201 allow print in cli + version_obj = json.loads( + ( + Path(__file__).resolve().parent.parent + / "resources" + / "last_known_good_chrome.json" + ).read_text(), + ) + version_string = f"{version_obj['version']}\n{version_obj['revision']}" chromium_sources = version_obj["downloads"]["chrome"] - url = "" + for src in chromium_sources: if src["platform"] == arch: url = src["url"] @@ -137,19 +136,16 @@ def get_chrome_sync( # noqa: PLR0912, C901 f"{arch} is not supported.", ) - if not path.exists(): - path.mkdir(parents=True) - filename = path / "chrome.zip" - with urllib.request.urlopen(url) as response, filename.open("wb") as out_file: # noqa: S310 audit url - shutil.copyfileobj(response, out_file) - with _ZipFilePermissions(filename, "r") as zip_ref: - zip_ref.extractall(path) - filename.unlink() + if verbose: + print(version_string) # noqa: T201 allow print in cli + version_tag = path / "version_tag.txt" + + path.mkdir(parents=True, exist_ok=True) if arch.startswith("linux"): - exe_name = path / f"chrome-{arch}" / "chrome" + exe_path = path / f"chrome-{arch}" / "chrome" elif arch.startswith("mac"): - exe_name = ( + exe_path = ( path / f"chrome-{arch}" / "Google Chrome for Testing.app" @@ -158,10 +154,33 @@ def get_chrome_sync( # noqa: PLR0912, C901 / "Google Chrome for Testing" ) elif arch.startswith("win"): - exe_name = path / f"chrome-{arch}" / "chrome.exe" + exe_path = path / f"chrome-{arch}" / "chrome.exe" else: raise RuntimeError("Couldn't calculate exe_name, unsupported architecture.") - return exe_name + + if ( + exe_path.exists() + and version_tag.is_file() + and version_tag.read_text() == version_string + and not force + ): + return exe_path + else: + if exe_path.exists(): # delete it + shutil.rmtree(exe_path) + if version_tag.exists(): # delete it + version_tag.unlink() + + # Download + zip_path = path / "chrome.zip" + with urllib.request.urlopen(url) as response, zip_path.open("wb") as out_file: # noqa: S310 audit url + shutil.copyfileobj(response, out_file) + with _ZipFilePermissions(zip_path, "r") as zip_ref: + zip_ref.extractall(path) + zip_path.unlink() + version_tag.write_text(version_string) + + return exe_path async def get_chrome( diff --git a/tests/test_get_chrome.py b/tests/test_get_chrome.py new file mode 100644 index 00000000..592f7837 --- /dev/null +++ b/tests/test_get_chrome.py @@ -0,0 +1,81 @@ +import io +import zipfile +from unittest.mock import MagicMock, patch + +import pytest + +from choreographer.cli._cli_utils import get_chrome_sync + + +@pytest.fixture +def mock_last_known_good_json(): + """Mock the last known good chrome version.""" + return { + "version": "135.0.7011.0", + "revision": "1418433", + "downloads": { + "chrome": [ + {"platform": "linux64", "url": "https://example.com/linux64.zip"}, + {"platform": "mac-arm64", "url": "https://example.com/mac-arm64.zip"}, + {"platform": "win64", "url": "https://example.com/win64.zip"}, + ], + }, + } + + +# Thanks Claude! +def test_get_chrome_sync_download_behavior( + tmp_path, + mock_last_known_good_json, +): + """Test chrome download/skip behavior: existing, force, version change.""" + exe_path = tmp_path / "chrome-linux64" / "chrome" + version_tag = tmp_path / "version_tag.txt" + + # Setup: create a fake existing installation with matching version + exe_path.parent.mkdir(parents=True) + exe_path.write_text("old chrome") + version_tag.write_text("135.0.7011.0\n1418433") + + # Mock the URL download + def create_mock_zip_response(): + mock_response = MagicMock() + # Create a fresh zip buffer each time + zip_buffer = io.BytesIO() + with zipfile.ZipFile(zip_buffer, "w") as zf: + zf.writestr("chrome-linux64/chrome", "newly downloaded chrome") + zip_buffer.seek(0) + mock_response.read.return_value = zip_buffer.read() + mock_response.__enter__ = MagicMock(return_value=mock_response) + mock_response.__exit__ = MagicMock(return_value=False) + return mock_response + + # Patch json.load to return our mock data (avoid broad Path.read_text patch) + with patch("json.load", return_value=mock_last_known_good_json), patch( + "urllib.request.urlopen", + side_effect=lambda url: create_mock_zip_response(), # noqa: ARG005 + ): + # a) First call without force - should return existing, no download + result = get_chrome_sync(arch="linux64", path=tmp_path, force=False) + assert result == exe_path + assert exe_path.read_text() == "old chrome" # Should still be old + + # b) Call with force=True - should download and replace + result = get_chrome_sync(arch="linux64", path=tmp_path, force=True) + assert result == exe_path + assert exe_path.read_text() == "newly downloaded chrome" + assert version_tag.read_text() == "135.0.7011.0\n1418433" + + # c) Call again without force - should return existing, no re-download + # (Modify the file to verify it doesn't get re-downloaded) + exe_path.write_text("manually modified chrome") + result = get_chrome_sync(arch="linux64", path=tmp_path, force=False) + assert result == exe_path + assert exe_path.read_text() == "manually modified chrome" + + # d) Change version on disk - should trigger download (version mismatch) + version_tag.write_text("999.0.0.0\n999999") # Old/different version + result = get_chrome_sync(arch="linux64", path=tmp_path, force=False) + assert result == exe_path + assert exe_path.read_text() == "newly downloaded chrome" + assert version_tag.read_text() == "135.0.7011.0\n1418433" # Updated to new From b3b6eedaeeee3bbd6bc76e0c7cdadb2caed2e56c Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Thu, 13 Nov 2025 19:22:27 -0500 Subject: [PATCH 02/15] Propogate arguments through --- src/choreographer/cli/_cli_utils.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index bc1ae168..ee470510 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -189,6 +189,7 @@ async def get_chrome( path: str | Path = default_download_path, *, verbose: bool = False, + force: bool = False, ) -> Path | str: """ Download google chrome from google-chrome-for-testing server. @@ -199,10 +200,18 @@ async def get_chrome( still in the testing directory. path: where to download it too (the folder). verbose: print out version found + force: download chrome again even if already present at that version """ loop = asyncio.get_running_loop() - fn = partial(get_chrome_sync, arch=arch, i=i, path=path, verbose=verbose) + fn = partial( + get_chrome_sync, + arch=arch, + i=i, + path=path, + verbose=verbose, + force=force, + ) return await loop.run_in_executor( executor=None, func=fn, @@ -250,6 +259,14 @@ def get_chrome_cli() -> None: action="store_true", help="Display found version number if using -i (to stdout)", ) + parser.add_argument( + "-f", + "--force", + dest="force", + action="store_true", + default=False, + help="Force download even if already present.", + ) parser.set_defaults(path=default_download_path) parser.set_defaults(arch=None) parser.set_defaults(verbose=False) @@ -257,5 +274,6 @@ def get_chrome_cli() -> None: i = parsed.i arch = parsed.arch path = Path(parsed.path) + force = parsed.force verbose = parsed.verbose - print(get_chrome_sync(arch=arch, i=i, path=path, verbose=verbose)) # noqa: T201 allow print in cli + print(get_chrome_sync(arch=arch, i=i, path=path, verbose=verbose, force=force)) # noqa: T201 allow print in cli From 4329e2be1a9386aad6ab8a251122769a64dc1e09 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Thu, 13 Nov 2025 19:31:43 -0500 Subject: [PATCH 03/15] Change installation directory --- pyproject.toml | 1 + src/choreographer/cli/_cli_utils.py | 9 ++++++- uv.lock | 42 +++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f5c293ae..d019ef4e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,6 +30,7 @@ maintainers = [ ] dependencies = [ "logistro>=2.0.1", + "platformdirs>=4.3.6", "simplejson>=3.19.3", ] diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index ee470510..dd461725 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -13,6 +13,7 @@ from pathlib import Path import logistro +from platformdirs import PlatformDirs from choreographer.cli.defaults import default_download_path @@ -48,7 +49,13 @@ def get_chrome_download_path() -> Path | None: if not _chrome_platform_detected: return None - _default_exe_path = Path() + _default_exe_path = ( + Path( + PlatformDirs("choreographer", "plotly").user_data_dir, + ) + / "deps" + ) + _default_exe_path.mkdir(parents=True, exist_ok=True) if platform.system().startswith("Linux"): _default_exe_path = ( diff --git a/uv.lock b/uv.lock index 8972506b..f288e144 100644 --- a/uv.lock +++ b/uv.lock @@ -32,6 +32,9 @@ name = "choreographer" source = { editable = "." } dependencies = [ { name = "logistro" }, + { name = "platformdirs", version = "4.3.6", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.9'" }, + { name = "platformdirs", version = "4.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version == '3.9.*'" }, + { name = "platformdirs", version = "4.5.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.10'" }, { name = "simplejson" }, ] @@ -60,6 +63,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "logistro", specifier = ">=2.0.1" }, + { name = "platformdirs", specifier = ">=4.3.6" }, { name = "simplejson", specifier = ">=3.19.3" }, ] @@ -543,6 +547,44 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cc/20/ff623b09d963f88bfde16306a54e12ee5ea43e9b597108672ff3a408aad6/pathspec-0.12.1-py3-none-any.whl", hash = "sha256:a0d503e138a4c123b27490a4f7beda6a01c6f288df0e4a8b79c7eb0dc7b4cc08", size = 31191, upload-time = "2023-12-10T22:30:43.14Z" }, ] +[[package]] +name = "platformdirs" +version = "4.3.6" +source = { registry = "https://pypi.org/simple" } +resolution-markers = [ + "python_full_version < '3.9'", +] +sdist = { url = "https://files.pythonhosted.org/packages/13/fc/128cc9cb8f03208bdbf93d3aa862e16d376844a14f9a0ce5cf4507372de4/platformdirs-4.3.6.tar.gz", hash = "sha256:357fb2acbc885b0419afd3ce3ed34564c13c9b95c89360cd9563f73aa5e2b907", size = 21302, upload-time = "2024-09-17T19:06:50.688Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/3c/a6/bc1012356d8ece4d66dd75c4b9fc6c1f6650ddd5991e421177d9f8f671be/platformdirs-4.3.6-py3-none-any.whl", hash = "sha256:73e575e1408ab8103900836b97580d5307456908a03e92031bab39e4554cc3fb", size = 18439, upload-time = "2024-09-17T19:06:49.212Z" }, +] + +[[package]] +name = "platformdirs" +version = "4.4.0" +source = { registry = "https://pypi.org/simple" } +resolution-markers = [ + "python_full_version == '3.9.*'", +] +sdist = { url = "https://files.pythonhosted.org/packages/23/e8/21db9c9987b0e728855bd57bff6984f67952bea55d6f75e055c46b5383e8/platformdirs-4.4.0.tar.gz", hash = "sha256:ca753cf4d81dc309bc67b0ea38fd15dc97bc30ce419a7f58d13eb3bf14c4febf", size = 21634, upload-time = "2025-08-26T14:32:04.268Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/40/4b/2028861e724d3bd36227adfa20d3fd24c3fc6d52032f4a93c133be5d17ce/platformdirs-4.4.0-py3-none-any.whl", hash = "sha256:abd01743f24e5287cd7a5db3752faf1a2d65353f38ec26d98e25a6db65958c85", size = 18654, upload-time = "2025-08-26T14:32:02.735Z" }, +] + +[[package]] +name = "platformdirs" +version = "4.5.0" +source = { registry = "https://pypi.org/simple" } +resolution-markers = [ + "python_full_version >= '3.14'", + "python_full_version >= '3.11' and python_full_version < '3.14'", + "python_full_version == '3.10.*'", +] +sdist = { url = "https://files.pythonhosted.org/packages/61/33/9611380c2bdb1225fdef633e2a9610622310fed35ab11dac9620972ee088/platformdirs-4.5.0.tar.gz", hash = "sha256:70ddccdd7c99fc5942e9fc25636a8b34d04c24b335100223152c2803e4063312", size = 21632, upload-time = "2025-10-08T17:44:48.791Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/73/cb/ac7874b3e5d58441674fb70742e6c374b28b0c7cb988d37d991cde47166c/platformdirs-4.5.0-py3-none-any.whl", hash = "sha256:e578a81bb873cbb89a41fcc904c7ef523cc18284b7e3b3ccf06aca1403b7ebd3", size = 18651, upload-time = "2025-10-08T17:44:47.223Z" }, +] + [[package]] name = "pluggy" version = "1.5.0" From cecc9f4b8c209833d88c3fdd56c436d089048601 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Thu, 13 Nov 2025 19:32:34 -0500 Subject: [PATCH 04/15] Update CHANGELOG.txt --- CHANGELOG.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index cd77c98a..8ccef4e0 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,5 @@ +- Change chrome download path to use XDG cache dir +- Don't download chrome if we already have that version: add force argument 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 From 4a839e7a1148c62f87b27fde00d419800d6a1c05 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Thu, 13 Nov 2025 19:35:59 -0500 Subject: [PATCH 05/15] Add is_dir check for testing's sake --- src/choreographer/cli/_cli_utils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index dd461725..3edec54e 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -174,7 +174,11 @@ def get_chrome_sync( # noqa: C901, PLR0912 return exe_path else: if exe_path.exists(): # delete it - shutil.rmtree(exe_path) + if exe_path.is_dir(): + shutil.rmtree(exe_path) + else: + exe_path.unlink() + # It really should always be a dir but in testing we fake it if version_tag.exists(): # delete it version_tag.unlink() From 64c4abf8a844c17482a43f90d3b067596c1c3279 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 12:03:16 -0500 Subject: [PATCH 06/15] Fix tests: remove MagicMock Claude used a MagicMock to imitate a file-like object, but you'd be building all that behavior from scratch. The original BytesIO object does that job without error. --- tests/test_get_chrome.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/test_get_chrome.py b/tests/test_get_chrome.py index 592f7837..1d9e138a 100644 --- a/tests/test_get_chrome.py +++ b/tests/test_get_chrome.py @@ -1,6 +1,6 @@ import io import zipfile -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest @@ -39,19 +39,16 @@ def test_get_chrome_sync_download_behavior( # Mock the URL download def create_mock_zip_response(): - mock_response = MagicMock() # Create a fresh zip buffer each time zip_buffer = io.BytesIO() with zipfile.ZipFile(zip_buffer, "w") as zf: zf.writestr("chrome-linux64/chrome", "newly downloaded chrome") zip_buffer.seek(0) - mock_response.read.return_value = zip_buffer.read() - mock_response.__enter__ = MagicMock(return_value=mock_response) - mock_response.__exit__ = MagicMock(return_value=False) - return mock_response + # Return BytesIO directly - it already has all file-like methods needed + return zip_buffer # Patch json.load to return our mock data (avoid broad Path.read_text patch) - with patch("json.load", return_value=mock_last_known_good_json), patch( + with patch("json.loads", return_value=mock_last_known_good_json), patch( "urllib.request.urlopen", side_effect=lambda url: create_mock_zip_response(), # noqa: ARG005 ): From 68ffcde2e52d51721304c76406a0ea8ddec04ff3 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 13:22:24 -0500 Subject: [PATCH 07/15] Change: get_chrome --verbose prints whole version --- src/choreographer/cli/_cli_utils.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index 3edec54e..b4459cf8 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -114,20 +114,23 @@ def get_chrome_sync( # noqa: C901, PLR0912 if i: _logger.info("Loading chrome from list") + raw_json = urllib.request.urlopen( # noqa: S310 audit url for schemes + _chrome_for_testing_url, + ).read() browser_list = json.loads( - urllib.request.urlopen( # noqa: S310 audit url for schemes - _chrome_for_testing_url, - ).read(), + raw_json, ) version_obj = browser_list["versions"][i] + raw_json = json.dumps(version_obj) else: _logger.info("Using last known good version of chrome") + raw_json = ( + Path(__file__).resolve().parent.parent + / "resources" + / "last_known_good_chrome.json" + ).read_text() version_obj = json.loads( - ( - Path(__file__).resolve().parent.parent - / "resources" - / "last_known_good_chrome.json" - ).read_text(), + raw_json, ) version_string = f"{version_obj['version']}\n{version_obj['revision']}" chromium_sources = version_obj["downloads"]["chrome"] @@ -144,7 +147,7 @@ def get_chrome_sync( # noqa: C901, PLR0912 ) if verbose: - print(version_string) # noqa: T201 allow print in cli + print(raw_json) # noqa: T201 allow print in cli version_tag = path / "version_tag.txt" path.mkdir(parents=True, exist_ok=True) From a81b5511727a4e2a759440398ac7a52447b316b2 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 13:22:38 -0500 Subject: [PATCH 08/15] Update last known good version --- .../resources/last_known_good_chrome.json | 73 +------------------ 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/src/choreographer/resources/last_known_good_chrome.json b/src/choreographer/resources/last_known_good_chrome.json index 878dfcc7..c99834a6 100644 --- a/src/choreographer/resources/last_known_good_chrome.json +++ b/src/choreographer/resources/last_known_good_chrome.json @@ -1,72 +1 @@ -{ - "version": "135.0.7011.0", - "revision": "1418433", - "downloads": { - "chrome": [ - { - "platform": "linux64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chrome-linux64.zip" - }, - { - "platform": "mac-arm64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chrome-mac-arm64.zip" - }, - { - "platform": "mac-x64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chrome-mac-x64.zip" - }, - { - "platform": "win32", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chrome-win32.zip" - }, - { - "platform": "win64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chrome-win64.zip" - } - ], - "chromedriver": [ - { - "platform": "linux64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chromedriver-linux64.zip" - }, - { - "platform": "mac-arm64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chromedriver-mac-arm64.zip" - }, - { - "platform": "mac-x64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chromedriver-mac-x64.zip" - }, - { - "platform": "win32", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chromedriver-win32.zip" - }, - { - "platform": "win64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chromedriver-win64.zip" - } - ], - "chrome-headless-shell": [ - { - "platform": "linux64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chrome-headless-shell-linux64.zip" - }, - { - "platform": "mac-arm64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chrome-headless-shell-mac-arm64.zip" - }, - { - "platform": "mac-x64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chrome-headless-shell-mac-x64.zip" - }, - { - "platform": "win32", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chrome-headless-shell-win32.zip" - }, - { - "platform": "win64", - "url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chrome-headless-shell-win64.zip" - } - ] - } -} +{"version": "144.0.7527.0", "revision": "1544685", "downloads": {"chrome": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chrome-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chrome-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chrome-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chrome-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chrome-win64.zip"}], "chromedriver": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chromedriver-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chromedriver-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chromedriver-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chromedriver-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chromedriver-win64.zip"}], "chrome-headless-shell": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chrome-headless-shell-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chrome-headless-shell-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chrome-headless-shell-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chrome-headless-shell-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chrome-headless-shell-win64.zip"}]}} From f29edc3afb8d2d17f3450432e9e5cffdc2f546dd Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:03:49 -0500 Subject: [PATCH 09/15] Fix testing and changelog --- CHANGELOG.txt | 2 ++ pyproject.toml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8ccef4e0..f232c585 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,5 @@ +- Update default chrome from 135.0.7011.0/1418433 to 144.0.7527.0/1544685 +- 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 v1.2.1 diff --git a/pyproject.toml b/pyproject.toml index d019ef4e..e3e3865d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,10 +124,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" From 91a7eb0e24fa4f40faeb75cfaa165969c2452dd7 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:42:55 -0500 Subject: [PATCH 10/15] Add backoff retry for looking for tabs --- src/choreographer/browser_async.py | 15 +++++++++++---- tests/test_process.py | 5 +++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/choreographer/browser_async.py b/src/choreographer/browser_async.py index a0900a38..610ffd9e 100644 --- a/src/choreographer/browser_async.py +++ b/src/choreographer/browser_async.py @@ -30,6 +30,8 @@ from .browsers._interface_type import BrowserImplInterface from .channels._interface_type import ChannelInterface +MAX_POPULATE_LOOPS = 3 # how many times to check for tab, race condition + _logger = logistro.getLogger(__name__) # Since I added locks to pipes, do we need locks here? @@ -162,9 +164,15 @@ 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.05 * counter) + counter += 1 + if counter == MAX_POPULATE_LOOPS: + break except (BrowserClosedError, BrowserFailedError, asyncio.CancelledError) as e: if ( hasattr(self._browser_impl, "missing_libs") @@ -350,7 +358,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" diff --git a/tests/test_process.py b/tests/test_process.py index dcee6fc8..0d9c2488 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -4,10 +4,11 @@ import signal import subprocess -import choreographer as choreo import logistro import pytest from async_timeout import timeout + +import choreographer as choreo from choreographer import errors # allows to create a browser pool for tests @@ -29,7 +30,7 @@ async def test_context(headless, sandbox, gpu): f"Sandbox: {sandbox}," f"Version: {platform.version().lower()}", ) - async with timeout(pytest.default_timeout): # type: ignore[reportAttributeAccessIssue] + async with timeout(100000): # type: ignore[reportAttributeAccessIssue] async with choreo.Browser( headless=headless, enable_sandbox=sandbox, From fa4a9ac1f756fbf7035fd3b73b4c21188cf9bbbf Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:48:50 -0500 Subject: [PATCH 11/15] Fix: duplicate code broke earlier changes --- src/choreographer/cli/_cli_utils.py | 8 +------- src/choreographer/cli/defaults.py | 9 ++++++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/choreographer/cli/_cli_utils.py b/src/choreographer/cli/_cli_utils.py index 3edec54e..95ba5be2 100644 --- a/src/choreographer/cli/_cli_utils.py +++ b/src/choreographer/cli/_cli_utils.py @@ -13,7 +13,6 @@ from pathlib import Path import logistro -from platformdirs import PlatformDirs from choreographer.cli.defaults import default_download_path @@ -49,12 +48,7 @@ def get_chrome_download_path() -> Path | None: if not _chrome_platform_detected: return None - _default_exe_path = ( - Path( - PlatformDirs("choreographer", "plotly").user_data_dir, - ) - / "deps" - ) + _default_exe_path = default_download_path _default_exe_path.mkdir(parents=True, exist_ok=True) if platform.system().startswith("Linux"): diff --git a/src/choreographer/cli/defaults.py b/src/choreographer/cli/defaults.py index aa119f72..a186db19 100644 --- a/src/choreographer/cli/defaults.py +++ b/src/choreographer/cli/defaults.py @@ -2,5 +2,12 @@ from pathlib import Path -default_download_path = Path(__file__).resolve().parent / "browser_exe" +from platformdirs import PlatformDirs + +default_download_path = ( + Path( + PlatformDirs("choreographer", "plotly").user_data_dir, + ) + / "deps" +) """The path where we download chrome if no path argument is supplied.""" From d979c8fe392ce75a590be5537100bcbda1f447c1 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:50:57 -0500 Subject: [PATCH 12/15] Update ROADMAP.md --- ROADMAP.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 081ec20a..e551e3c1 100644 --- a/ROADMAP.md +++ b/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 From 6a64aa17a2284602112994503b09f10715d545fe Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:51:34 -0500 Subject: [PATCH 13/15] Update CHANGELOG.txt --- CHANGELOG.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index f232c585..5d98d8ad 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,4 +1,6 @@ - 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 From c1c58ff802d8263df629ddcb95ba25270feb7527 Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 14:55:54 -0500 Subject: [PATCH 14/15] Increase number of retries on populate_targets --- src/choreographer/browser_async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/choreographer/browser_async.py b/src/choreographer/browser_async.py index 610ffd9e..2858307a 100644 --- a/src/choreographer/browser_async.py +++ b/src/choreographer/browser_async.py @@ -30,7 +30,7 @@ from .browsers._interface_type import BrowserImplInterface from .channels._interface_type import ChannelInterface -MAX_POPULATE_LOOPS = 3 # how many times to check for tab, race condition +MAX_POPULATE_LOOPS = 6 # how many times to check for tab, race condition _logger = logistro.getLogger(__name__) From df25f84185613f4e9b6c9e1fbc2966ab528790eb Mon Sep 17 00:00:00 2001 From: Andrew Pikul Date: Fri, 14 Nov 2025 15:39:05 -0500 Subject: [PATCH 15/15] Simplify retry scheme, windows is slow --- src/choreographer/browser_async.py | 5 +++-- tests/test_process.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/choreographer/browser_async.py b/src/choreographer/browser_async.py index 2858307a..3a6adfb1 100644 --- a/src/choreographer/browser_async.py +++ b/src/choreographer/browser_async.py @@ -30,7 +30,8 @@ from .browsers._interface_type import BrowserImplInterface from .channels._interface_type import ChannelInterface -MAX_POPULATE_LOOPS = 6 # how many times to check for tab, race condition +_N = MAX_POPULATE_LOOPS = 20 + _logger = logistro.getLogger(__name__) @@ -169,7 +170,7 @@ def run() -> subprocess.Popen[bytes] | subprocess.Popen[str]: # depends on args while not self.get_tab(): _logger.debug("Populating Targets") await self.populate_targets() - await asyncio.sleep(0.05 * counter) + await asyncio.sleep(0.1) counter += 1 if counter == MAX_POPULATE_LOOPS: break diff --git a/tests/test_process.py b/tests/test_process.py index 0d9c2488..8da9d276 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -30,7 +30,7 @@ async def test_context(headless, sandbox, gpu): f"Sandbox: {sandbox}," f"Version: {platform.version().lower()}", ) - async with timeout(100000): # type: ignore[reportAttributeAccessIssue] + async with timeout(pytest.default_timeout): # type: ignore[reportAttributeAccessIssue] async with choreo.Browser( headless=headless, enable_sandbox=sandbox,