Replace Click's deprecated isolated_filesystem in the CLI tests - #68
Merged
Conversation
Click 8.5.0 deprecates CliRunner.isolated_filesystem and removes it in 9.0, so every CLI test that used it would break on that upgrade. The commands fall into two groups. archive takes the agent directory as an argument and finds the project by walking up from it, so its tests need no working directory at all and now pass absolute paths only. validate, terraform, register, init, connections and graph read the project from the working directory by design, so their tests still have to run from inside a tree; monkeypatch.chdir moves there and pytest puts it back. Two fixtures carry the distinction the old calls made through their arguments: below_project for the walk up to gete.yaml, outside_any_project for the commands that have to answer before a project exists. The second asserts that no gete.yaml sits above the temporary directory, because one that did would let those tests pass without proving anything. 🤖 Generated with Claude Code
Merged
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
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.
Click 8.5.0 deprecates
CliRunner.isolated_filesystemand removes it in 9.0. The CLI tests used it in 19 places, so on 8.5.0 the suite still passes but emits 23DeprecationWarnings, and on 9.0 it stops working. This replaces it before that becomes urgent.What changes
The commands fall into two groups, and the replacement differs for each.
archiveneeds no working directory at all. It takes the agent directory as an argument and finds the project by walking up from that, so its tests now pass absolute paths and do not move anywhere.test_plain_mode_still_requires_the_directoryloses itsprojectfixture with it, since nothing in that test needs a project on disk any more.validate,terraform,register,init,connectionsandgraphread the project from the working directory by design. Their tests still have to run from inside a tree, somonkeypatch.chdirmoves there and pytest restores it afterwards. Two fixtures carry the distinction the old calls made through their arguments:below_project— a directory under the project root, for the walk up togete.yaml.outside_any_project— a directory with nothing above it, for the commands that have to answer before a project exists.outside_any_projectalso asserts that nogete.yamlsits above the temporary directory. Without that, a stray one would make every "there is no project" test pass without proving anything.Verification
ruff check,ruff format --check,mypyand the full suite pass; the collected test count is unchanged at 878.click==8.5.0: 878 passed, and the 23 deprecation warnings are gone.chdirfrombelow_projectfails 12 tests, and removing it fromoutside_any_projectfails 2. Neither fixture is decoration.What this does not do
The remaining
chdircalls could be removed only by giving the CLI a way to name the project explicitly, such as a--projector-Coption. That is a change to the command's interface rather than to its tests, so it is left out here.