From 03d2ba51ad61ddfbd2b22b2ec0a7c66265b77457 Mon Sep 17 00:00:00 2001 From: Roman Kharitonov Date: Sun, 1 Feb 2026 01:24:33 +1000 Subject: [PATCH] Update badges, add Codecov integration, improve tests - README: Update badges (CI, Codecov, Python 3.12, Release), fix Python version - CI: Add Codecov coverage upload, enable branch coverage, run integration tests - Fix URL parser to ignore non-tree paths (blob, commits) - Fix grammar in status output ("1 addon is" vs "2 addons are") - Refactor tests: extract common fixtures, remove boilerplate, fix imports Co-Authored-By: Claude Opus 4.5 --- .github/workflows/ci.yml | 14 ++- README.md | 8 +- src/snapjaw.py | 12 ++- tests/conftest.py | 86 ++++++++++++++++- tests/test_mygit.py | 150 +++++++----------------------- tests/test_snapjaw_commands.py | 142 ++++++++++++++-------------- tests/test_snapjaw_url_parsing.py | 23 +++++ 7 files changed, 240 insertions(+), 195 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a56de1..272a951 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,19 @@ jobs: run: uv run mypy src/ - name: Tests with coverage - run: uv run pytest --cov=src --cov-report=term-missing --cov-fail-under=100 + run: uv run pytest -m "" --cov=src --cov-branch --cov-report=term-missing --cov-report=xml --cov-fail-under=100 --junitxml=junit.xml -o junit_family=legacy + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: coverage.xml + + - name: Upload test results to Codecov + if: ${{ !cancelled() }} + uses: codecov/test-results-action@v1 + with: + token: ${{ secrets.CODECOV_TOKEN }} build: needs: check diff --git a/README.md b/README.md index 2711988..d6a43fb 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ # snapjaw: Vanilla World of Warcraft AddOn manager -[![License: GPL v3](https://img.shields.io/badge/License-GPLv3-blue.svg)](https://www.gnu.org/licenses/gpl-3.0) [![CodeQL](https://github.com/refaim/snapjaw/actions/workflows/codeql.yml/badge.svg?branch=master)](https://github.com/refaim/snapjaw/actions/workflows/codeql.yml) [![Package](https://github.com/refaim/snapjaw/actions/workflows/package.yml/badge.svg)](https://github.com/refaim/snapjaw/actions/workflows/package.yml) +[![CI](https://github.com/refaim/snapjaw/actions/workflows/ci.yml/badge.svg)](https://github.com/refaim/snapjaw/actions/workflows/ci.yml) +[![codecov](https://codecov.io/gh/refaim/snapjaw/graph/badge.svg)](https://codecov.io/gh/refaim/snapjaw) +[![Python 3.12](https://img.shields.io/badge/python-3.12-blue.svg)](https://www.python.org/downloads/release/python-3120/) +[![License: GPL v3](https://img.shields.io/badge/License-GPLv3-blue.svg)](https://www.gnu.org/licenses/gpl-3.0) +[![GitHub release](https://img.shields.io/github/v/release/refaim/snapjaw)](https://github.com/refaim/snapjaw/releases/latest) ## Features - Support for Git repositories as addon sources @@ -70,5 +74,5 @@ snapjaw update ShaguTweaks ``` ## Requirements for developers -- [Python 3.10](https://www.python.org) +- [Python 3.12](https://www.python.org) - [uv](https://docs.astral.sh/uv/) diff --git a/src/snapjaw.py b/src/snapjaw.py index 33b1071..5c6823b 100644 --- a/src/snapjaw.py +++ b/src/snapjaw.py @@ -158,11 +158,11 @@ def cmd_install(config: Config, args): author = path.pop(0) repository = path.pop(0) if path: - if path[0] == "-" and path[1] == "tree": - path = path[2:] + if path[0] == "-" and len(path) > 1 and path[1] == "tree": + branch_from_url = "/".join(path[2:]) elif path[0] == "tree": - path = path[1:] - branch_from_url = "/".join(path) + branch_from_url = "/".join(path[1:]) + # Ignore non-tree paths (blob, commits, etc.) path_string = "/".join([author, repository]) if not path_string.endswith(".git"): path_string += ".git" @@ -318,7 +318,9 @@ def format_dt(dt: datetime | None) -> str: if not args.verbose: num_updated = Counter(s.status for s in addon_states)[AddonStatus.UpToDate] if num_updated > 0: - msg = f"{num_updated}{' other' if table else ''} addons are up to date" + other = " other" if table else "" + noun = "addon is" if num_updated == 1 else "addons are" + msg = f"{num_updated}{other} {noun} up to date" print(cr.Fore.GREEN + msg + cr.Fore.RESET) cr.deinit() diff --git a/tests/conftest.py b/tests/conftest.py index dad8b12..d528f8b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,7 @@ """Shared fixtures for snapjaw tests.""" from datetime import datetime -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pygit2 import pytest @@ -78,6 +78,37 @@ def _make_toc_addon(name, interface_version): return _make_toc_addon +@pytest.fixture +def mock_tmpdir_context(): + """Context manager patch for mocking TemporaryDirectory.""" + mock = MagicMock() + mock.__enter__ = MagicMock(return_value="/tmp/repo") + mock.__exit__ = MagicMock(return_value=False) + return patch("mygit.TemporaryDirectory", return_value=mock) + + +@pytest.fixture +def fetch_states_patches(mock_pygit2_repo, mock_tmpdir_context): + """Combined patches for fetch_states tests. + + Returns a context manager with mocked pygit2 repo, GitError, sha1, and tmpdir. + The sha1 mock is configured to return "abc123" as hexdigest. + """ + from contextlib import ExitStack, contextmanager + + @contextmanager + def _patches(): + with ExitStack() as stack: + stack.enter_context(patch("mygit.pygit2.init_repository", return_value=mock_pygit2_repo)) + stack.enter_context(patch("mygit.pygit2.GitError", pygit2.GitError)) + mock_sha1 = stack.enter_context(patch("mygit.sha1")) + mock_sha1.return_value.hexdigest.return_value = "abc123" + stack.enter_context(mock_tmpdir_context) + yield + + return _patches + + @pytest.fixture def mock_pygit2_repo(): """Create a mocked pygit2.Repository.""" @@ -103,3 +134,56 @@ def _mock_remote(name, url, refs=None, error=None): return remote return _mock_remote + + +@pytest.fixture +def mock_install_env(tmp_path, monkeypatch, fixed_now): + """Setup mocked environment for install_addon tests. + + Returns a context manager that sets up repo_dir, mocks clone/TemporaryDirectory/signature, + and provides addons_dir and config. + """ + from contextlib import contextmanager + + from mygit import RepositoryInfo + from snapjaw import Config + + @contextmanager + def _setup(*, trailing_slash=True): + addons_dir = tmp_path / "Addons" + addons_dir.mkdir(exist_ok=True) + + repo_dir = tmp_path / "repo" + repo_dir.mkdir(exist_ok=True) + + workdir = str(repo_dir) + ("/" if trailing_slash else "") + + repo_info = RepositoryInfo( + workdir=workdir, + branch="master", + head_commit_hex="abc123", + head_commit_time=fixed_now, + ) + monkeypatch.setattr("snapjaw.mygit.clone", lambda url, branch, path: repo_info) + + mock_tmpdir = MagicMock() + mock_tmpdir.__enter__ = MagicMock(return_value=str(repo_dir)) + mock_tmpdir.__exit__ = MagicMock(return_value=False) + monkeypatch.setattr("snapjaw.TemporaryDirectory", lambda: mock_tmpdir) + monkeypatch.setattr("snapjaw.signature.calculate", lambda path: "sig|2") + + config = Config(addons_by_key={}) + config._loaded_from = str(tmp_path / "snapjaw.json") + + class Env: + pass + + env = Env() + env.addons_dir = addons_dir + env.repo_dir = repo_dir + env.config = config + env.repo_info = repo_info + + yield env + + return _setup diff --git a/tests/test_mygit.py b/tests/test_mygit.py index 4459332..2b8802d 100644 --- a/tests/test_mygit.py +++ b/tests/test_mygit.py @@ -1,19 +1,23 @@ """Tests for mygit.py - Git operations wrapper. -Tests focus on public API (clone, fetch_states) through behavior verification. -Internal implementation details are not tested directly. +Tests cover both public API (clone, fetch_states) and internal helpers +(_clone, _GitProgressCallbacks, _has_remote) to ensure complete coverage. """ from datetime import datetime +from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pygit2 import pytest from mygit import ( GitError, - RemoteState, RemoteStateRequest, RepositoryInfo, + _clone, + _GitProgressCallbacks, + _has_remote, clone, fetch_states, ) @@ -66,15 +70,7 @@ def test_error_raises_git_error(self): class TestFetchStates: """Tests for fetch_states() - checking remote repository states.""" - @pytest.fixture - def mock_tmpdir_context(self): - """Context manager for mocking TemporaryDirectory.""" - mock = MagicMock() - mock.__enter__ = MagicMock(return_value="/tmp/repo") - mock.__exit__ = MagicMock(return_value=False) - return patch("mygit.TemporaryDirectory", return_value=mock) - - def test_success_returns_commit_hash(self, mock_remote, mock_pygit2_repo, mock_tmpdir_context): + def test_success_returns_commit_hash(self, mock_remote, mock_pygit2_repo, fetch_states_patches): """Successful fetch returns commit hash for branch.""" remote = mock_remote( "abc123", @@ -83,14 +79,7 @@ def test_success_returns_commit_hash(self, mock_remote, mock_pygit2_repo, mock_t ) mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote])) - with ( - patch("mygit.pygit2.init_repository", return_value=mock_pygit2_repo), - patch("mygit.pygit2.GitError", Exception), - patch("mygit.sha1") as mock_sha1, - mock_tmpdir_context, - ): - mock_sha1.return_value.hexdigest.return_value = "abc123" - + with fetch_states_patches(): requests = [RemoteStateRequest("https://github.com/test/repo.git", "master")] states = list(fetch_states(requests)) @@ -98,7 +87,7 @@ def test_success_returns_commit_hash(self, mock_remote, mock_pygit2_repo, mock_t assert states[0].head_commit_hex == "deadbeef" assert states[0].error is None - def test_head_symref_resolves_branch(self, mock_remote, mock_pygit2_repo, mock_tmpdir_context): + def test_head_symref_resolves_branch(self, mock_remote, mock_pygit2_repo, fetch_states_patches): """HEAD symref pointing to branch returns correct commit.""" remote = mock_remote( "abc123", @@ -107,21 +96,14 @@ def test_head_symref_resolves_branch(self, mock_remote, mock_pygit2_repo, mock_t ) mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote])) - with ( - patch("mygit.pygit2.init_repository", return_value=mock_pygit2_repo), - patch("mygit.pygit2.GitError", Exception), - patch("mygit.sha1") as mock_sha1, - mock_tmpdir_context, - ): - mock_sha1.return_value.hexdigest.return_value = "abc123" - + with fetch_states_patches(): requests = [RemoteStateRequest("https://github.com/test/repo.git", "main")] states = list(fetch_states(requests)) assert len(states) == 1 assert states[0].head_commit_hex == "cafebabe" - def test_git_error_returns_error_state(self, mock_remote, mock_pygit2_repo, mock_tmpdir_context): + def test_git_error_returns_error_state(self, mock_remote, mock_pygit2_repo, fetch_states_patches): """Git error returns state with error message, no commit.""" remote = mock_remote( "abc123", @@ -130,14 +112,7 @@ def test_git_error_returns_error_state(self, mock_remote, mock_pygit2_repo, mock ) mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote])) - with ( - patch("mygit.pygit2.init_repository", return_value=mock_pygit2_repo), - patch("mygit.pygit2.GitError", Exception), - patch("mygit.sha1") as mock_sha1, - mock_tmpdir_context, - ): - mock_sha1.return_value.hexdigest.return_value = "abc123" - + with fetch_states_patches(): requests = [RemoteStateRequest("https://github.com/test/repo.git", "master")] states = list(fetch_states(requests)) @@ -145,25 +120,33 @@ def test_git_error_returns_error_state(self, mock_remote, mock_pygit2_repo, mock assert states[0].error == "connection refused" assert states[0].head_commit_hex is None - def test_existing_remote_reused(self, mock_remote, mock_pygit2_repo, mock_tmpdir_context): + def test_existing_remote_reused(self, mock_remote, mock_pygit2_repo, fetch_states_patches): """Existing remote is reused, not recreated.""" remote = mock_remote("abc123", "https://github.com/test/repo.git", refs=[]) mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote])) mock_pygit2_repo.remotes.__getitem__ = MagicMock(return_value=remote) - with ( - patch("mygit.pygit2.init_repository", return_value=mock_pygit2_repo), - patch("mygit.pygit2.GitError", Exception), - patch("mygit.sha1") as mock_sha1, - mock_tmpdir_context, - ): - mock_sha1.return_value.hexdigest.return_value = "abc123" - + with fetch_states_patches(): requests = [RemoteStateRequest("https://github.com/test/repo.git", "master")] list(fetch_states(requests)) mock_pygit2_repo.remotes.create.assert_not_called() + def test_branch_not_found_in_refs(self, mock_remote, mock_pygit2_repo, fetch_states_patches): + """When requested branch is not in refs, no state is yielded for it.""" + remote = mock_remote( + "abc123", + "https://github.com/test/repo.git", + refs=[{"name": "refs/heads/other-branch", "symref_target": "", "oid": "deadbeef"}], + ) + mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote])) + + with fetch_states_patches(): + requests = [RemoteStateRequest("https://github.com/test/repo.git", "master")] + states = list(fetch_states(requests)) + + assert len(states) == 0 + class TestCloneInternalProcess: """Tests for _clone() - the subprocess worker function. @@ -174,7 +157,7 @@ class TestCloneInternalProcess: def test_success_sends_repository_info(self): """Successful clone sends RepositoryInfo through data connection.""" - mock_commit = MagicMock() + mock_commit = MagicMock(spec=pygit2.Commit) mock_commit.id = "abc123" mock_commit.commit_time = 1704067200 # 2024-01-01 00:00:00 UTC @@ -190,11 +173,9 @@ def test_success_sends_repository_info(self): data_conn = MagicMock() error_conn = MagicMock() - from mygit import _clone - with patch("mygit.pygit2") as mock_pygit2: mock_pygit2.clone_repository.return_value = mock_repo - mock_pygit2.Commit = type(mock_commit) + mock_pygit2.Commit = pygit2.Commit _clone("https://github.com/test/repo.git", "master", "/tmp/repo", data_conn, error_conn) data_conn.send.assert_called_once() @@ -208,11 +189,9 @@ def test_git_error_sends_error(self): data_conn = MagicMock() error_conn = MagicMock() - from mygit import _clone - with patch("mygit.pygit2") as mock_pygit2: - mock_pygit2.GitError = Exception - mock_pygit2.clone_repository.side_effect = Exception("auth failed") + mock_pygit2.GitError = pygit2.GitError + mock_pygit2.clone_repository.side_effect = pygit2.GitError("auth failed") _clone("https://github.com/test/repo.git", None, "/tmp/repo", data_conn, error_conn) error_conn.send.assert_called_once() @@ -225,10 +204,8 @@ def test_key_error_sends_error(self): data_conn = MagicMock() error_conn = MagicMock() - from mygit import _clone - with patch("mygit.pygit2") as mock_pygit2: - mock_pygit2.GitError = Exception + mock_pygit2.GitError = pygit2.GitError mock_pygit2.clone_repository.side_effect = KeyError("bad branch") _clone("https://github.com/test/repo.git", "nonexistent", "/tmp/repo", data_conn, error_conn) @@ -244,7 +221,6 @@ class TestGitProgressCallbacks: def test_sideband_progress_prints(self, capsys): """sideband_progress prints message.""" - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() cb.sideband_progress("remote: Counting objects") @@ -253,9 +229,6 @@ def test_sideband_progress_prints(self, capsys): def test_transfer_progress_receiving_objects(self, capsys): """Receiving objects shows percentage.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() progress = SimpleNamespace( @@ -271,9 +244,6 @@ def test_transfer_progress_receiving_objects(self, capsys): def test_transfer_progress_objects_done(self, capsys): """Completed objects shows 'done'.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() progress = SimpleNamespace( @@ -289,9 +259,6 @@ def test_transfer_progress_objects_done(self, capsys): def test_transfer_progress_indexing_deltas(self, capsys): """Indexing deltas shows percentage after objects done.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() cb._objects_done = True @@ -308,9 +275,6 @@ def test_transfer_progress_indexing_deltas(self, capsys): def test_transfer_progress_deltas_done(self, capsys): """Completed deltas shows 'done'.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() cb._objects_done = True @@ -327,9 +291,6 @@ def test_transfer_progress_deltas_done(self, capsys): def test_transfer_progress_zero_deltas_skipped(self, capsys): """Zero total deltas produces no output.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() cb._objects_done = True @@ -346,9 +307,6 @@ def test_transfer_progress_zero_deltas_skipped(self, capsys): def test_transfer_progress_after_all_done_no_output(self, capsys): """After both objects and deltas done, no output.""" - from types import SimpleNamespace - - from mygit import _GitProgressCallbacks cb = _GitProgressCallbacks() cb._objects_done = True @@ -370,7 +328,6 @@ class TestHasRemote: def test_remote_exists(self): """Returns True when remote exists.""" - from mygit import _has_remote repo = MagicMock() repo.remotes.__getitem__ = MagicMock(return_value=MagicMock()) @@ -378,44 +335,7 @@ def test_remote_exists(self): def test_remote_not_exists(self): """Returns False when remote doesn't exist.""" - from mygit import _has_remote repo = MagicMock() repo.remotes.__getitem__ = MagicMock(side_effect=KeyError) assert _has_remote(repo, "origin") is False - - -class TestDataClasses: - """Tests for data classes - ensure they hold expected data.""" - - def test_repository_info_fields(self): - """RepositoryInfo stores all required fields.""" - info = RepositoryInfo( - workdir="/tmp/repo/", - branch="master", - head_commit_hex="abc123", - head_commit_time=datetime(2024, 1, 1), - ) - assert info.workdir == "/tmp/repo/" - assert info.branch == "master" - assert info.head_commit_hex == "abc123" - - def test_remote_state_fields(self): - """RemoteState stores URL, branch, commit, and error.""" - state = RemoteState( - url="https://github.com/test/repo.git", - branch="master", - head_commit_hex="abc123", - error=None, - ) - assert state.url == "https://github.com/test/repo.git" - assert state.error is None - - def test_remote_state_request_fields(self): - """RemoteStateRequest stores URL and branch.""" - request = RemoteStateRequest( - url="https://github.com/test/repo.git", - branch="master", - ) - assert request.url == "https://github.com/test/repo.git" - assert request.branch == "master" diff --git a/tests/test_snapjaw_commands.py b/tests/test_snapjaw_commands.py index 32f7d3e..26b11c8 100644 --- a/tests/test_snapjaw_commands.py +++ b/tests/test_snapjaw_commands.py @@ -1,14 +1,13 @@ """Tests for snapjaw CLI commands (install, remove, update, status).""" import json -import os import sys from types import SimpleNamespace from unittest.mock import MagicMock import pytest -from mygit import RepositoryInfo +from mygit import GitError from snapjaw import ( AddonState, AddonStatus, @@ -21,6 +20,7 @@ remove_addon_dir, run_command, ) +from toc import Addon class TestRunCommand: @@ -40,14 +40,14 @@ def test_read_only_does_not_create_backup(self, tmp_path): args = SimpleNamespace(addons_dir=str(tmp_path)) run_command(callback, True, args) callback.assert_called_once() - assert not os.path.exists(tmp_path / "snapjaw.backup.json") + assert not (tmp_path / "snapjaw.backup.json").exists() def test_write_saves_config(self, tmp_path): """Write command saves config after execution.""" callback = MagicMock() args = SimpleNamespace(addons_dir=str(tmp_path)) run_command(callback, False, args) - assert os.path.exists(tmp_path / "snapjaw.json") + assert (tmp_path / "snapjaw.json").exists() def test_write_creates_backup(self, tmp_path): """Write command creates backup of existing config.""" @@ -56,7 +56,7 @@ def test_write_creates_backup(self, tmp_path): callback = MagicMock() args = SimpleNamespace(addons_dir=str(tmp_path)) run_command(callback, False, args) - assert os.path.exists(tmp_path / "snapjaw.backup.json") + assert (tmp_path / "snapjaw.backup.json").exists() def test_error_restores_backup(self, tmp_path, make_addon): """On error, backup is restored.""" @@ -76,94 +76,94 @@ def bad_callback(config, args): restored = json.load(f) assert restored["addons_by_key"] == {} + def test_read_only_error_does_not_restore_backup(self, tmp_path): + """On error in read-only mode, no backup restoration is attempted.""" + config_path = tmp_path / "snapjaw.json" + config_path.write_text('{"addons_by_key": {}}') + + def bad_callback(config, args): + raise RuntimeError("boom") + + args = SimpleNamespace(addons_dir=str(tmp_path)) + with pytest.raises(RuntimeError, match="boom"): + run_command(bad_callback, True, args) + + # Config unchanged, no backup was created or restored + assert not (tmp_path / "snapjaw.backup.json").exists() + class TestInstallAddon: """Tests for install_addon() function.""" - def test_success(self, tmp_path, monkeypatch, fixed_now): + def test_success(self, mock_install_env): """Successful install copies addon and updates config.""" - addons_dir = tmp_path / "Addons" - addons_dir.mkdir() - - repo_dir = tmp_path / "repo" - repo_dir.mkdir() - addon_dir = repo_dir / "MyAddon" - addon_dir.mkdir() - (addon_dir / "MyAddon.toc").write_text("## Interface: 11200\n") - (addon_dir / "init.lua").write_text("-- addon code") - - repo_info = RepositoryInfo( - workdir=str(repo_dir) + "/", - branch="master", - head_commit_hex="abc123", - head_commit_time=fixed_now, - ) - monkeypatch.setattr("snapjaw.mygit.clone", lambda url, branch, path: repo_info) - mock_tmpdir = MagicMock() - mock_tmpdir.__enter__ = MagicMock(return_value=str(repo_dir)) - mock_tmpdir.__exit__ = MagicMock(return_value=False) - monkeypatch.setattr("snapjaw.TemporaryDirectory", lambda: mock_tmpdir) - monkeypatch.setattr("snapjaw.signature.calculate", lambda path: "sig|2") + with mock_install_env() as env: + addon_dir = env.repo_dir / "MyAddon" + addon_dir.mkdir() + (addon_dir / "MyAddon.toc").write_text("## Interface: 11200\n") + (addon_dir / "init.lua").write_text("-- addon code") - config = Config(addons_by_key={}) - config._loaded_from = str(tmp_path / "snapjaw.json") - install_addon(config, "https://github.com/test/repo.git", "master", str(addons_dir)) + install_addon(env.config, "https://github.com/test/repo.git", "master", str(env.addons_dir)) - assert "myaddon" in config.addons_by_key - assert os.path.exists(addons_dir / "MyAddon" / "init.lua") + assert "myaddon" in env.config.addons_by_key + assert (env.addons_dir / "MyAddon" / "init.lua").exists() def test_git_error_raises_cli_error(self, tmp_path, monkeypatch): """Git clone failure raises CliError.""" - from mygit import GitError - monkeypatch.setattr("snapjaw.mygit.clone", MagicMock(side_effect=GitError("auth failed"))) config = Config(addons_by_key={}) with pytest.raises(CliError, match="auth failed"): install_addon(config, "https://github.com/test/repo.git", None, str(tmp_path)) - def test_no_addons_found_raises_error(self, tmp_path, monkeypatch, fixed_now): + def test_no_addons_found_raises_error(self, mock_install_env, monkeypatch): """No vanilla addons in repo raises CliError.""" - repo_info = RepositoryInfo( - workdir=str(tmp_path) + "/", branch="master", head_commit_hex="abc", head_commit_time=fixed_now - ) - monkeypatch.setattr("snapjaw.mygit.clone", lambda url, branch, path: repo_info) - monkeypatch.setattr("snapjaw.toc.find_addons", lambda workdir, version: iter([])) + with mock_install_env() as env: + monkeypatch.setattr("snapjaw.toc.find_addons", lambda workdir, version: iter([])) - config = Config(addons_by_key={}) - with pytest.raises(CliError, match="no vanilla addons found"): - install_addon(config, "https://github.com/test/repo.git", None, str(tmp_path)) + with pytest.raises(CliError, match="no vanilla addons found"): + install_addon(env.config, "https://github.com/test/repo.git", None, str(env.addons_dir)) - def test_copies_readme_from_root(self, tmp_path, monkeypatch, fixed_now): + def test_copies_readme_from_root(self, mock_install_env): """Readme from repo root is copied to addon directory.""" - addons_dir = tmp_path / "Addons" - addons_dir.mkdir() + with mock_install_env() as env: + addon_dir = env.repo_dir / "MyAddon" + addon_dir.mkdir() + (addon_dir / "MyAddon.toc").write_text("## Interface: 11200\n") + (env.repo_dir / "README.txt").write_text("read me") - repo_dir = tmp_path / "repo" - repo_dir.mkdir() - addon_dir = repo_dir / "MyAddon" - addon_dir.mkdir() - (addon_dir / "MyAddon.toc").write_text("## Interface: 11200\n") - (repo_dir / "README.txt").write_text("read me") - - repo_info = RepositoryInfo( - workdir=str(repo_dir) + "/", - branch="master", - head_commit_hex="abc123", - head_commit_time=fixed_now, - ) - monkeypatch.setattr("snapjaw.mygit.clone", lambda url, branch, path: repo_info) - mock_tmpdir = MagicMock() - mock_tmpdir.__enter__ = MagicMock(return_value=str(repo_dir)) - mock_tmpdir.__exit__ = MagicMock(return_value=False) - monkeypatch.setattr("snapjaw.TemporaryDirectory", lambda: mock_tmpdir) - monkeypatch.setattr("snapjaw.signature.calculate", lambda path: "sig|2") + install_addon(env.config, "https://github.com/test/repo.git", "master", str(env.addons_dir)) - config = Config(addons_by_key={}) - config._loaded_from = str(tmp_path / "snapjaw.json") - install_addon(config, "https://github.com/test/repo.git", "master", str(addons_dir)) + assert (env.addons_dir / "MyAddon" / "README.txt").read_text() == "read me" + + def test_addon_in_repo_root_skips_readme_copy(self, mock_install_env, monkeypatch): + """When addon is in repo root (workdir == addon.path), readme copy is skipped.""" + with mock_install_env(trailing_slash=False) as env: + (env.repo_dir / "MyAddon.toc").write_text("## Interface: 11200\n") + (env.repo_dir / "README.txt").write_text("root readme") + # Mock find_addons to return addon with path equal to workdir + monkeypatch.setattr( + "snapjaw.toc.find_addons", lambda workdir, version: iter([Addon("MyAddon", str(env.repo_dir))]) + ) + + install_addon(env.config, "https://github.com/test/repo.git", "master", str(env.addons_dir)) + + # README exists because it was copied with the addon (copytree), not from root copy logic + assert (env.addons_dir / "MyAddon" / "README.txt").read_text() == "root readme" + + def test_existing_readme_not_overwritten(self, mock_install_env): + """Readme already in addon directory is not overwritten by root copy.""" + with mock_install_env() as env: + addon_dir = env.repo_dir / "MyAddon" + addon_dir.mkdir() + (addon_dir / "MyAddon.toc").write_text("## Interface: 11200\n") + (addon_dir / "README.txt").write_text("addon readme") + (env.repo_dir / "README.txt").write_text("root readme") + + install_addon(env.config, "https://github.com/test/repo.git", "master", str(env.addons_dir)) - assert (addons_dir / "MyAddon" / "README.txt").read_text() == "read me" + # Addon's own readme is preserved, not overwritten by root readme + assert (env.addons_dir / "MyAddon" / "README.txt").read_text() == "addon readme" class TestCmdRemove: @@ -337,7 +337,7 @@ def test_with_addons_shows_table(self, tmp_path, monkeypatch, fixed_now, capsys) cmd_status(config, args) out = capsys.readouterr().out assert "MyAddon" in out - assert "1 other addons are up to date" in out + assert "1 other addon is up to date" in out def test_verbose_shows_all_addons(self, tmp_path, monkeypatch, fixed_now, capsys): """Verbose mode shows up-to-date addons in table.""" diff --git a/tests/test_snapjaw_url_parsing.py b/tests/test_snapjaw_url_parsing.py index 883acfe..87bdef5 100644 --- a/tests/test_snapjaw_url_parsing.py +++ b/tests/test_snapjaw_url_parsing.py @@ -59,6 +59,18 @@ def mock_install(config, repo_url, branch, addons_dir): "https://gitlab.com/Artur91425/GrimoireKeeper.git", "master", ), + # GitLab URL without -/ prefix (old format) + ( + "https://gitlab.com/Artur91425/GrimoireKeeper/tree/master", + "https://gitlab.com/Artur91425/GrimoireKeeper.git", + "master", + ), + # GitHub URL with non-tree path (e.g. blob link) - path is ignored + ( + "https://github.com/refaim/MissingCrafts/blob/master", + "https://github.com/refaim/MissingCrafts.git", + None, + ), # Non-GitHub/GitLab URL passed through ( "https://custom.server/repo.git", @@ -72,6 +84,8 @@ def mock_install(config, repo_url, branch, addons_dir): "github_with_branch", "gitlab_simple", "gitlab_with_branch", + "gitlab_with_branch_old_format", + "github_with_non_tree_path", "custom_url_passthrough", ], ) @@ -90,6 +104,15 @@ def test_explicit_branch_override(self, run_install): assert repo_url == "https://github.com/fusionpit/QuestFrameFixer.git" assert branch == "1.12.1" + def test_matching_branch_in_url_and_arg(self, run_install): + """Matching branch in URL and --branch argument works fine.""" + repo_url, branch = run_install( + "https://github.com/fusionpit/QuestFrameFixer/tree/1.12.1", + branch="1.12.1", + ) + assert repo_url == "https://github.com/fusionpit/QuestFrameFixer.git" + assert branch == "1.12.1" + def test_branch_conflict_raises_error(self, run_install): """Conflicting branch in URL and --branch argument raises CliError.""" with pytest.raises(CliError, match="requested branch"):