feat(sync): Continue on error by default, CLI test infra (#387)

Fixes #363, related to #366

CLI sync:
- Fix arguments on sub commands
- Continue to next repo if encountering error when syncing
- New flag: `--exit-on-error` / `-x`
- Also stubs out basic CLI tests

Author: tony

CHANGES

@@ -22,6 +22,23 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force
 ### What's new
 
 - Refreshed logo
+- `vcspull sync`:
+
+  - Syncing will now skip to the next repos if an error is encountered
+
+  - Learned `--exit-on-error` / `-x`
+
+    Usage:
+
+    ```console
+    $ vcspull sync --exit-on-error grako django
+    ```
+
+    Print traceback for errored repos:
+
+    ```console
+    $ vcspull --log-level DEBUG sync --exit-on-error grako django
+    ```
 
 ### Development
 
@@ -33,6 +50,10 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force
 - Add [flake8-bugbear](https://github.com/PyCQA/flake8-bugbear) (#379)
 - Add [flake8-comprehensions](https://github.com/adamchainz/flake8-comprehensions) (#380)
 
+### Testing
+
+- Add CLI tests (#387)
+
 ### Documentation
 
 - Render changelog in sphinx-autoissues (#378)

docs/cli/sync.md

@@ -4,6 +4,22 @@
 
 # vcspull sync
 
+## Error handling
+
+As of 1.13.x, vcspull will continue to the next repo if an error is encountered when syncing multiple repos.
+
+To imitate the old behavior, use `--exit-on-error` / `-x`:
+
+```console
+$ vcspull sync --exit-on-error grako django
+```
+
+Print traceback for errored repos:
+
+```console
+$ vcspull --log-level DEBUG sync --exit-on-error grako django
+```
+
 ```{eval-rst}
 .. click:: vcspull.cli.sync:sync
     :prog: vcspull sync

src/vcspull/cli/__init__.py

@@ -19,8 +19,8 @@
 
 @click.group(
     context_settings={
+        "obj": {},
         "help_option_names": ["-h", "--help"],
-        "allow_interspersed_args": True,
     }
 )
 @click.option(

src/vcspull/cli/sync.py

@@ -59,6 +59,9 @@ def clamp(n, _min, _max):
     return max(_min, min(n, _max))
 
 
+EXIT_ON_ERROR_MSG = "Exiting via error (--exit-on-error passed)"
+
+
 @click.command(name="sync")
 @click.argument(
     "repo_terms", type=click.STRING, nargs=-1, shell_complete=get_repo_completions
@@ -71,7 +74,15 @@ def clamp(n, _min, _max):
     help="Specify config",
     shell_complete=get_config_file_completions,
 )
-def sync(repo_terms, config):
+@click.option(
+    "exit_on_error",
+    "--exit-on-error",
+    "-x",
+    is_flag=True,
+    default=False,
+    help="Exit immediately when encountering an error syncing multiple repos",
+)
+def sync(repo_terms, config, exit_on_error: bool) -> None:
     if config:
         configs = load_configs([config])
     else:
@@ -95,7 +106,19 @@ def sync(repo_terms, config):
     else:
         found_repos = configs
 
-    list(map(update_repo, found_repos))
+    for repo in found_repos:
+        try:
+            update_repo(repo)
+        except Exception:
+            click.echo(
+                f'Failed syncing {repo.get("name")}',
+            )
+            if log.isEnabledFor(logging.DEBUG):
+                import traceback
+
+                traceback.print_exc()
+            if exit_on_error:
+                raise click.ClickException(EXIT_ON_ERROR_MSG)
 
 
 def progress_cb(output, timestamp):

src/vcspull/log.py

@@ -29,7 +29,7 @@ def setup_logger(log=None, level="INFO"):
 
     Parameters
     ----------
-    log : :py:class:`Logger`
+    log : :py:class:`logging.Logger`
         instance of logger
     """
     if not log:

tests/conftest.py

@@ -71,7 +71,7 @@ def git_repo_kwargs(repos_path: pathlib.Path, git_dummy_repo_dir):
     """Return kwargs for :func:`create_project`."""
     return {
         "url": "git+file://" + git_dummy_repo_dir,
-        "parent_dir": str(repos_path),
+        "dir": str(repos_path / "repo_name"),
         "name": "repo_name",
     }
 

tests/test_cli.py

@@ -1,14 +1,184 @@
+import pathlib
+import typing as t
+
 import pytest
 
+import yaml
 from click.testing import CliRunner
 
+from libvcs.sync.git import GitSync
 from vcspull.cli import cli
+from vcspull.cli.sync import EXIT_ON_ERROR_MSG
+
+
+def test_sync_cli_non_existent(tmp_path: pathlib.Path) -> None:
+    runner = CliRunner()
+    with runner.isolated_filesystem(temp_dir=tmp_path):
+        result = runner.invoke(cli, ["sync", "hi"])
+        assert result.exit_code == 0
+        assert "" in result.output
+
+
+def test_sync(
+    home_path: pathlib.Path,
+    config_path: pathlib.Path,
+    tmp_path: pathlib.Path,
+    git_repo: GitSync,
+) -> None:
+    runner = CliRunner()
+    with runner.isolated_filesystem(temp_dir=tmp_path):
+        config = {
+            "~/github_projects/": {
+                "my_git_repo": {
+                    "url": f"git+file://{git_repo.dir}",
+                    "remotes": {"test_remote": f"git+file://{git_repo.dir}"},
+                },
+                "broken_repo": {
+                    "url": f"git+file://{git_repo.dir}",
+                    "remotes": {"test_remote": "git+file://non-existent-remote"},
+                },
+            }
+        }
+        yaml_config = config_path / ".vcspull.yaml"
+        yaml_config_data = yaml.dump(config, default_flow_style=False)
+        yaml_config.write_text(yaml_config_data, encoding="utf-8")
+
+        # CLI can sync
+        result = runner.invoke(cli, ["sync", "my_git_repo"])
+        assert result.exit_code == 0
+        output = "".join(list(result.output))
+        assert "my_git_repo" in output
+
+
+if t.TYPE_CHECKING:
+    from typing_extensions import TypeAlias
 
+    ExpectedOutput: TypeAlias = t.Optional[t.Union[str, t.List[str]]]
 
-@pytest.mark.skip(reason="todo")
-def test_command_line(self):
+
+class SyncBrokenFixture(t.NamedTuple):
+    test_id: str
+    sync_args: list[str]
+    expected_exit_code: int
+    expected_in_output: "ExpectedOutput" = None
+    expected_not_in_output: "ExpectedOutput" = None
+
+
+SYNC_BROKEN_REPO_FIXTURES = [
+    SyncBrokenFixture(
+        test_id="normal-checkout",
+        sync_args=["my_git_repo"],
+        expected_exit_code=0,
+        expected_in_output="Already on 'master'",
+    ),
+    SyncBrokenFixture(
+        test_id="normal-checkout--exit-on-error",
+        sync_args=["my_git_repo", "--exit-on-error"],
+        expected_exit_code=0,
+        expected_in_output="Already on 'master'",
+    ),
+    SyncBrokenFixture(
+        test_id="normal-checkout--x",
+        sync_args=["my_git_repo", "-x"],
+        expected_exit_code=0,
+        expected_in_output="Already on 'master'",
+    ),
+    SyncBrokenFixture(
+        test_id="normal-first-broken",
+        sync_args=["non_existent_repo", "my_git_repo"],
+        expected_exit_code=0,
+        expected_not_in_output=EXIT_ON_ERROR_MSG,
+    ),
+    SyncBrokenFixture(
+        test_id="normal-last-broken",
+        sync_args=["my_git_repo", "non_existent_repo"],
+        expected_exit_code=0,
+        expected_not_in_output=EXIT_ON_ERROR_MSG,
+    ),
+    SyncBrokenFixture(
+        test_id="exit-on-error--exit-on-error-first-broken",
+        sync_args=["non_existent_repo", "my_git_repo", "--exit-on-error"],
+        expected_exit_code=1,
+        expected_in_output=EXIT_ON_ERROR_MSG,
+    ),
+    SyncBrokenFixture(
+        test_id="exit-on-error--x-first-broken",
+        sync_args=["non_existent_repo", "my_git_repo", "-x"],
+        expected_exit_code=1,
+        expected_in_output=EXIT_ON_ERROR_MSG,
+        expected_not_in_output="master",
+    ),
+    #
+    # Verify ordering
+    #
+    SyncBrokenFixture(
+        test_id="exit-on-error--exit-on-error-last-broken",
+        sync_args=["my_git_repo", "non_existent_repo", "-x"],
+        expected_exit_code=1,
+        expected_in_output=[EXIT_ON_ERROR_MSG, "Already on 'master'"],
+    ),
+    SyncBrokenFixture(
+        test_id="exit-on-error--x-last-item",
+        sync_args=["my_git_repo", "non_existent_repo", "--exit-on-error"],
+        expected_exit_code=1,
+        expected_in_output=[EXIT_ON_ERROR_MSG, "Already on 'master'"],
+    ),
+]
+
+
+@pytest.mark.parametrize(
+    list(SyncBrokenFixture._fields),
+    SYNC_BROKEN_REPO_FIXTURES,
+    ids=[test.test_id for test in SYNC_BROKEN_REPO_FIXTURES],
+)
+def test_sync_broken(
+    home_path: pathlib.Path,
+    config_path: pathlib.Path,
+    tmp_path: pathlib.Path,
+    git_repo: GitSync,
+    test_id: str,
+    sync_args: list[str],
+    expected_exit_code: int,
+    expected_in_output: "ExpectedOutput",
+    expected_not_in_output: "ExpectedOutput",
+) -> None:
     runner = CliRunner()
-    result = runner.invoke(cli, ["sync", "hi"])
-    assert result.exit_code == 0
-    assert "Debug mode is on" in result.output
-    assert "Syncing" in result.output
+
+    github_projects = home_path / "github_projects"
+    my_git_repo = github_projects / "my_git_repo"
+    if my_git_repo.is_dir():
+        my_git_repo.rmdir()
+
+    with runner.isolated_filesystem(temp_dir=tmp_path):
+        config = {
+            "~/github_projects/": {
+                "my_git_repo": {
+                    "url": f"git+file://{git_repo.dir}",
+                    "remotes": {"test_remote": f"git+file://{git_repo.dir}"},
+                },
+                "non_existent_repo": {
+                    "url": "git+file:///dev/null",
+                },
+            }
+        }
+        yaml_config = config_path / ".vcspull.yaml"
+        yaml_config_data = yaml.dump(config, default_flow_style=False)
+        yaml_config.write_text(yaml_config_data, encoding="utf-8")
+
+        # CLI can sync
+        assert isinstance(sync_args, list)
+        result = runner.invoke(cli, ["sync", *sync_args])
+        assert result.exit_code == expected_exit_code
+        output = "".join(list(result.output))
+
+        if expected_in_output is not None:
+            if isinstance(expected_in_output, str):
+                expected_in_output = [expected_in_output]
+            for needle in expected_in_output:
+                assert needle in output
+
+        if expected_not_in_output is not None:
+            if isinstance(expected_not_in_output, str):
+                expected_not_in_output = [expected_not_in_output]
+            for needle in expected_not_in_output:
+                assert needle not in output