From 7d1ac9d6f24289bf00b7b4a7cfd0a2748affb668 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 18 Aug 2026 09:23:47 -0400 Subject: [PATCH 1/2] test: stop the suite from writing the developer's real version cache `auto_update._get_cache_path` resolves through platformdirs, so it ignores --config-dir and always points at the machine's own `~/.config/keboola-agent-cli/version_cache.json`. Two tests in TestMaybeAutoUpdate mock `_read_cache` but not `_write_cache`, and the class's autouse fixture disables the dev-install skip gate, so they reach the real writer: test_version_comparison_none_no_update -> latest_version "1.0.0" test_fetch_failure_continues -> latest_version <__version__> Running `make test` therefore stamped the canonical fake latest version 1.0.0 into the developer's own cache. For the next hour (AUTO_UPDATE_CHECK_INTERVAL) every kbagent command then believed 1.0.0 was available, printed an update banner, and failed the reinstall against a release tag that does not exist -- while `kbagent version`, which bypasses the cache and refetches from GitHub, kept reporting "up to date". Adds an autouse `_redirect_version_cache` fixture pointing the module attribute at tmp_path, which also covers tests written later that forget to mock the write, plus a guard test that fails if the fixture is removed. The top-level import in test_auto_update.py stays bound to the original resolver, so TestGetCachePath still exercises the real path logic. --- tests/conftest.py | 26 ++++++++++++++++++++++++++ tests/test_auto_update.py | 24 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 8cc29485..aac9b02d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -62,6 +62,32 @@ def _head(url: str, *args: object, **kwargs: object) -> SimpleNamespace: monkeypatch.setattr(version_service.httpx, "head", _head) +@pytest.fixture(autouse=True) +def _redirect_version_cache(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """Keep the version cache out of the developer's real config directory. + + ``auto_update._get_cache_path`` resolves through ``platformdirs``, so it + ignores ``--config-dir`` and always points at the machine's own + ``version_cache.json``. Any test that reaches the real ``_write_cache`` + -- ``TestMaybeAutoUpdate`` has two that mock ``_read_cache`` but not the + write -- therefore stamps the canonical fake latest version ``1.0.0`` + into that file. The CLI then believes for a full + ``AUTO_UPDATE_CHECK_INTERVAL`` (1 hour) that a release tag which does not + exist is available, prints an update banner on every command, and fails + the reinstall: running ``make test`` broke auto-update for whoever ran it. + + Redirecting the module attribute (not the file's top-level import, which + ``TestGetCachePath`` still exercises against the real resolver) fixes it + for the whole suite, including tests written later that forget to mock + the write. ``test_suite_never_resolves_to_the_real_user_cache`` fails if + this fixture is removed. + """ + from keboola_agent_cli import auto_update + + cache_file = tmp_path / "version_cache.json" + monkeypatch.setattr(auto_update, "_get_cache_path", lambda: cache_file) + + @pytest.fixture def tmp_config_dir(tmp_path: Path) -> Path: """Provide a temporary directory for configuration files.""" diff --git a/tests/test_auto_update.py b/tests/test_auto_update.py index b8abb56d..06f2664a 100644 --- a/tests/test_auto_update.py +++ b/tests/test_auto_update.py @@ -682,6 +682,30 @@ def test_returns_path_with_filename(self): assert path.name == "version_cache.json" assert "keboola-agent-cli" in str(path) + def test_suite_never_resolves_to_the_real_user_cache(self): + """Guard for the `_redirect_version_cache` autouse fixture. + + `_get_cache_path` resolves through `platformdirs`, so it is NOT + `--config-dir`-aware and points at the developer's own + `version_cache.json`. Two tests in `TestMaybeAutoUpdate` reach the + real `_write_cache` (they mock `_read_cache`, not the write), which + stamped the canonical fake latest version `1.0.0` into that file -- + after which every `kbagent` command on the machine spent an hour + (`AUTO_UPDATE_CHECK_INTERVAL`) trying to install a release tag that + does not exist. The conftest fixture redirects the module attribute + per test; this asserts it is actually in force, so deleting the + fixture fails here instead of silently on someone's laptop. + + Uses the module attribute, not the name imported at the top of this + file -- the latter is bound to the original function object and is + deliberately left unpatched for the test above. + """ + import platformdirs + + real_dir = Path(platformdirs.user_config_dir("keboola-agent-cli")).resolve() + resolved = auto_update_module._get_cache_path().resolve() + assert real_dir != resolved.parent + # --------------------------------------------------------------------------- # `kbagent changelog` does not duplicate "What's new" output From df22ac543e8cbaa3dec66f3cb34a6aee3e654ecc Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 18 Aug 2026 10:18:46 -0400 Subject: [PATCH 2/2] test: redirect the deferred-update state dir too, not just the version cache Review follow-up: `update_runner.state_dir()` resolves through the same `platformdirs.user_config_dir("keboola-agent-cli")` as the version cache and backs the deferred-update marker / exit / log files, but only `tests/test_update_runner.py` patched it, file-scoped. Verified latent rather than live: a full suite run creates no files in the real config directory, because the deferral tests mock `request_deferred_update` and `should_defer()` is False off Windows. It is the same class of bug though -- on Windows `should_defer()` defaults to True, and `report_finished_deferred_update` runs ahead of every skip gate and unlinks a genuine pending report as it reads it, so a developer's own update outcome would disappear into a test run. Extends the autouse fixture (renamed `_redirect_global_state_paths`) and its guard test to cover both writers. --- tests/conftest.py | 41 ++++++++++++++++++++++++++------------- tests/test_auto_update.py | 14 ++++++++++--- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index aac9b02d..ec546903 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -63,29 +63,42 @@ def _head(url: str, *args: object, **kwargs: object) -> SimpleNamespace: @pytest.fixture(autouse=True) -def _redirect_version_cache(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: - """Keep the version cache out of the developer's real config directory. - - ``auto_update._get_cache_path`` resolves through ``platformdirs``, so it - ignores ``--config-dir`` and always points at the machine's own - ``version_cache.json``. Any test that reaches the real ``_write_cache`` - -- ``TestMaybeAutoUpdate`` has two that mock ``_read_cache`` but not the - write -- therefore stamps the canonical fake latest version ``1.0.0`` - into that file. The CLI then believes for a full +def _redirect_global_state_paths(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """Keep auto-update bookkeeping out of the developer's real config directory. + + Both writers below resolve through ``platformdirs.user_config_dir``, so + neither honours ``--config-dir``: they always address the machine's own + files. That is correct for the shipped CLI -- the version cache and a + pending update are per-machine, not per-project -- and exactly why the + test suite has to redirect them rather than the production code. + + ``auto_update._get_cache_path`` backs ``version_cache.json``. Any test + reaching the real ``_write_cache`` -- ``TestMaybeAutoUpdate`` has two that + mock ``_read_cache`` but not the write -- stamps the canonical fake latest + version ``1.0.0`` into it. The CLI then believes for a full ``AUTO_UPDATE_CHECK_INTERVAL`` (1 hour) that a release tag which does not exist is available, prints an update banner on every command, and fails the reinstall: running ``make test`` broke auto-update for whoever ran it. - Redirecting the module attribute (not the file's top-level import, which - ``TestGetCachePath`` still exercises against the real resolver) fixes it - for the whole suite, including tests written later that forget to mock - the write. ``test_suite_never_resolves_to_the_real_user_cache`` fails if + ``update_runner.state_dir`` backs the deferred-update marker / exit / log + files. No test reaches it today (the deferral tests mock + ``request_deferred_update``, and ``should_defer()`` is False off Windows), + but it is the same latent bug: on Windows ``should_defer()`` defaults to + True, and ``report_finished_deferred_update`` -- which runs ahead of every + skip gate -- consumes and unlinks a genuine pending report while reading + it, so a developer's own update outcome would vanish into a test run. + + Redirecting the module attributes (not the top-level import in + ``test_auto_update.py``, which ``TestGetCachePath`` still exercises + against the real resolver) covers tests written later that forget to mock + the writer. ``test_suite_never_resolves_to_the_real_user_cache`` fails if this fixture is removed. """ - from keboola_agent_cli import auto_update + from keboola_agent_cli import auto_update, update_runner cache_file = tmp_path / "version_cache.json" monkeypatch.setattr(auto_update, "_get_cache_path", lambda: cache_file) + monkeypatch.setattr(update_runner, "state_dir", lambda: tmp_path) @pytest.fixture diff --git a/tests/test_auto_update.py b/tests/test_auto_update.py index 06f2664a..8e265504 100644 --- a/tests/test_auto_update.py +++ b/tests/test_auto_update.py @@ -683,7 +683,7 @@ def test_returns_path_with_filename(self): assert "keboola-agent-cli" in str(path) def test_suite_never_resolves_to_the_real_user_cache(self): - """Guard for the `_redirect_version_cache` autouse fixture. + """Guard for the `_redirect_global_state_paths` autouse fixture. `_get_cache_path` resolves through `platformdirs`, so it is NOT `--config-dir`-aware and points at the developer's own @@ -702,9 +702,17 @@ def test_suite_never_resolves_to_the_real_user_cache(self): """ import platformdirs + from keboola_agent_cli import update_runner + real_dir = Path(platformdirs.user_config_dir("keboola-agent-cli")).resolve() - resolved = auto_update_module._get_cache_path().resolve() - assert real_dir != resolved.parent + assert real_dir != auto_update_module._get_cache_path().resolve().parent + # `update_runner.state_dir` resolves through the very same call and backs + # the deferred-update marker / exit / log files. `should_defer()` is True + # by default on Windows, so a test reaching the scheduler there would + # write real state -- and `report_finished_deferred_update`, which runs + # before every skip gate, unlinks a genuine pending report while reading + # it. Same class of bug, so the fixture covers both. + assert real_dir != update_runner.state_dir().resolve() # ---------------------------------------------------------------------------