From 0c19a24e5dc2303acd4264b351c2eeead7904f52 Mon Sep 17 00:00:00 2001 From: Suvayu Ali Date: Thu, 4 Jan 2024 12:32:07 +0100 Subject: [PATCH] publish: handle errors Fixes #16 - do not choose default remote when multiple remotes are present - cleanly indicate when pushing fails due to new commits in remote - cleanly indicate when pushing tag fails due to an existing conflicting tag - conftest: accurately replicate repos in fixture, bare remote & checked out working copy --- orchestra/__init__.py | 3 ++ orchestra/publish.py | 48 ++++++++++++++++---------- tests/conftest.py | 50 ++++++++++++++++++++++++--- tests/test_publish.py | 80 ++++++++++++++++++++++++++++++++----------- 4 files changed, 138 insertions(+), 43 deletions(-) diff --git a/orchestra/__init__.py b/orchestra/__init__.py index d23f41b..32ff509 100644 --- a/orchestra/__init__.py +++ b/orchestra/__init__.py @@ -6,6 +6,9 @@ class ErrorCodes(IntEnum): CONFIG_ERR = 1 BRANCH_ERR = 2 COMMIT_ERR = 3 + REMOTE_ERR = 4 + DUPTAG_ERR = 5 + USERINPUT_ERR = 6 def exit(self, msg: str = ""): if msg: diff --git a/orchestra/publish.py b/orchestra/publish.py index d6ed83a..f0d0621 100644 --- a/orchestra/publish.py +++ b/orchestra/publish.py @@ -3,10 +3,12 @@ import shlex import subprocess -from git import Repo +from git import GitCommandError, Repo +from git.remote import PushInfoList from rich.console import Console from rich.prompt import Prompt +from orchestra import ErrorCodes from orchestra.config import CONF from orchestra.release import remote_name @@ -15,7 +17,19 @@ console = Console() -def push_tags(repo: Repo, tag: str): +def git_error_handler(res: PushInfoList, err: ErrorCodes, msg: str): + try: + res.raise_if_error() + except GitCommandError as exc: + console.print(f"pushing {msg} failed!") + console.print(f"Git command: {exc.command!r} failed with {exc.status=}") + console.print(exc.stderr) + err.exit() + else: + console.print(f"pushed {msg}") + + +def push_tags(pkg: str, repo: Repo, tag: str): if len(repo.remotes) > 1: console.print( *( @@ -28,23 +42,21 @@ def push_tags(repo: Repo, tag: str): try: remote = repo.remotes[int(response)] except ValueError: - console.print(f"invalid selection: {response}, selecting first") - remote = repo.remotes[0] + console.print(f"invalid selection: {response}, aborting!") + ErrorCodes.USERINPUT_ERR.exit() + return # for pyright else: - console.print("empty response, selecting first") - remote = repo.remotes[0] + console.print("empty response, aborting!") + ErrorCodes.USERINPUT_ERR.exit() + return # for pyright else: remote = repo.remotes[0] - res = remote.push(tag) - # FIXME: not sure if this is the best way to check for errors - if len(res) == 0: - console.print(f"pushing {tag!r} failed") - res.raise_if_error() - errs = [err for err in res if err.flags & err.ERROR] - if len(errs) > 0: - console.print(f"pushing {tag!r} failed partially") - for err in errs: - console.print(err.summary) + ref = CONF["branches"][pkg] + res = remote.push(refspec=ref) + git_error_handler(res, ErrorCodes.REMOTE_ERR, f"{ref=} -> {remote.name!r}") + + res = remote.push(tags=True) + git_error_handler(res, ErrorCodes.DUPTAG_ERR, f"{tag=} -> {remote.name!r}") def dispatch_workflow(pkgtags_json: Path, **kwargs): @@ -96,7 +108,7 @@ def publish_tags_whls(config: dict, pkgtags: Path): Path to the JSON file containing the package tags """ tags = json.loads(pkgtags.read_text()) - for _, repo_path in config["repos"].items(): + for pkg, repo_path in config["repos"].items(): repo = Repo(repo_path) - push_tags(repo, tags[remote_name(repo)]) + push_tags(pkg, repo, tags[remote_name(repo)]) dispatch_workflow(pkgtags) diff --git a/tests/conftest.py b/tests/conftest.py index 49121cf..daf7318 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,25 +47,65 @@ def rm_ro(_fn, path, exc_info): @pytest.fixture def dup_repos(tmp_path, request): + """Creates a pair of repos with a remote pointing to the other + + The original remote is named 'upstream' in clone2, and 'origin' in + the clone1. The repos are cloned into tmp_path, and deleted after + the test. The fixture returns a tuple of (name, original, clone1, + clone2). + + tests/ -> tmp_path/ -> tmp_path/ + + tmp_path/: + tests/ (origin) + + tmp_path/: + tmp_path/ (origin) + tests/ (upstream) + + Parameters + ---------- + tmp_path : Path + pytest fixture + request : pytest fixture + pytest request object + + Yields + ------ + tuple + (name, original, clone1, clone2) + + """ name = request.param repo = clone_repo(name) - def _clone(_repo: Repo, path: Path): + def _clone(_repo: Repo, path: Path, **kwargs): if path.exists(): return Repo(path) else: - rclone = _repo.clone(f"{path}") + rclone = _repo.clone(f"{path}", **kwargs) return rclone - repo1 = _clone(repo, tmp_path / f"{name}1") - repo2 = _clone(repo1, tmp_path / f"{name}2") + repo1 = _clone(repo, tmp_path / f"{name}.git", bare=True) + repo2 = _clone(repo1, tmp_path / f"{name}") repo2.create_remote("upstream", repo.working_dir) - yield repo, repo1, repo2 + yield name, repo, repo1, repo2 # FIXME: from Python 3.12 onerror is deprecated in favour or onexc shutil.rmtree(repo1.working_dir, onerror=rm_ro) shutil.rmtree(repo2.working_dir, onerror=rm_ro) +def edit(fname: str, payload: str, append: bool = True): + mode = "a" if append else "w" + with open(fname, mode=mode) as f: + f.write(payload) + + +def commit(repo: Repo, fnames: list[str], msg: str): + repo.index.add(fnames) + repo.index.commit(msg) + + @pytest.fixture def repo(request): repo = clone_repo(request.param) diff --git a/tests/test_publish.py b/tests/test_publish.py index 6f14c42..a483ab2 100644 --- a/tests/test_publish.py +++ b/tests/test_publish.py @@ -2,49 +2,88 @@ from pathlib import Path import sys -from git import GitCommandError, Remote +from git import Remote import pytest +from orchestra import ErrorCodes from orchestra.config import CONF from orchestra.publish import dispatch_workflow, push_tags +from .conftest import commit, edit, example_pkgs + @pytest.mark.parametrize("dup_repos", ["scm"], indirect=True) -@pytest.mark.parametrize("response", ["0", "bad", "\n", None]) -def test_push_tags(dup_repos, response, monkeypatch, capsys): - _, src, dst = dup_repos - dst.create_tag("test_tag") +@pytest.mark.parametrize("response", ["0", None]) +def test_push_tags(dup_repos, response, monkeypatch): + name, _, src, dst = dup_repos if response is None: Remote.rm(dst, "upstream") # created in fixture else: monkeypatch.setattr("sys.stdin", StringIO(response)) - push_tags(dst, "test_tag") + dst.create_tag("test_tag") + push_tags(example_pkgs[name], dst, "test_tag") # test_tag from fixture assert any(t for t in src.tags if t.name == "test_tag") + +@pytest.mark.parametrize("dup_repos", ["scm"], indirect=True) +@pytest.mark.parametrize("response", ["bad", "\n"]) +def test_push_tags_bad_prompt(dup_repos, response, monkeypatch, capsys): + name, _, _, dst = dup_repos + monkeypatch.setattr("sys.stdin", StringIO(response)) + + with pytest.raises(SystemExit, match=str(ErrorCodes.USERINPUT_ERR)): + push_tags(example_pkgs[name], dst, "dummy") + captured = capsys.readouterr() - if response in ("bad", "\n"): - assert "selecting first" in captured.out - if response == "bad": + match response: + case "bad": assert "invalid selection" in captured.out - if response == "\n": + case "\n": assert "empty response" in captured.out -@pytest.mark.skip(reason="push error handling not implemented") +def conflicting_edit(repo1, repo2, fname): + edit(f"{repo2.working_dir}/{fname}", "upstream") + commit(repo2, [fname], "upstream edit") + repo2.remotes[0].push() # master + repo2.git.reset("--hard", "HEAD~1") + + edit(f"{repo2.working_dir}/{fname}", "downstream") + commit(repo2, [fname], "downstream edit") + + +def conflicting_tags(repo1, repo2, fname, tag): + edit(f"{repo2.working_dir}/{fname}", "downstream") + commit(repo2, [fname], "downstream edit") + + repo1.create_tag(tag) + repo2.create_tag(tag) + + @pytest.mark.parametrize("dup_repos", ["scm"], indirect=True) -def test_push_tags_err(dup_repos, capsys): - _, src, dst = dup_repos - Remote.rm(dst, "upstream") # created in fixture - dst.create_tag("test_tag") +@pytest.mark.parametrize("err", [ErrorCodes.DUPTAG_ERR, ErrorCodes.REMOTE_ERR]) +def test_push_tags_err(dup_repos, err, capsys): + name, _, src, dst = dup_repos + Remote.rm(dst, "upstream") # simplify choice of remotes + + if err == ErrorCodes.REMOTE_ERR: + # edit README.md to trigger a merge conflict + conflicting_edit(src, dst, "README.md") + if err == ErrorCodes.DUPTAG_ERR: + # create the same tag on both repos + conflicting_tags(src, dst, "README.md", "test_tag") - Path(src.working_dir).rename(f"{src.working_dir}.bak") + with pytest.raises(SystemExit, match=str(err)): + push_tags(example_pkgs[name], dst, "test_tag") # test_tag from fixture - with pytest.raises(GitCommandError): - push_tags(dst, "test_tag") captured = capsys.readouterr() - assert "pushing 'test_tag' failed" in captured.out - Path(f"{src.working_dir}.bak").rename(src.working_dir) + if err == ErrorCodes.REMOTE_ERR: + ref = CONF["branches"][example_pkgs[name]] + assert f"pushing {ref=}" in captured.out + if err == ErrorCodes.DUPTAG_ERR: + tag = "test_tag" + assert f"pushing {tag=}" in captured.out @pytest.mark.parametrize("cmd", ["echo {repo} {workflow}", "grep packagename_regex"]) @@ -58,6 +97,7 @@ def test_dispatch_workflow(cmd, monkeypatch): monkeypatch.setattr("orchestra.publish.CMD_FMT", cmd) res = dispatch_workflow(Path(__file__).parent / "scm.toml") + assert not isinstance(res, Exception) out = res.stdout.decode("utf8") if "echo" in cmd: assert all(token in out for token in CONF["workflow"].values())