From 2fe47b2bbf3e9f9cdf907ae94cbefd900f5861b8 Mon Sep 17 00:00:00 2001 From: Dave Wilding Date: Tue, 27 Oct 2026 05:55:36 -0500 Subject: [PATCH 1/4] test: add unit tests for GitHub token handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/functional/helpers_functional.py | 2 +- tests/functional/test_token.py | 8 +- tests/unit/test_context_no_token.py | 5 + tests/unit/test_token.py | 235 +++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_token.py diff --git a/tests/functional/helpers_functional.py b/tests/functional/helpers_functional.py index 258f1a8..c12f69f 100644 --- a/tests/functional/helpers_functional.py +++ b/tests/functional/helpers_functional.py @@ -5,7 +5,7 @@ no_ssh = ["--ssh", "never"] -no_token = types.SimpleNamespace( +token_unset = types.SimpleNamespace( condition="GITHUB_TOKEN" not in os.environ, reason="GITHUB_TOKEN is not set", ) diff --git a/tests/functional/test_token.py b/tests/functional/test_token.py index 04aa8f9..7f3f79e 100644 --- a/tests/functional/test_token.py +++ b/tests/functional/test_token.py @@ -4,7 +4,7 @@ import pytest -@pytest.mark.skipif(helpers.no_token.condition, reason=helpers.no_token.reason) +@pytest.mark.skipif(helpers.token_unset.condition, reason=helpers.token_unset.reason) def test_forked_repo_token(uv_run, test_dir): command = [ *uv_run, @@ -41,7 +41,7 @@ def test_forked_repo_token(uv_run, test_dir): assert not helpers.get_tags(expected_dir) -@pytest.mark.skipif(helpers.no_token.condition, reason=helpers.no_token.reason) +@pytest.mark.skipif(helpers.token_unset.condition, reason=helpers.token_unset.reason) def test_invalid_repo_token(uv_run, test_dir): command = [ *uv_run, @@ -69,7 +69,7 @@ def test_invalid_repo_token(uv_run, test_dir): assert not (test_dir / "invalid").exists() -@pytest.mark.skipif(helpers.no_token.condition, reason=helpers.no_token.reason) +@pytest.mark.skipif(helpers.token_unset.condition, reason=helpers.token_unset.reason) def test_u_sets_upstream_owner(uv_run, test_dir): command = [ *uv_run, @@ -100,7 +100,7 @@ def test_u_sets_upstream_owner(uv_run, test_dir): assert result.stdout == expected_stdout -@pytest.mark.skipif(helpers.no_token.condition, reason=helpers.no_token.reason) +@pytest.mark.skipif(helpers.token_unset.condition, reason=helpers.token_unset.reason) def test_b_sets_upstream_owner(uv_run, test_dir): command = [ *uv_run, diff --git a/tests/unit/test_context_no_token.py b/tests/unit/test_context_no_token.py index c29a270..3242bd1 100644 --- a/tests/unit/test_context_no_token.py +++ b/tests/unit/test_context_no_token.py @@ -7,6 +7,11 @@ from gimmegit import _cli +@pytest.fixture(autouse=True) +def no_token(monkeypatch): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", None) + + @pytest.fixture(autouse=True) def no_ssh(monkeypatch): monkeypatch.setattr("gimmegit._remote.is_ssh_configured", lambda: False) diff --git a/tests/unit/test_token.py b/tests/unit/test_token.py new file mode 100644 index 0000000..529071d --- /dev/null +++ b/tests/unit/test_token.py @@ -0,0 +1,235 @@ +import argparse +from dataclasses import dataclass, field +from pathlib import Path +from types import SimpleNamespace + +import github +import pytest + +from gimmegit import _cli, _remote + +FORKED_REPO = SimpleNamespace( + fork=True, + parent=SimpleNamespace( + owner=SimpleNamespace(login="canonical"), + name="jubilant", + ), +) + + +@pytest.fixture(autouse=True) +def no_ssh(monkeypatch): + monkeypatch.setattr("gimmegit._remote.is_ssh_configured", lambda: False) + + +@pytest.fixture(autouse=True) +def snapshot_name(monkeypatch): + monkeypatch.setattr("gimmegit._cli.make_snapshot_name", lambda: "snapshot0801") + + +@dataclass +class FakeGithubState: + repo: object | None = None + user: object | None = None + missing: bool = False # Whether to raise UnknownObjectException for a missing repo. + denied: bool = False # Whether to raise GithubException for a rejected token. + clients: list = field(default_factory=list) + repo_calls: list = field(default_factory=list) + + +@pytest.fixture +def fake_github(monkeypatch): + """Stand-in for github.Github. Set state.repo or state.user before calling the function under test.""" + + state = FakeGithubState() + + class FakeGithub: + def __init__(self, login_or_token=None, **kwargs): + self.token = login_or_token + state.clients.append(self) + + def get_repo(self, owner_project): + state.repo_calls.append(owner_project) + if state.missing: + raise github.UnknownObjectException(404, None) + if state.denied: + raise github.GithubException(401, "Bad credentials") + return state.repo + + def get_user(self): + return state.user + + monkeypatch.setattr(github, "Github", FakeGithub) + return state + + +@pytest.mark.parametrize("token", [None, ""]) +def test_upstream_no_token(monkeypatch, fake_github, token): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", token) + assert _cli.get_github_upstream("dwilding", "forked-jubilant") is None + assert fake_github.clients == [] + + +@pytest.mark.parametrize("token", [None, ""]) +def test_github_url_no_token(monkeypatch, fake_github, token): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", token) + with pytest.raises(ValueError) as exc_info: + _cli.make_github_url("frogtab") + assert str(exc_info.value) == ( + "GIMMEGIT_GITHUB_TOKEN is not set. For the repo, use '/' or a GitHub URL." + ) + assert fake_github.clients == [] + + +def test_upstream_fork(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.repo = FORKED_REPO + remote = _cli.get_github_upstream("dwilding", "forked-jubilant") + assert remote == _remote.Remote( + owner="canonical", + project="jubilant", + url="https://github.com/canonical/jubilant.git", + ) + assert fake_github.repo_calls == ["dwilding/forked-jubilant"] + assert fake_github.clients[0].token == "fake-token" + + +def test_upstream_fork_ssh(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + monkeypatch.setattr(_cli, "SSH", True) + fake_github.repo = FORKED_REPO + remote = _cli.get_github_upstream("dwilding", "forked-jubilant") + assert remote == _remote.Remote( + owner="canonical", + project="jubilant", + url="git@github.com:canonical/jubilant.git", + ) + + +def test_upstream_not_fork(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.repo = SimpleNamespace(fork=False) + assert _cli.get_github_upstream("dwilding", "forked-jubilant") is None + + +def test_upstream_missing(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.missing = True + with pytest.raises(ValueError) as exc_info: + _cli.get_github_upstream("dwilding", "invalid") + assert ( + str(exc_info.value) + == "Unable to find 'dwilding/invalid' on GitHub. Do you have access to the repo?" + ) + + +def test_upstream_denied(monkeypatch, fake_github): + # GitHub rejects an invalid token with GithubException, which get_github_upstream does not catch. + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "bad-token") + fake_github.denied = True + with pytest.raises(github.GithubException): + _cli.get_github_upstream("dwilding", "frogtab") + + +@pytest.mark.parametrize("repo", ["frogtab", "frogtab/"]) +def test_github_url_project(monkeypatch, fake_github, repo): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.user = SimpleNamespace(login="dwilding") + assert _cli.make_github_url(repo) == "https://github.com/dwilding/frogtab" + assert fake_github.clients[0].token == "fake-token" + + +def test_context_fork(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.repo = FORKED_REPO + args = argparse.Namespace( + base_branch=None, + upstream_owner=None, + repo="dwilding/forked-jubilant", + new_branch=None, + ) + expected_context = _cli.Context( + base_branch=None, + branch="snapshot0801", + clone_url="https://github.com/dwilding/forked-jubilant.git", + clone_dir=Path("jubilant/dwilding-snapshot0801"), + create_branch=True, + owner="dwilding", + project="jubilant", + upstream_owner="canonical", + upstream_url="https://github.com/canonical/jubilant.git", + ) + assert _cli.get_context(args) == expected_context + + +def test_context_fork_u_overrides_upstream(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.repo = FORKED_REPO + args = argparse.Namespace( + base_branch=None, + upstream_owner="some-org", + repo="dwilding/forked-jubilant", + new_branch=None, + ) + expected_context = _cli.Context( + base_branch=None, + branch="snapshot0801", + clone_url="https://github.com/dwilding/forked-jubilant.git", + clone_dir=Path("forked-jubilant/dwilding-snapshot0801"), + create_branch=True, + owner="dwilding", + project="forked-jubilant", + upstream_owner="some-org", + upstream_url="https://github.com/some-org/forked-jubilant.git", + ) + assert _cli.get_context(args) == expected_context + # get_context still queries the fork on GitHub. + assert fake_github.repo_calls == ["dwilding/forked-jubilant"] + + +def test_context_fork_b_overrides_upstream(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.repo = FORKED_REPO + args = argparse.Namespace( + base_branch="https://github.com/some-org/frogtab/tree/maintenance", + upstream_owner=None, + repo="dwilding/forked-jubilant", + new_branch=None, + ) + expected_context = _cli.Context( + base_branch="maintenance", + branch="snapshot0801", + clone_url="https://github.com/dwilding/forked-jubilant.git", + clone_dir=Path("frogtab/dwilding-snapshot0801"), + create_branch=True, + owner="dwilding", + project="frogtab", + upstream_owner="some-org", + upstream_url="https://github.com/some-org/frogtab.git", + ) + assert _cli.get_context(args) == expected_context + + +def test_context_repo_from_login(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.user = SimpleNamespace(login="dwilding") + fake_github.repo = SimpleNamespace(fork=False) + args = argparse.Namespace( + base_branch=None, + upstream_owner=None, + repo="jubilant", + new_branch=None, + ) + expected_context = _cli.Context( + base_branch=None, + branch="snapshot0801", + clone_url="https://github.com/dwilding/jubilant.git", + clone_dir=Path("jubilant/dwilding-snapshot0801"), + create_branch=True, + owner="dwilding", + project="jubilant", + upstream_owner=None, + upstream_url=None, + ) + assert _cli.get_context(args) == expected_context + assert fake_github.repo_calls == ["dwilding/jubilant"] From 39f104051ddafdf8667be69524535265d2361c82 Mon Sep 17 00:00:00 2001 From: Dave Wilding Date: Tue, 27 Oct 2026 05:56:45 -0500 Subject: [PATCH 2/4] test: cover the bad-token ValueError in unit tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/unit/test_token.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_token.py b/tests/unit/test_token.py index 529071d..5343955 100644 --- a/tests/unit/test_token.py +++ b/tests/unit/test_token.py @@ -32,7 +32,7 @@ class FakeGithubState: repo: object | None = None user: object | None = None missing: bool = False # Whether to raise UnknownObjectException for a missing repo. - denied: bool = False # Whether to raise GithubException for a rejected token. + denied: bool = False # Whether to raise BadCredentialsException for a rejected token. clients: list = field(default_factory=list) repo_calls: list = field(default_factory=list) @@ -53,10 +53,12 @@ def get_repo(self, owner_project): if state.missing: raise github.UnknownObjectException(404, None) if state.denied: - raise github.GithubException(401, "Bad credentials") + raise github.BadCredentialsException(401, "Bad credentials") return state.repo def get_user(self): + if state.denied: + raise github.BadCredentialsException(401, "Bad credentials") return state.user monkeypatch.setattr(github, "Github", FakeGithub) @@ -124,11 +126,19 @@ def test_upstream_missing(monkeypatch, fake_github): def test_upstream_denied(monkeypatch, fake_github): - # GitHub rejects an invalid token with GithubException, which get_github_upstream does not catch. monkeypatch.setattr(_cli, "GITHUB_TOKEN", "bad-token") fake_github.denied = True - with pytest.raises(github.GithubException): + with pytest.raises(ValueError) as exc_info: _cli.get_github_upstream("dwilding", "frogtab") + assert str(exc_info.value) == "GitHub rejected the token. Has the token expired?" + + +def test_github_url_denied(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "bad-token") + fake_github.denied = True + with pytest.raises(ValueError) as exc_info: + _cli.make_github_url("frogtab") + assert str(exc_info.value) == "GitHub rejected the token. Has the token expired?" @pytest.mark.parametrize("repo", ["frogtab", "frogtab/"]) From 6f72ab71649857ad1ba3b7c0fc697c91085d1b49 Mon Sep 17 00:00:00 2001 From: Dave Wilding Date: Tue, 27 Oct 2026 05:59:50 -0500 Subject: [PATCH 3/4] fix: assert on clone dir, not its parent, in bad-token tests The jubilant/ parent dir is created by earlier tests' clones in CI, so asserting it doesn't exist fails there. Assert the dir holds no clone instead, tolerating its absence when the earlier tests are skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/functional/test_token.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_token.py b/tests/functional/test_token.py index 0091767..b24b148 100644 --- a/tests/functional/test_token.py +++ b/tests/functional/test_token.py @@ -93,7 +93,7 @@ def test_bad_token_no_owner(uv_run, test_dir): Error: GitHub rejected the token. Has the token expired? """ assert result.stderr == expected_stderr - assert not (test_dir / "jubilant").exists() + assert not (test_dir / "jubilant").exists() or not any((test_dir / "jubilant").iterdir()) def test_bad_token_no_upstream_owner(uv_run, test_dir): @@ -120,7 +120,7 @@ def test_bad_token_no_upstream_owner(uv_run, test_dir): Error: GitHub rejected the token. Has the token expired? """ assert result.stderr == expected_stderr - assert not (test_dir / "jubilant").exists() + assert not (test_dir / "jubilant").exists() or not any((test_dir / "jubilant").iterdir()) @pytest.mark.skipif(helpers.token_unset.condition, reason=helpers.token_unset.reason) From 1cb60c78566212ace16fc2ba9f83db93669d6dbb Mon Sep 17 00:00:00 2001 From: Dave Wilding Date: Tue, 27 Oct 2026 06:22:45 -0500 Subject: [PATCH 4/4] adjust new tests --- tests/unit/test_token.py | 54 +++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/tests/unit/test_token.py b/tests/unit/test_token.py index 5343955..02d9e35 100644 --- a/tests/unit/test_token.py +++ b/tests/unit/test_token.py @@ -66,14 +66,7 @@ def get_user(self): @pytest.mark.parametrize("token", [None, ""]) -def test_upstream_no_token(monkeypatch, fake_github, token): - monkeypatch.setattr(_cli, "GITHUB_TOKEN", token) - assert _cli.get_github_upstream("dwilding", "forked-jubilant") is None - assert fake_github.clients == [] - - -@pytest.mark.parametrize("token", [None, ""]) -def test_github_url_no_token(monkeypatch, fake_github, token): +def test_github_url_token_unset(monkeypatch, fake_github, token): monkeypatch.setattr(_cli, "GITHUB_TOKEN", token) with pytest.raises(ValueError) as exc_info: _cli.make_github_url("frogtab") @@ -83,6 +76,29 @@ def test_github_url_no_token(monkeypatch, fake_github, token): assert fake_github.clients == [] +@pytest.mark.parametrize("repo", ["frogtab", "frogtab/"]) +def test_github_url_project(monkeypatch, fake_github, repo): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") + fake_github.user = SimpleNamespace(login="dwilding") + assert _cli.make_github_url(repo) == "https://github.com/dwilding/frogtab" + assert fake_github.clients[0].token == "fake-token" + + +def test_github_url_denied(monkeypatch, fake_github): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", "bad-token") + fake_github.denied = True + with pytest.raises(ValueError) as exc_info: + _cli.make_github_url("frogtab") + assert str(exc_info.value) == "GitHub rejected the token. Has the token expired?" + + +@pytest.mark.parametrize("token", [None, ""]) +def test_upstream_token_unset(monkeypatch, fake_github, token): + monkeypatch.setattr(_cli, "GITHUB_TOKEN", token) + assert _cli.get_github_upstream("dwilding", "forked-jubilant") is None + assert fake_github.clients == [] + + def test_upstream_fork(monkeypatch, fake_github): monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") fake_github.repo = FORKED_REPO @@ -133,22 +149,6 @@ def test_upstream_denied(monkeypatch, fake_github): assert str(exc_info.value) == "GitHub rejected the token. Has the token expired?" -def test_github_url_denied(monkeypatch, fake_github): - monkeypatch.setattr(_cli, "GITHUB_TOKEN", "bad-token") - fake_github.denied = True - with pytest.raises(ValueError) as exc_info: - _cli.make_github_url("frogtab") - assert str(exc_info.value) == "GitHub rejected the token. Has the token expired?" - - -@pytest.mark.parametrize("repo", ["frogtab", "frogtab/"]) -def test_github_url_project(monkeypatch, fake_github, repo): - monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") - fake_github.user = SimpleNamespace(login="dwilding") - assert _cli.make_github_url(repo) == "https://github.com/dwilding/frogtab" - assert fake_github.clients[0].token == "fake-token" - - def test_context_fork(monkeypatch, fake_github): monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") fake_github.repo = FORKED_REPO @@ -172,6 +172,8 @@ def test_context_fork(monkeypatch, fake_github): assert _cli.get_context(args) == expected_context +# -u overrides the fork's upstream, but get_context still queries the fork. +# The query doubles as a fast-fail check that the repo exists, so it's worth keeping. def test_context_fork_u_overrides_upstream(monkeypatch, fake_github): monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") fake_github.repo = FORKED_REPO @@ -193,10 +195,11 @@ def test_context_fork_u_overrides_upstream(monkeypatch, fake_github): upstream_url="https://github.com/some-org/forked-jubilant.git", ) assert _cli.get_context(args) == expected_context - # get_context still queries the fork on GitHub. assert fake_github.repo_calls == ["dwilding/forked-jubilant"] +# -b overrides the fork's upstream, but get_context still queries the fork. +# The query doubles as a fast-fail check that the repo exists, so it's worth keeping. def test_context_fork_b_overrides_upstream(monkeypatch, fake_github): monkeypatch.setattr(_cli, "GITHUB_TOKEN", "fake-token") fake_github.repo = FORKED_REPO @@ -218,6 +221,7 @@ def test_context_fork_b_overrides_upstream(monkeypatch, fake_github): upstream_url="https://github.com/some-org/frogtab.git", ) assert _cli.get_context(args) == expected_context + assert fake_github.repo_calls == ["dwilding/forked-jubilant"] def test_context_repo_from_login(monkeypatch, fake_github):