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

@@ -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:

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)

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/<repo> -> tmp_path/<clone1> -> tmp_path/<clone2>
+
+        tmp_path/<clone1>:
+          tests/<repo> (origin)
+
+        tmp_path/<clone2>:
+          tmp_path/<clone1> (origin)
+          tests/<repo> (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)

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())