From 40a3b0fb7b4a22f11d1ec529409b7dc8501e0e0a Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:35:16 +0900 Subject: [PATCH] Set the working directory from pytest, not from Click's test runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- tests/conftest.py | 31 +++++++++++++++++ tests/test_cli.py | 46 +++++++++++--------------- tests/test_cli_archive.py | 55 +++++++++++++++---------------- tests/test_cli_terraform.py | 27 +++++++-------- tests/test_connections_command.py | 31 ++++++++--------- tests/test_graph.py | 6 ++-- tests/test_init.py | 22 ++++++------- 7 files changed, 121 insertions(+), 97 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 580856e..24300ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -61,6 +61,37 @@ def project(tmp_path: Path) -> ProjectBuilder: return ProjectBuilder(tmp_path) +@pytest.fixture +def below_project(project: ProjectBuilder, monkeypatch: pytest.MonkeyPatch) -> Path: + """A working directory inside the project, so discovery has to walk up. + + The commands that read a project take it from the working directory. A test + about such a command, rather than about the loader it calls, therefore has + to run from somewhere in the tree instead of at its root. + """ + directory = project.root / "somewhere" + directory.mkdir() + monkeypatch.chdir(directory) + return directory + + +@pytest.fixture +def outside_any_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """A working directory with no gete.yaml above it. + + For the commands that have to answer before there is a project at all. + """ + directory = tmp_path / "elsewhere" + directory.mkdir() + # A gete.yaml anywhere above would be found by the walk up, and every test + # that means "there is no project" would pass without proving anything. + assert not any( + (parent / "gete.yaml").exists() for parent in (directory, *directory.parents) + ) + monkeypatch.chdir(directory) + return directory + + class FakeGcp: """Answers from a table of (method, url) and records every write.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 7977ae1..05281db 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -11,16 +11,15 @@ from gete.errors import DeclarationError +@pytest.mark.usefixtures("below_project") def test_validate_exits_zero_when_everything_is_fine(project: ProjectBuilder) -> None: project.write_agent("mail-triage") - runner = CliRunner() - # A directory below the project root; validate walks up to gete.yaml. - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["validate"]) + result = CliRunner().invoke(main, ["validate"]) assert result.exit_code == 0, result.output assert "1 agent" in result.output +@pytest.mark.usefixtures("below_project") def test_validate_lists_every_problem_and_exits_one(project: ProjectBuilder) -> None: project.write_agent( "mail-triage", @@ -29,22 +28,20 @@ def test_validate_lists_every_problem_and_exits_one(project: ProjectBuilder) -> "runtime": {"agent_engine": {"env": {"GOOGLE_CLOUD_PROJECT": "x"}}}, }, ) - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["validate"]) + result = CliRunner().invoke(main, ["validate"]) assert result.exit_code == 1 assert "salesforce" in result.output assert "GOOGLE_CLOUD_PROJECT" in result.output +@pytest.mark.usefixtures("outside_any_project") def test_validate_reports_a_missing_project_file() -> None: - runner = CliRunner() - with runner.isolated_filesystem(): - result = runner.invoke(main, ["validate"]) + result = CliRunner().invoke(main, ["validate"]) assert result.exit_code == 1 assert "gete.yaml" in result.output +@pytest.mark.usefixtures("below_project") def test_register_passes_the_authorizations_to_reset_through( project: ProjectBuilder, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -63,19 +60,17 @@ def fake_register( monkeypatch.setattr("gete.cli.register_project", fake_register) monkeypatch.setattr("gete.gcp.GcpClient", lambda quota_project: object()) project.write_agent("finance", {"connections": ["freee"]}) - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke( - main, - [ - "register", - "finance", - "--reset-authorization", - "finance-freee", - "--reset-authorization", - "finance-github", - ], - ) + result = CliRunner().invoke( + main, + [ + "register", + "finance", + "--reset-authorization", + "finance-freee", + "--reset-authorization", + "finance-github", + ], + ) assert result.exit_code == 0, result.output assert seen == {"names": ["finance"], "reset": ["finance-freee", "finance-github"]} @@ -86,6 +81,7 @@ def test_version_is_shown() -> None: assert result.output.startswith("gete, version ") +@pytest.mark.usefixtures("below_project") def test_a_failure_inside_the_import_check_is_a_message_not_a_traceback( project: ProjectBuilder, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -94,9 +90,7 @@ def boom(directory: Path, **kwargs: Any) -> None: monkeypatch.setattr("gete.importcheck.import_check", boom) project.write_agent("mail-triage") - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["validate", "--import-check"]) + result = CliRunner().invoke(main, ["validate", "--import-check"]) assert result.exit_code == 1 assert "requirements.txt cannot be read" in result.output assert not isinstance(result.exception, DeclarationError) diff --git a/tests/test_cli_archive.py b/tests/test_cli_archive.py index 5c673e3..0721f81 100644 --- a/tests/test_cli_archive.py +++ b/tests/test_cli_archive.py @@ -1,4 +1,9 @@ -"""gete archive from the command line.""" +"""gete archive from the command line. + +archive takes the agent directory as an argument and finds the project by +walking up from it, so these tests pass absolute paths and never have to move +the working directory. +""" import hashlib from pathlib import Path @@ -14,12 +19,10 @@ def test_archive_writes_the_file_and_prints_the_hash( ) -> None: project.write_agent("mail-triage") out = tmp_path / "out" / "mail-triage.tar.gz" - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke( - main, - ["archive", str(project.agents_dir / "mail-triage"), "--out", str(out)], - ) + result = CliRunner().invoke( + main, + ["archive", str(project.agents_dir / "mail-triage"), "--out", str(out)], + ) assert result.exit_code == 0, result.output assert hashlib.sha256(out.read_bytes()).hexdigest() in result.output @@ -28,17 +31,15 @@ def test_archive_refuses_an_agent_that_does_not_validate( project: ProjectBuilder, tmp_path: Path ) -> None: project.write_agent("mail-triage", {"connections": ["nope"]}) - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke( - main, - [ - "archive", - str(project.agents_dir / "mail-triage"), - "--out", - str(tmp_path / "a.tgz"), - ], - ) + result = CliRunner().invoke( + main, + [ + "archive", + str(project.agents_dir / "mail-triage"), + "--out", + str(tmp_path / "a.tgz"), + ], + ) assert result.exit_code == 1 assert "nope" in result.output @@ -48,21 +49,17 @@ def test_external_mode_needs_no_directory_argument(project: ProjectBuilder) -> N import json directory = project.write_agent("mail-triage") - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke( - main, - ["archive", "--external"], - input=json.dumps({"directory": str(directory)}), - ) + result = CliRunner().invoke( + main, + ["archive", "--external"], + input=json.dumps({"directory": str(directory)}), + ) assert result.exit_code == 0, result.output payload = json.loads(result.output) assert set(payload) == {"archive", "sha256"} -def test_plain_mode_still_requires_the_directory(project: ProjectBuilder) -> None: - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["archive"]) +def test_plain_mode_still_requires_the_directory() -> None: + result = CliRunner().invoke(main, ["archive"]) assert result.exit_code != 0 assert "DIRECTORY" in result.output diff --git a/tests/test_cli_terraform.py b/tests/test_cli_terraform.py index eea1fb7..c80101a 100644 --- a/tests/test_cli_terraform.py +++ b/tests/test_cli_terraform.py @@ -1,32 +1,33 @@ """gete terraform from the command line.""" +import pytest from click.testing import CliRunner from conftest import ProjectBuilder from gete.cli import main +@pytest.mark.usefixtures("below_project") def test_terraform_writes_files_then_check_passes(project: ProjectBuilder) -> None: project.write_agent("mail-triage") runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - written = runner.invoke(main, ["terraform", "--out", str(project.root / "tf")]) - assert written.exit_code == 0, written.output - assert (project.root / "tf" / "mail_triage.tf").is_file() - checked = runner.invoke( - main, ["terraform", "--out", str(project.root / "tf"), "--check"] - ) + written = runner.invoke(main, ["terraform", "--out", str(project.root / "tf")]) + assert written.exit_code == 0, written.output + assert (project.root / "tf" / "mail_triage.tf").is_file() + checked = runner.invoke( + main, ["terraform", "--out", str(project.root / "tf"), "--check"] + ) assert checked.exit_code == 0, checked.output +@pytest.mark.usefixtures("below_project") def test_check_exits_one_and_names_the_stale_file(project: ProjectBuilder) -> None: project.write_agent("mail-triage") runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - runner.invoke(main, ["terraform", "--out", str(project.root / "tf")]) - project.write_agent("mail-triage", {"display_name": "Renamed"}) - checked = runner.invoke( - main, ["terraform", "--out", str(project.root / "tf"), "--check"] - ) + runner.invoke(main, ["terraform", "--out", str(project.root / "tf")]) + project.write_agent("mail-triage", {"display_name": "Renamed"}) + checked = runner.invoke( + main, ["terraform", "--out", str(project.root / "tf"), "--check"] + ) assert checked.exit_code == 1 assert "mail_triage.tf" in checked.output diff --git a/tests/test_connections_command.py b/tests/test_connections_command.py index 2a6cd05..b16dc33 100644 --- a/tests/test_connections_command.py +++ b/tests/test_connections_command.py @@ -2,6 +2,7 @@ from typing import Any +import pytest from click.testing import CliRunner from conftest import ProjectBuilder @@ -60,22 +61,20 @@ def test_the_description_shows_the_menu_next_to_the_default_scopes() -> None: assert "optional scopes" in output +@pytest.mark.usefixtures("below_project") def test_cli_prints_one_line_per_connection(project: ProjectBuilder) -> None: write_connections(project) - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["connections"]) + result = CliRunner().invoke(main, ["connections"]) assert result.exit_code == 0, result.output lines = [line for line in result.output.splitlines() if line.strip()] assert any(line.startswith("freee") for line in lines) assert any("retired" in line and line.startswith("old-api") for line in lines) +@pytest.mark.usefixtures("outside_any_project") def test_cli_connections_works_without_a_project() -> None: """The catalog is worth reading before there is a gete.yaml.""" - runner = CliRunner() - with runner.isolated_filesystem(): - result = runner.invoke(main, ["connections"]) + result = CliRunner().invoke(main, ["connections"]) assert result.exit_code == 0, result.output assert "freee" in result.output @@ -126,12 +125,12 @@ def write_connections(project: ProjectBuilder) -> None: def describe(project: ProjectBuilder, connection_id: str) -> Any: + """Callers take below_project: connections reads the overrides from there.""" write_connections(project) - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - return runner.invoke(main, ["connections", connection_id]) + return CliRunner().invoke(main, ["connections", connection_id]) +@pytest.mark.usefixtures("below_project") def test_describing_a_connection_prints_what_a_person_has_to_do_first( project: ProjectBuilder, ) -> None: @@ -141,6 +140,7 @@ def test_describing_a_connection_prints_what_a_person_has_to_do_first( assert line in result.output +@pytest.mark.usefixtures("below_project") def test_the_description_names_the_secrets_and_the_redirect_uri( project: ProjectBuilder, ) -> None: @@ -151,6 +151,7 @@ def test_the_description_names_the_secrets_and_the_redirect_uri( assert "https://vertexaisearch.cloud.google.com/oauth-redirect" in output +@pytest.mark.usefixtures("below_project") def test_a_connection_without_setup_notes_is_still_described( project: ProjectBuilder, ) -> None: @@ -160,6 +161,7 @@ def test_a_connection_without_setup_notes_is_still_described( assert "https://accounts.secure.freee.co.jp/public_api/token" in result.output +@pytest.mark.usefixtures("below_project") def test_describing_an_unknown_connection_names_the_known_ones( project: ProjectBuilder, ) -> None: @@ -168,14 +170,14 @@ def test_describing_an_unknown_connection_names_the_known_ones( assert "freee" in result.output +@pytest.mark.usefixtures("outside_any_project") def test_a_catalog_connection_can_be_described_without_a_project() -> None: - runner = CliRunner() - with runner.isolated_filesystem(): - result = runner.invoke(main, ["connections", "github"]) + result = CliRunner().invoke(main, ["connections", "github"]) assert result.exit_code == 0, result.output assert "api.github.com" in result.output +@pytest.mark.usefixtures("below_project") def test_a_retired_connection_reads_retired_with_the_reason_alongside( project: ProjectBuilder, ) -> None: @@ -186,11 +188,10 @@ def test_a_retired_connection_reads_retired_with_the_reason_alongside( assert "native connector" in result.output +@pytest.mark.usefixtures("outside_any_project") def test_describing_slack_mcp_prints_the_app_setup_without_a_project() -> None: """The Slack app is prepared by a person; the description has to carry it.""" - runner = CliRunner() - with runner.isolated_filesystem(): - result = runner.invoke(main, ["connections", "slack-mcp"]) + result = CliRunner().invoke(main, ["connections", "slack-mcp"]) assert result.exit_code == 0, result.output assert "mcp.slack.com" in result.output assert "Before anyone can authorize:" in result.output diff --git a/tests/test_graph.py b/tests/test_graph.py index 8e9ea0e..acefd69 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -2,6 +2,7 @@ from typing import Any +import pytest from click.testing import CliRunner from conftest import ProjectBuilder @@ -70,12 +71,11 @@ def test_node_ids_are_safe_mermaid_identifiers(project: ProjectBuilder) -> None: assert head.replace("_", "").isalnum(), line +@pytest.mark.usefixtures("below_project") def test_cli_prints_mermaid(project: ProjectBuilder) -> None: project.write_agent("finance", FINANCE) (project.agents_dir / "finance" / "src").mkdir() - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["graph", "finance"]) + result = CliRunner().invoke(main, ["graph", "finance"]) assert result.exit_code == 0, result.output assert result.output.startswith("flowchart LR") diff --git a/tests/test_init.py b/tests/test_init.py index 62f647b..a49ea8b 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -2,6 +2,7 @@ from pathlib import Path +import pytest from click.testing import CliRunner from conftest import ProjectBuilder @@ -62,21 +63,20 @@ def test_init_agent_refuses_a_name_that_is_not_a_label(tmp_path: Path) -> None: init_agent(tmp_path, "Mail_Triage") -def test_cli_init_creates_the_project_when_there_is_none(tmp_path: Path) -> None: - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=tmp_path) as cwd: - result = runner.invoke(main, ["init", "mail-triage"]) - assert result.exit_code == 0, result.output - assert (Path(cwd) / "gete.yaml").is_file() - assert (Path(cwd) / "agents" / "mail-triage" / "agent.yaml").is_file() - assert "gete.yaml" in result.output +def test_cli_init_creates_the_project_when_there_is_none( + outside_any_project: Path, +) -> None: + result = CliRunner().invoke(main, ["init", "mail-triage"]) + assert result.exit_code == 0, result.output + assert (outside_any_project / "gete.yaml").is_file() + assert (outside_any_project / "agents" / "mail-triage" / "agent.yaml").is_file() + assert "gete.yaml" in result.output +@pytest.mark.usefixtures("below_project") def test_cli_init_adds_to_an_existing_project_from_anywhere_below_it( project: ProjectBuilder, ) -> None: - runner = CliRunner() - with runner.isolated_filesystem(temp_dir=project.root): - result = runner.invoke(main, ["init", "new-agent"]) + result = CliRunner().invoke(main, ["init", "new-agent"]) assert result.exit_code == 0, result.output assert (project.root / "agents" / "new-agent" / "agent.yaml").is_file()