From 13326080a5160b2605f3b92dd0301e07f2d81bec Mon Sep 17 00:00:00 2001
From: Andrew Pikul <ajpikul@gmail.com>
Date: Thu, 13 Nov 2025 19:14:14 -0500
Subject: [PATCH 1/7] 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 <ajpikul@gmail.com>
Date: Thu, 13 Nov 2025 19:22:27 -0500
Subject: [PATCH 2/7] 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 <ajpikul@gmail.com>
Date: Thu, 13 Nov 2025 19:31:43 -0500
Subject: [PATCH 3/7] 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 <ajpikul@gmail.com>
Date: Thu, 13 Nov 2025 19:32:34 -0500
Subject: [PATCH 4/7] 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 <ajpikul@gmail.com>
Date: Thu, 13 Nov 2025 19:35:59 -0500
Subject: [PATCH 5/7] 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 <ajpikul@gmail.com>
Date: Fri, 14 Nov 2025 12:03:16 -0500
Subject: [PATCH 6/7] 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 fa4a9ac1f756fbf7035fd3b73b4c21188cf9bbbf Mon Sep 17 00:00:00 2001
From: Andrew Pikul <ajpikul@gmail.com>
Date: Fri, 14 Nov 2025 14:48:50 -0500
Subject: [PATCH 7/7] 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."""