diff --git a/CHANGES b/CHANGES index f0f687b29..ccfea4452 100644 --- a/CHANGES +++ b/CHANGES @@ -38,6 +38,34 @@ $ uvx --from 'vcspull' --prerelease allow vcspull _Notes on upcoming releases will be added here_ +### Breaking changes + +#### Exceptions renamed with an `Error` suffix (#420) + +Two public exceptions were renamed to match the `Error`-suffix naming +convention (pep8-naming `N818`), with no aliases kept: + +- {exc}`~vcspull.exc.VCSPullError` — the base every vcspull error derives + from (was `VCSPullException`). +- {exc}`~vcspull.exc.MultipleConfigError` — raised when the home directory + holds more than one eligible config file (was `MultipleConfigWarning`; + it was always raised as a hard error, never a warning). + +Update `except` clauses accordingly: + +```python +# Before +except vcspull.exc.VCSPullException: + ... + +# After +except vcspull.exc.VCSPullError: + ... +``` + +Other subclasses ({exc}`~vcspull.exc.WorktreeError` and friends) keep their +names. + ## vcspull v1.65.0 (2026-07-05) vcspull v1.65.0 sharpens the feedback you get when a sync cannot check out diff --git a/conftest.py b/conftest.py index 9a5b07cb7..8191c72ca 100644 --- a/conftest.py +++ b/conftest.py @@ -69,17 +69,12 @@ def xdg_config_path( @pytest.fixture def config_path( xdg_config_path: pathlib.Path, - request: pytest.FixtureRequest, -) -> pathlib.Path: +) -> t.Generator[pathlib.Path, None, None]: """Ensure and return vcspull configuration path.""" conf_path = xdg_config_path / "vcspull" conf_path.mkdir(exist_ok=True) - - def clean() -> None: - shutil.rmtree(conf_path) - - request.addfinalizer(clean) - return conf_path + yield conf_path + shutil.rmtree(conf_path) @pytest.fixture(autouse=True) @@ -92,16 +87,14 @@ def set_xdg_config_path( @pytest.fixture -def repos_path(user_path: pathlib.Path, request: pytest.FixtureRequest) -> pathlib.Path: +def repos_path( + user_path: pathlib.Path, +) -> t.Generator[pathlib.Path, None, None]: """Return temporary directory for repository checkout guaranteed unique.""" path = user_path / "repos" path.mkdir(exist_ok=True) - - def clean() -> None: - shutil.rmtree(path) - - request.addfinalizer(clean) - return path + yield path + shutil.rmtree(path) def pytest_collection_modifyitems( diff --git a/docs/api/exc.md b/docs/api/exc.md index e91d230f8..0168314f3 100644 --- a/docs/api/exc.md +++ b/docs/api/exc.md @@ -2,7 +2,7 @@ When configuration loading or a worktree operation fails, vcspull raises an exception from {mod}`vcspull.exc`. Catch -{exc}`~vcspull.exc.VCSPullException` to handle any of them; the generated +{exc}`~vcspull.exc.VCSPullError` to handle any of them; the generated reference below lists the specific subclasses. ```{eval-rst} diff --git a/pyproject.toml b/pyproject.toml index b3e674783..4cb8c0fea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -214,9 +214,11 @@ select = [ "F", # pyflakes "I", # isort "UP", # pyupgrade + "N", # pep8-naming "A", # flake8-builtins "B", # flake8-bugbear "C4", # flake8-comprehensions + "ICN", # flake8-import-conventions "COM", # flake8-commas "EM", # flake8-errmsg "Q", # flake8-quotes @@ -224,6 +226,7 @@ select = [ "SIM", # flake8-simplify "TRY", # Trycertatops "PERF", # Perflint + "PT", # flake8-pytest-style "RUF", # Ruff-specific rules "D", # pydocstyle "FA100", # future annotations diff --git a/src/vcspull/cli/import_cmd/_common.py b/src/vcspull/cli/import_cmd/_common.py index b1d050abc..92af5c75b 100644 --- a/src/vcspull/cli/import_cmd/_common.py +++ b/src/vcspull/cli/import_cmd/_common.py @@ -40,7 +40,7 @@ save_config, workspace_root_label, ) -from vcspull.exc import MultipleConfigWarning +from vcspull.exc import MultipleConfigError from .._colors import Colors, get_color_mode from .._output import OutputFormatter, get_output_mode @@ -774,7 +774,7 @@ def _run_import( # Resolve config file try: config_file_path = _resolve_config_file(config_path_str) - except (ValueError, MultipleConfigWarning) as exc_: + except (ValueError, MultipleConfigError) as exc_: log.error("%s %s", colors.error("✗"), exc_) # noqa: TRY400 return 1 display_config_path = str(PrivatePath(config_file_path)) diff --git a/src/vcspull/cli/sync.py b/src/vcspull/cli/sync.py index 73ec66ca7..d11843dbd 100644 --- a/src/vcspull/cli/sync.py +++ b/src/vcspull/cli/sync.py @@ -2026,14 +2026,14 @@ def guess_vcs(url: str) -> VCSLiteral | None: return t.cast("VCSLiteral", vcs_matches[0].vcs) -class CouldNotGuessVCSFromURL(exc.VCSPullException): +class CouldNotGuessVCSFromURL(exc.VCSPullError): """Raised when no VCS could be guessed from a URL.""" def __init__(self, repo_url: str, *args: object, **kwargs: object) -> None: return super().__init__(f"Could not automatically determine VCS for {repo_url}") -class SyncFailedError(exc.VCSPullException): +class SyncFailedError(exc.VCSPullError): """Raised when a sync operation completes but with errors.""" def __init__( diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 6697236df..6ce866fc5 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -121,7 +121,7 @@ def _validate_worktrees_config( Raises ------ - VCSPullException + VCSPullError If the worktrees configuration is invalid. Examples @@ -157,35 +157,35 @@ def _validate_worktrees_config( >>> _validate_worktrees_config("not-a-list", "myrepo") Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...worktrees must be a list, got str + vcspull.exc.VCSPullError: ...worktrees must be a list, got str Error: worktree entry must be a dict: >>> _validate_worktrees_config(["not-a-dict"], "myrepo") Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...must be a dict, got str + vcspull.exc.VCSPullError: ...must be a dict, got str Error: missing required 'dir' field: >>> _validate_worktrees_config([{"tag": "v1.0.0"}], "myrepo") Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...missing required 'dir' field + vcspull.exc.VCSPullError: ...missing required 'dir' field Error: no ref type specified: >>> _validate_worktrees_config([{"dir": "../wt"}], "myrepo") Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...must specify one of: tag, branch, or commit + vcspull.exc.VCSPullError: ...must specify one of: tag, branch, or commit Error: empty ref value: >>> _validate_worktrees_config([{"dir": "../wt", "tag": ""}], "myrepo") Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...empty ref value... + vcspull.exc.VCSPullError: ...empty ref value... Error: multiple refs specified: @@ -194,14 +194,14 @@ def _validate_worktrees_config( ... ) Traceback (most recent call last): ... - vcspull.exc.VCSPullException: ...cannot specify multiple refs... + vcspull.exc.VCSPullError: ...cannot specify multiple refs... """ if not isinstance(worktrees_raw, list): msg = ( f"Repository '{repo_name}': worktrees must be a list, " f"got {type(worktrees_raw).__name__}" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) validated: list[WorktreeConfigDict] = [] @@ -211,7 +211,7 @@ def _validate_worktrees_config( f"Repository '{repo_name}': worktree entry {idx} must be a dict, " f"got {type(wt).__name__}" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) # Validate required 'dir' field if "dir" not in wt or not wt["dir"]: @@ -219,14 +219,14 @@ def _validate_worktrees_config( f"Repository '{repo_name}': worktree entry {idx} " "missing required 'dir' field" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) if not isinstance(wt["dir"], str): msg = ( f"Repository '{repo_name}': worktree entry {idx} " f"'dir' must be a string, got {type(wt['dir']).__name__}" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) # Validate exactly one ref type tag = wt.get("tag") @@ -243,19 +243,19 @@ def _validate_worktrees_config( f"Repository '{repo_name}': worktree entry {idx} " "must specify one of: tag, branch, or commit" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) if refs_specified == 0 and empty_refs > 0: msg = ( f"Repository '{repo_name}': worktree entry {idx} " "has empty ref value (tag, branch, or commit)" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) if refs_specified > 1: msg = ( f"Repository '{repo_name}': worktree entry {idx} " "cannot specify multiple refs (tag, branch, commit)" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) # Validate ref types are strings for ref_name, ref_val in [("tag", tag), ("branch", branch), ("commit", commit)]: @@ -264,7 +264,7 @@ def _validate_worktrees_config( f"Repository '{repo_name}': worktree entry {idx} " f"'{ref_name}' must be a string, got {type(ref_val).__name__}" ) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) # Build validated worktree config wt_config: WorktreeConfigDict = {"dir": wt["dir"]} @@ -447,7 +447,7 @@ def find_home_config_files( ) else: if sum(filter(None, [has_json_config, has_yaml_config])) > 1: - raise exc.MultipleConfigWarning + raise exc.MultipleConfigError if has_yaml_config: configs.append(yaml_config) if has_json_config: @@ -616,7 +616,7 @@ def load_configs( if len(dupes) > 0: msg = ("repos with same path + different VCS detected!", dupes) - raise exc.VCSPullException(msg) + raise exc.VCSPullError(msg) repos.extend(newrepos) return repos diff --git a/src/vcspull/exc.py b/src/vcspull/exc.py index 0f11d9884..7c5359bf4 100644 --- a/src/vcspull/exc.py +++ b/src/vcspull/exc.py @@ -3,17 +3,17 @@ from __future__ import annotations -class VCSPullException(Exception): +class VCSPullError(Exception): """Standard exception raised by vcspull.""" -class MultipleConfigWarning(VCSPullException): +class MultipleConfigError(VCSPullError): """Multiple eligible config files found at the same time.""" message = "Multiple configs found in home directory use only one. .yaml, .json." -class WorktreeError(VCSPullException): +class WorktreeError(VCSPullError): """Base exception for worktree operations.""" diff --git a/src/vcspull/log.py b/src/vcspull/log.py index beb9b3c04..656de6c88 100644 --- a/src/vcspull/log.py +++ b/src/vcspull/log.py @@ -295,7 +295,7 @@ def template(self, record: logging.LogRecord) -> str: Style.RESET_ALL, " ", ] - module_funcName = [Fore.GREEN, Style.BRIGHT, "%(module)s.%(funcName)s()"] + module_func_name = [Fore.GREEN, Style.BRIGHT, "%(module)s.%(funcName)s()"] lineno = [ Fore.BLACK, Style.DIM, @@ -307,7 +307,7 @@ def template(self, record: logging.LogRecord) -> str: ] return "".join( - reset + levelname + asctime + name + module_funcName + lineno + reset, + reset + levelname + asctime + name + module_func_name + lineno + reset, ) diff --git a/tests/cli/test_import_repos.py b/tests/cli/test_import_repos.py index 3aa6e6d4e..ba9a4b527 100644 --- a/tests/cli/test_import_repos.py +++ b/tests/cli/test_import_repos.py @@ -1419,8 +1419,8 @@ def test_import_repos_catches_multiple_config_warning( monkeypatch: MonkeyPatch, caplog: pytest.LogCaptureFixture, ) -> None: - """Test _run_import logs error instead of crashing on MultipleConfigWarning.""" - from vcspull.exc import MultipleConfigWarning + """Test _run_import logs error instead of crashing on MultipleConfigError.""" + from vcspull.exc import MultipleConfigError caplog.set_level(logging.ERROR) @@ -1430,9 +1430,9 @@ def test_import_repos_catches_multiple_config_warning( importer = MockImporter(repos=[_make_repo("repo1")]) - # Mock _resolve_config_file: raise MultipleConfigWarning to test error handling + # Mock _resolve_config_file: raise MultipleConfigError to test error handling def raise_multiple_config(_: str | None) -> pathlib.Path: - raise MultipleConfigWarning(MultipleConfigWarning.message) + raise MultipleConfigError(MultipleConfigError.message) monkeypatch.setattr( import_common_mod, diff --git a/tests/cli/test_sync_sigint.py b/tests/cli/test_sync_sigint.py index 569d1dd5c..5424c1f72 100644 --- a/tests/cli/test_sync_sigint.py +++ b/tests/cli/test_sync_sigint.py @@ -66,7 +66,8 @@ def test_exit_on_sigint_produces_wifsignaled_sigint() -> None: # vcspull. Prepending the parent's resolved package dir keeps the child # importable regardless of the surrounding install style. vcspull_spec = importlib.util.find_spec("vcspull") - assert vcspull_spec is not None and vcspull_spec.origin is not None + assert vcspull_spec is not None + assert vcspull_spec.origin is not None vcspull_parent = str(pathlib.Path(vcspull_spec.origin).resolve().parent.parent) env = { **os.environ, diff --git a/tests/cli/test_sync_watchdog.py b/tests/cli/test_sync_watchdog.py index 695cb376e..3931bb818 100644 --- a/tests/cli/test_sync_watchdog.py +++ b/tests/cli/test_sync_watchdog.py @@ -127,14 +127,14 @@ def test_watchdog_preserves_failed_outcome( ) -> None: """Synchronous exceptions from ``update_repo`` surface as ``failed``.""" - class _Boom(RuntimeError): + class _BoomError(RuntimeError): """Sentinel used to trace exception propagation.""" def _raising_update_repo( repo: dict[str, t.Any], *, progress_callback: t.Any ) -> None: msg = "remote exploded" - raise _Boom(msg) + raise _BoomError(msg) monkeypatch.setattr(sync_module, "update_repo", _raising_update_repo) @@ -146,7 +146,7 @@ def _raising_update_repo( ) assert outcome.status == "failed" - assert isinstance(outcome.error, _Boom) + assert isinstance(outcome.error, _BoomError) assert "remote exploded" in str(outcome.error) @@ -196,7 +196,8 @@ def test_rerun_recipe_emits_one_line_per_workspace( # One rerun command per distinct workspace root, with the repo names # appended as positional args -- this is what the user copy-pastes. assert "vcspull sync --workspace" in captured - assert "codex" in captured and "rust" in captured + assert "codex" in captured + assert "rust" in captured assert "opentelemetry-rust" in captured # Suggest 10x the current timeout, clamped to 120 s minimum. assert "--timeout 120" in captured @@ -306,10 +307,10 @@ def _fake() -> t.NoReturn: monkeypatch.setattr(sync_module, "_exit_on_sigint", _fake) +@pytest.mark.usefixtures("_fake_sigint_escalation") def test_sync_handles_keyboard_interrupt_during_config_load( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], - _fake_sigint_escalation: None, ) -> None: """Ctrl-C during pre-loop work (e.g. YAML parse) exits cleanly with 130. diff --git a/tests/test_config_file.py b/tests/test_config_file.py index b51fbc80f..b14713464 100644 --- a/tests/test_config_file.py +++ b/tests/test_config_file.py @@ -203,11 +203,12 @@ def test_multiple_config_files_raises_exception(tmp_path: pathlib.Path) -> None: json_conf_file.touch() yaml_conf_file = tmp_path / ".vcspull.yaml" yaml_conf_file.touch() - with EnvironmentVarGuard() as env, pytest.raises(exc.MultipleConfigWarning): + with EnvironmentVarGuard() as env: env.set("HOME", str(tmp_path)) assert pathlib.Path.home() == tmp_path - config.find_home_config_files() + with pytest.raises(exc.MultipleConfigError): + config.find_home_config_files() def test_find_home_config_files_filetype_yaml_only(tmp_path: pathlib.Path) -> None: @@ -216,7 +217,7 @@ def test_find_home_config_files_filetype_yaml_only(tmp_path: pathlib.Path) -> No (tmp_path / ".vcspull.json").touch() with EnvironmentVarGuard() as env: env.set("HOME", str(tmp_path)) - # Should NOT raise MultipleConfigWarning because json is filtered out + # Should NOT raise MultipleConfigError because json is filtered out results = config.find_home_config_files(filetype=["yaml"]) assert len(results) == 1 assert results[0].suffix == ".yaml" @@ -236,12 +237,13 @@ def test_find_home_config_files_filetype_json_only(tmp_path: pathlib.Path) -> No def test_find_home_config_files_both_types_still_raises( tmp_path: pathlib.Path, ) -> None: - """Default filetype still raises MultipleConfigWarning when both exist.""" + """Default filetype still raises MultipleConfigError when both exist.""" (tmp_path / ".vcspull.yaml").touch() (tmp_path / ".vcspull.json").touch() - with EnvironmentVarGuard() as env, pytest.raises(exc.MultipleConfigWarning): + with EnvironmentVarGuard() as env: env.set("HOME", str(tmp_path)) - config.find_home_config_files() + with pytest.raises(exc.MultipleConfigError): + config.find_home_config_files() def test_find_home_config_files_preserves_symlink_suffix( @@ -505,5 +507,5 @@ def test_merge_nested_dict(tmp_path: pathlib.Path, config_path: pathlib.Path) -> ) assert config1 in config_files assert config2 in config_files - with pytest.raises(exc.VCSPullException): + with pytest.raises(exc.VCSPullError): config.load_configs(config_files) diff --git a/tests/test_worktree.py b/tests/test_worktree.py index 243c11ecf..77be85ec8 100644 --- a/tests/test_worktree.py +++ b/tests/test_worktree.py @@ -126,8 +126,8 @@ def test_worktree_config_parsing( } if expected_error: - with pytest.raises(exc.VCSPullException, match=expected_error): - typed_config = t.cast("RawConfigDict", full_config) + typed_config = t.cast("RawConfigDict", full_config) + with pytest.raises(exc.VCSPullError, match=expected_error): vcspull_config.extract_repos(typed_config, cwd=tmp_path) else: # Validate each worktree individually @@ -285,9 +285,9 @@ def test_worktree_exception_construction( def test_worktree_exception_hierarchy() -> None: """Test worktree exceptions inherit from correct base classes.""" assert issubclass(exc.WorktreeRefNotFoundError, exc.WorktreeError) - assert issubclass(exc.WorktreeRefNotFoundError, exc.VCSPullException) + assert issubclass(exc.WorktreeRefNotFoundError, exc.VCSPullError) assert issubclass(exc.WorktreeDirtyError, exc.WorktreeError) - assert issubclass(exc.WorktreeDirtyError, exc.VCSPullException) + assert issubclass(exc.WorktreeDirtyError, exc.VCSPullError) # --------------------------------------------------------------------------- @@ -1026,8 +1026,8 @@ def test_extract_repos_worktrees_validation_error(tmp_path: pathlib.Path) -> Non }, } - with pytest.raises(exc.VCSPullException, match="must specify one of"): - typed_config = t.cast("RawConfigDict", raw_config) + typed_config = t.cast("RawConfigDict", raw_config) + with pytest.raises(exc.VCSPullError, match="must specify one of"): vcspull_config.extract_repos(typed_config, cwd=tmp_path) @@ -2044,7 +2044,7 @@ def test_validate_empty_string_refs() -> None: ) # config-level validation should also reject empty refs - with pytest.raises(exc.VCSPullException, match="empty ref value"): + with pytest.raises(exc.VCSPullError, match="empty ref value"): vcspull_config._validate_worktrees_config( [{"dir": "../wt", "branch": ""}], "myrepo" )