test: stop the suite from writing the developer's real version cache - #604
Merged
Conversation
`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.
…n 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The incident
Running
make testbrokekbagentauto-update for whoever ran it. Observed on a dev machine today:There is no
v1.0.0release — no tag, no draft, nothing inpyproject.tomlhistory. The number came out of the test suite.Root cause
auto_update._get_cache_path()resolves throughplatformdirs, so it ignores--config-dirand always points at the machine's ownversion_cache.json. Two tests inTestMaybeAutoUpdatemock_read_cachebut not_write_cache, and the class's autouse_no_real_mcp_callsfixture forces the per-stage skip helpers toFalse— which also bypasses the_is_dev_install()gate that would otherwise stop the orchestrator in a dev tree. So both reach the real writer:test_version_comparison_none_no_updatelatest_version: "1.0.0"(the canonical fake latest)test_fetch_failure_continueslatest_version: <__version__>(theNonefallback)AUTO_UPDATE_CHECK_INTERVALis 1 hour, so for the next hour everykbagentcommand served1.0.0from cache, printed an update banner, and failed the reinstall against a tag that does not exist.kbagent versiondisagreed because it is excluded from the startup hook (_should_skip_all→_top_level_subcommand_is_versioning) andVersionServicecalls_fetch_kbagent_latest_version()directly, with no cache — a live GitHub fetch, correctly reporting0.84.2.The fix
tests/conftest.py: autouse_redirect_version_cachefixture repoints theauto_update._get_cache_pathmodule attribute attmp_path. Fixing it in conftest rather than in the two tests also covers tests written later that forget to mock the write.tests/test_auto_update.py:test_suite_never_resolves_to_the_real_user_cachefails if the fixture is removed. The file's top-level_get_cache_pathimport stays bound to the original function, soTestGetCachePath::test_returns_path_with_filenamestill exercises the real resolver.No
src/change:_get_cache_pathstaying platformdirs-based is correct for the shipped CLI — the cache is per-machine, not per-project.Verification
Written test-first (RED on the guard, GREEN after the fixture). Verified by positive evidence rather than a sentinel file, because a concurrent suite run on the same machine can write that file too — both writes now land inside
tmp_path:Full suite: 5748 passed, 166 skipped.
ruff check,ruff format --checkandty checkclean.Tests only — no version bump, no changelog entry.