diff --git a/CHANGES b/CHANGES index 9dd1b1fd..2c0a4ac5 100644 --- a/CHANGES +++ b/CHANGES @@ -33,6 +33,24 @@ $ uvx --from 'vcspull' --prerelease allow vcspull _Upcoming changes will be written here._ +### Improvements + +#### `vcspull fmt` gains duplicate root merging (#479) + +- Detects repeated workspace root labels and merges their repositories during + formatting so users keep a single normalized section. +- Adds `--no-merge` for workflows that need to preserve duplicate entries while + still seeing diagnostic warnings. + +### Development + +#### Snapshot coverage for formatter tests (#479) + +- Formatter scenarios now use [Syrupy]-backed JSON and YAML snapshots to guard + against regressions in duplicate workspace-root merging. + +[syrupy]: https://github.com/syrupy-project/syrupy + ## vcspull v1.42.0 (2025-11-01) ### Breaking changes diff --git a/pyproject.toml b/pyproject.toml index 489e300a..3bd9b43d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,6 +85,7 @@ dev = [ # Testing "gp-libs", "pytest", + "syrupy", "pytest-asyncio", "pytest-rerunfailures", "pytest-mock", @@ -119,6 +120,7 @@ docs = [ testing = [ "gp-libs", "pytest", + "syrupy", "pytest-asyncio", "pytest-rerunfailures", "pytest-mock", diff --git a/src/vcspull/cli/__init__.py b/src/vcspull/cli/__init__.py index 4cb8c212..199f5875 100644 --- a/src/vcspull/cli/__init__.py +++ b/src/vcspull/cli/__init__.py @@ -386,4 +386,9 @@ def cli(_args: list[str] | None = None) -> None: dry_run=args.dry_run, ) elif args.subparser_name == "fmt": - format_config_file(args.config, args.write, args.all) + format_config_file( + args.config, + args.write, + args.all, + merge_roots=args.merge_roots, + ) diff --git a/src/vcspull/cli/fmt.py b/src/vcspull/cli/fmt.py index 41552e48..1ac60078 100644 --- a/src/vcspull/cli/fmt.py +++ b/src/vcspull/cli/fmt.py @@ -2,11 +2,13 @@ from __future__ import annotations +import copy import logging import pathlib import traceback import typing as t +import yaml from colorama import Fore, Style from vcspull._internal.config_reader import ConfigReader @@ -23,6 +25,149 @@ log = logging.getLogger(__name__) +class _DuplicateTrackingSafeLoader(yaml.SafeLoader): + """PyYAML loader that records duplicate top-level keys.""" + + def __init__(self, stream: str) -> None: + super().__init__(stream) + self.top_level_key_values: dict[t.Any, list[t.Any]] = {} + self._mapping_depth = 0 + + +def _duplicate_tracking_construct_mapping( + loader: _DuplicateTrackingSafeLoader, + node: yaml.nodes.MappingNode, + deep: bool = False, +) -> dict[t.Any, t.Any]: + loader._mapping_depth += 1 + loader.flatten_mapping(node) + mapping: dict[t.Any, t.Any] = {} + + for key_node, value_node in node.value: + construct = t.cast( + t.Callable[[yaml.nodes.Node], t.Any], + loader.construct_object, + ) + key = construct(key_node) + value = construct(value_node) + + if loader._mapping_depth == 1: + loader.top_level_key_values.setdefault(key, []).append(copy.deepcopy(value)) + + mapping[key] = value + + loader._mapping_depth -= 1 + return mapping + + +_DuplicateTrackingSafeLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, + _duplicate_tracking_construct_mapping, +) + + +def _load_yaml_config_with_duplicates( + config_file_path: pathlib.Path, +) -> tuple[dict[str, t.Any], dict[str, list[t.Any]]]: + content = config_file_path.read_text(encoding="utf-8") + loader = _DuplicateTrackingSafeLoader(content) + + try: + data = loader.get_single_data() + finally: + dispose = t.cast(t.Callable[[], None], loader.dispose) + dispose() + + if data is None: + return {}, {} + if not isinstance(data, dict): + msg = f"Config file {config_file_path} is not a valid YAML dictionary." + raise TypeError(msg) + + duplicates = { + t.cast(str, key): values + for key, values in loader.top_level_key_values.items() + if len(values) > 1 + } + + return t.cast("dict[str, t.Any]", data), duplicates + + +def _load_config_with_duplicate_roots( + config_file_path: pathlib.Path, +) -> tuple[dict[str, t.Any], dict[str, list[t.Any]]]: + if config_file_path.suffix.lower() in {".yaml", ".yml"}: + return _load_yaml_config_with_duplicates(config_file_path) + + return ConfigReader._from_file(config_file_path), {} + + +def _merge_duplicate_workspace_root_entries( + label: str, + occurrences: list[t.Any], +) -> tuple[t.Any, list[str], int]: + conflicts: list[str] = [] + change_count = max(len(occurrences) - 1, 0) + + if not occurrences: + return {}, conflicts, change_count + + if not all(isinstance(entry, dict) for entry in occurrences): + conflicts.append( + ( + f"Workspace root '{label}' contains duplicate entries that are not " + "mappings. Keeping the last occurrence." + ), + ) + return occurrences[-1], conflicts, change_count + + merged: dict[str, t.Any] = {} + + for entry in occurrences: + assert isinstance(entry, dict) + for repo_name, repo_config in entry.items(): + if repo_name not in merged: + merged[repo_name] = copy.deepcopy(repo_config) + elif merged[repo_name] != repo_config: + conflicts.append( + ( + f"Workspace root '{label}' contains conflicting definitions " + f"for repository '{repo_name}'. Keeping the existing entry." + ), + ) + + return merged, conflicts, change_count + + +def _merge_duplicate_workspace_roots( + config_data: dict[str, t.Any], + duplicate_roots: dict[str, list[t.Any]], +) -> tuple[dict[str, t.Any], list[str], int, list[tuple[str, int]]]: + if not duplicate_roots: + return config_data, [], 0, [] + + merged_config = copy.deepcopy(config_data) + conflicts: list[str] = [] + change_count = 0 + details: list[tuple[str, int]] = [] + + for label, occurrences in duplicate_roots.items(): + ( + merged_value, + entry_conflicts, + entry_changes, + ) = _merge_duplicate_workspace_root_entries( + label, + occurrences, + ) + merged_config[label] = merged_value + conflicts.extend(entry_conflicts) + change_count += entry_changes + details.append((label, len(occurrences))) + + return merged_config, conflicts, change_count, details + + def create_fmt_subparser(parser: argparse.ArgumentParser) -> None: """Create ``vcspull fmt`` argument subparser.""" parser.add_argument( @@ -43,6 +188,13 @@ def create_fmt_subparser(parser: argparse.ArgumentParser) -> None: action="store_true", help="Format all discovered config files (home, config dir, and current dir)", ) + parser.add_argument( + "--no-merge", + dest="merge_roots", + action="store_false", + help="Do not merge duplicate workspace roots when formatting", + ) + parser.set_defaults(merge_roots=True) def normalize_repo_config(repo_data: t.Any) -> dict[str, t.Any]: @@ -130,6 +282,8 @@ def format_config(config_data: dict[str, t.Any]) -> tuple[dict[str, t.Any], int] def format_single_config( config_file_path: pathlib.Path, write: bool, + *, + merge_roots: bool, ) -> bool: """Format a single vcspull configuration file. @@ -139,6 +293,8 @@ def format_single_config( Path to config file write : bool Whether to write changes back to file + merge_roots : bool + Merge duplicate workspace roots when True (default behavior) Returns ------- @@ -159,13 +315,18 @@ def format_single_config( # Load existing config try: - raw_config = ConfigReader._from_file(config_file_path) + raw_config, duplicate_root_occurrences = _load_config_with_duplicate_roots( + config_file_path, + ) if not isinstance(raw_config, dict): log.error( "Config file %s is not a valid YAML dictionary.", config_file_path, ) return False + except TypeError: + log.exception("Invalid configuration in %s", config_file_path) + return False except Exception: log.exception("Error loading config from %s", config_file_path) if log.isEnabledFor(logging.DEBUG): @@ -176,18 +337,62 @@ def format_single_config( cwd = pathlib.Path.cwd() home = pathlib.Path.home() - normalization_result = normalize_workspace_roots( - raw_config, - cwd=cwd, - home=home, - ) - raw_config, _workspace_map, merge_conflicts, merge_changes = normalization_result + duplicate_merge_conflicts: list[str] = [] + duplicate_merge_changes = 0 + duplicate_merge_details: list[tuple[str, int]] = [] + + working_config = copy.deepcopy(raw_config) + + if merge_roots: + ( + working_config, + duplicate_merge_conflicts, + duplicate_merge_changes, + duplicate_merge_details, + ) = _merge_duplicate_workspace_roots(working_config, duplicate_root_occurrences) + elif duplicate_root_occurrences: + duplicate_merge_details = [ + (label, len(values)) for label, values in duplicate_root_occurrences.items() + ] + for label, occurrence_count in duplicate_merge_details: + log.warning( + "%s•%s Duplicate workspace root %s%s%s appears %s%d%s time%s; " + "skipping merge because --no-merge was provided.", + Fore.BLUE, + Style.RESET_ALL, + Fore.MAGENTA, + label, + Style.RESET_ALL, + Fore.YELLOW, + occurrence_count, + Style.RESET_ALL, + "" if occurrence_count == 1 else "s", + ) + + if merge_roots: + normalization_result = normalize_workspace_roots( + working_config, + cwd=cwd, + home=home, + ) + ( + normalized_config, + _workspace_map, + merge_conflicts, + normalization_changes, + ) = normalization_result + else: + normalized_config = working_config + merge_conflicts = [] + normalization_changes = 0 for message in merge_conflicts: log.warning(message) + for message in duplicate_merge_conflicts: + log.warning(message) - formatted_config, change_count = format_config(raw_config) - change_count += merge_changes + formatted_config, change_count = format_config(normalized_config) + change_count += normalization_changes + duplicate_merge_changes if change_count == 0: log.info( @@ -215,17 +420,32 @@ def format_single_config( ) # Analyze and report specific changes - if merge_changes > 0: + if merge_roots and normalization_changes > 0: log.info( " %s•%s Normalized workspace root labels", Fore.BLUE, Style.RESET_ALL, ) + if merge_roots and duplicate_merge_details: + for label, occurrence_count in duplicate_merge_details: + log.info( + " %s•%s Merged %s%d%s duplicate entr%s for workspace root %s%s%s", + Fore.BLUE, + Style.RESET_ALL, + Fore.YELLOW, + occurrence_count, + Style.RESET_ALL, + "y" if occurrence_count == 1 else "ies", + Fore.MAGENTA, + label, + Style.RESET_ALL, + ) + compact_to_verbose = 0 url_to_repo = 0 - for repos in raw_config.values(): + for repos in normalized_config.values(): if isinstance(repos, dict): for repo_data in repos.values(): if isinstance(repo_data, str): @@ -255,7 +475,7 @@ def format_single_config( "repository" if url_to_repo == 1 else "repositories", ) - if list(raw_config.keys()) != sorted(raw_config.keys()): + if list(normalized_config.keys()) != sorted(normalized_config.keys()): log.info( " %s•%s Directories will be sorted alphabetically", Fore.BLUE, @@ -263,7 +483,7 @@ def format_single_config( ) # Check if any repos need sorting - for directory, repos in raw_config.items(): + for directory, repos in normalized_config.items(): if isinstance(repos, dict) and list(repos.keys()) != sorted(repos.keys()): log.info( " %s•%s Repositories in %s%s%s will be sorted alphabetically", @@ -308,6 +528,8 @@ def format_config_file( config_file_path_str: str | None, write: bool, format_all: bool = False, + *, + merge_roots: bool = True, ) -> None: """Format vcspull configuration file(s). @@ -319,6 +541,8 @@ def format_config_file( Whether to write changes back to file format_all : bool If True, format all discovered config files + merge_roots : bool + Merge duplicate workspace roots when True (default) """ if format_all: # Format all discovered config files @@ -366,7 +590,11 @@ def format_config_file( success_count = 0 for config_file in config_files: - if format_single_config(config_file, write): + if format_single_config( + config_file, + write, + merge_roots=merge_roots, + ): success_count += 1 # Summary @@ -413,4 +641,8 @@ def format_config_file( else: config_file_path = home_configs[0] - format_single_config(config_file_path, write) + format_single_config( + config_file_path, + write, + merge_roots=merge_roots, + ) diff --git a/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[merge-default][fmt-cli-merge].yaml b/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[merge-default][fmt-cli-merge].yaml new file mode 100644 index 00000000..8e27120b --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[merge-default][fmt-cli-merge].yaml @@ -0,0 +1,5 @@ +~/study/c/: + repo1: + repo: url1 + repo2: + repo: url2 diff --git a/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[no-merge][fmt-cli-no-merge].yaml b/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[no-merge][fmt-cli-no-merge].yaml new file mode 100644 index 00000000..176c8a90 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_fmt_cli_integration[no-merge][fmt-cli-no-merge].yaml @@ -0,0 +1,3 @@ +~/study/c/: + repo2: + repo: url2 diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_complex_changes.json b/tests/cli/__snapshots__/test_fmt/test_format_config_complex_changes.json new file mode 100644 index 00000000..39df33d8 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_complex_changes.json @@ -0,0 +1,21 @@ +{ + "~/aaa/": { + "repo1": { + "repo": "another-compact" + } + }, + "~/zzz/": { + "alpha": { + "repo": "verbose-url" + }, + "beta": { + "remotes": { + "upstream": "upstream-url" + }, + "repo": "already-good" + }, + "zebra": { + "repo": "compact-url" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_converts_compact_entries.json b/tests/cli/__snapshots__/test_fmt/test_format_config_converts_compact_entries.json new file mode 100644 index 00000000..bdc0c21c --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_converts_compact_entries.json @@ -0,0 +1,13 @@ +{ + "~/projects/": { + "repo1": { + "repo": "git+https://github.com/user/repo1.git" + }, + "repo2": { + "repo": "git+https://github.com/user/repo2.git" + }, + "repo3": { + "repo": "git+https://github.com/user/repo3.git" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_file_with_write[format-config-file-with-write].yaml b/tests/cli/__snapshots__/test_fmt/test_format_config_file_with_write[format-config-file-with-write].yaml new file mode 100644 index 00000000..c8c9937a --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_file_with_write[format-config-file-with-write].yaml @@ -0,0 +1,5 @@ +~/zzz/: + repo1: + repo: url1 + repo2: + repo: url2 diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_merges_duplicate_roots_when_writing[format-config-file-merge].yaml b/tests/cli/__snapshots__/test_fmt/test_format_config_merges_duplicate_roots_when_writing[format-config-file-merge].yaml new file mode 100644 index 00000000..8e27120b --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_merges_duplicate_roots_when_writing[format-config-file-merge].yaml @@ -0,0 +1,5 @@ +~/study/c/: + repo1: + repo: url1 + repo2: + repo: url2 diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_no_changes_when_already_normalized.json b/tests/cli/__snapshots__/test_fmt/test_format_config_no_changes_when_already_normalized.json new file mode 100644 index 00000000..8105e421 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_no_changes_when_already_normalized.json @@ -0,0 +1,15 @@ +{ + "~/aaa/": { + "alpha": { + "repo": "url1" + }, + "beta": { + "repo": "url2" + } + }, + "~/bbb/": { + "charlie": { + "repo": "url3" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_no_merge_flag_skips_duplicate_merge[format-config-file-no-merge].yaml b/tests/cli/__snapshots__/test_fmt/test_format_config_no_merge_flag_skips_duplicate_merge[format-config-file-no-merge].yaml new file mode 100644 index 00000000..176c8a90 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_no_merge_flag_skips_duplicate_merge[format-config-file-no-merge].yaml @@ -0,0 +1,3 @@ +~/study/c/: + repo2: + repo: url2 diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_directories.json b/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_directories.json new file mode 100644 index 00000000..46c88ba4 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_directories.json @@ -0,0 +1,17 @@ +{ + "~/aaa/": { + "repo2": { + "repo": "url2" + } + }, + "~/mmm/": { + "repo3": { + "repo": "url3" + } + }, + "~/zzz/": { + "repo1": { + "repo": "url1" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_repositories.json b/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_repositories.json new file mode 100644 index 00000000..51eb7545 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_format_config_sorts_repositories.json @@ -0,0 +1,13 @@ +{ + "~/projects/": { + "alpha": { + "repo": "url2" + }, + "beta": { + "repo": "url3" + }, + "zebra": { + "repo": "url1" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[home-vs-absolute].json b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[home-vs-absolute].json new file mode 100644 index 00000000..ed99b593 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[home-vs-absolute].json @@ -0,0 +1,10 @@ +{ + "~/study/c/": { + "cpython": { + "repo": "git+https://github.com/python/cpython.git" + }, + "tmux": { + "repo": "git+https://github.com/tmux/tmux.git" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[relative-vs-tilde].json b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[relative-vs-tilde].json new file mode 100644 index 00000000..ed99b593 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[relative-vs-tilde].json @@ -0,0 +1,10 @@ +{ + "~/study/c/": { + "cpython": { + "repo": "git+https://github.com/python/cpython.git" + }, + "tmux": { + "repo": "git+https://github.com/tmux/tmux.git" + } + } +} diff --git a/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[tilde-mixed-trailing-slash].json b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[tilde-mixed-trailing-slash].json new file mode 100644 index 00000000..ed99b593 --- /dev/null +++ b/tests/cli/__snapshots__/test_fmt/test_workspace_root_normalization[tilde-mixed-trailing-slash].json @@ -0,0 +1,10 @@ +{ + "~/study/c/": { + "cpython": { + "repo": "git+https://github.com/python/cpython.git" + }, + "tmux": { + "repo": "git+https://github.com/tmux/tmux.git" + } + } +} diff --git a/tests/cli/test_fmt.py b/tests/cli/test_fmt.py index 7078adb7..6d4e23a4 100644 --- a/tests/cli/test_fmt.py +++ b/tests/cli/test_fmt.py @@ -2,13 +2,16 @@ from __future__ import annotations +import contextlib import logging import pathlib import typing as t import pytest import yaml +from syrupy.assertion import SnapshotAssertion +from vcspull.cli import cli from vcspull.cli.fmt import format_config, format_config_file, normalize_repo_config from vcspull.config import ( canonicalize_workspace_path, @@ -78,6 +81,7 @@ class WorkspaceRootFixture(t.NamedTuple): def test_workspace_root_normalization( test_id: str, config_factory: t.Callable[[pathlib.Path], dict[str, t.Any]], + snapshot_json: SnapshotAssertion, ) -> None: """Ensure format_config merges duplicate workspace roots.""" home_dir = pathlib.Path.home() @@ -100,6 +104,7 @@ def test_workspace_root_normalization( assert sorted(normalized_config.keys()) == expected_labels formatted, _changes = format_config(normalized_config) assert sorted(formatted.keys()) == expected_labels + assert formatted == snapshot_json assert merge_changes >= len(config) - len(canonical_paths) @@ -149,7 +154,7 @@ def test_normalize_repo_config_both_url_and_repo() -> None: assert normalized == config -def test_format_config_sorts_directories() -> None: +def test_format_config_sorts_directories(snapshot_json: SnapshotAssertion) -> None: """Workspace roots should be sorted alphabetically.""" config = { "~/zzz/": {"repo1": "url1"}, @@ -157,11 +162,11 @@ def test_format_config_sorts_directories() -> None: "~/mmm/": {"repo3": "url3"}, } formatted, changes = format_config(config) - assert list(formatted.keys()) == ["~/aaa/", "~/mmm/", "~/zzz/"] + assert formatted == snapshot_json assert changes > 0 -def test_format_config_sorts_repositories() -> None: +def test_format_config_sorts_repositories(snapshot_json: SnapshotAssertion) -> None: """Repositories within a workspace root should be sorted.""" config = { "~/projects/": { @@ -171,11 +176,13 @@ def test_format_config_sorts_repositories() -> None: }, } formatted, changes = format_config(config) - assert list(formatted["~/projects/"].keys()) == ["alpha", "beta", "zebra"] + assert formatted == snapshot_json assert changes > 0 -def test_format_config_converts_compact_entries() -> None: +def test_format_config_converts_compact_entries( + snapshot_json: SnapshotAssertion, +) -> None: """Compact repository entries should convert to verbose form.""" config = { "~/projects/": { @@ -185,19 +192,13 @@ def test_format_config_converts_compact_entries() -> None: }, } formatted, changes = format_config(config) - assert formatted["~/projects/"]["repo1"] == { - "repo": "git+https://github.com/user/repo1.git", - } - assert formatted["~/projects/"]["repo2"] == { - "repo": "git+https://github.com/user/repo2.git", - } - assert formatted["~/projects/"]["repo3"] == { - "repo": "git+https://github.com/user/repo3.git", - } + assert formatted == snapshot_json assert changes == 2 -def test_format_config_no_changes_when_already_normalized() -> None: +def test_format_config_no_changes_when_already_normalized( + snapshot_json: SnapshotAssertion, +) -> None: """Formatter should report zero changes for already-normalized configs.""" config = { "~/aaa/": { @@ -209,11 +210,11 @@ def test_format_config_no_changes_when_already_normalized() -> None: }, } formatted, changes = format_config(config) - assert formatted == config + assert formatted == snapshot_json assert changes == 0 -def test_format_config_complex_changes() -> None: +def test_format_config_complex_changes(snapshot_json: SnapshotAssertion) -> None: """Formatter should handle sorting and conversions together.""" config = { "~/zzz/": { @@ -229,13 +230,7 @@ def test_format_config_complex_changes() -> None: }, } formatted, changes = format_config(config) - assert list(formatted.keys()) == ["~/aaa/", "~/zzz/"] - assert list(formatted["~/zzz/"].keys()) == ["alpha", "beta", "zebra"] - assert formatted["~/aaa/"]["repo1"] == {"repo": "another-compact"} - assert formatted["~/zzz/"]["zebra"] == {"repo": "compact-url"} - assert formatted["~/zzz/"]["alpha"] == {"repo": "verbose-url"} - assert formatted["~/zzz/"]["beta"]["repo"] == "already-good" - assert formatted["~/zzz/"]["beta"]["remotes"] == {"upstream": "upstream-url"} + assert formatted == snapshot_json assert changes > 0 @@ -266,7 +261,10 @@ def test_format_config_file_without_write( assert "Run with --write to apply" in caplog.text -def test_format_config_file_with_write(tmp_path: pathlib.Path) -> None: +def test_format_config_file_with_write( + tmp_path: pathlib.Path, + snapshot_yaml: SnapshotAssertion, +) -> None: """format_config_file should rewrite file when write=True.""" config_file = tmp_path / ".vcspull.yaml" original_config = { @@ -279,8 +277,8 @@ def test_format_config_file_with_write(tmp_path: pathlib.Path) -> None: format_config_file(str(config_file), write=True, format_all=False) - saved_config = yaml.safe_load(config_file.read_text(encoding="utf-8")) - assert saved_config["~/zzz/"]["repo1"] == {"repo": "url1"} + yaml_text = config_file.read_text(encoding="utf-8") + assert yaml_text == snapshot_yaml(name="format-config-file-with-write") def test_format_config_file_invalid_yaml( @@ -404,3 +402,143 @@ def fake_find_home_config_files( assert "already formatted correctly" in text assert "Repositories in ~/work/ will be sorted alphabetically" in text assert "All 3 configuration files processed successfully" in text + + +def test_format_config_detects_and_merges_duplicate_roots( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, +) -> None: + """Duplicate workspace roots should be detected and merged by default.""" + config_file = tmp_path / ".vcspull.yaml" + config_file.write_text( + """~/study/c/: + repo1: url1 +~/study/c/: + repo2: url2 +""", + encoding="utf-8", + ) + + with caplog.at_level(logging.INFO): + format_config_file(str(config_file), write=False, format_all=False) + + text = caplog.text + assert "Merged" in text + assert "workspace root" in text + + +def test_format_config_no_merge_flag_skips_duplicate_merge( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, + snapshot_yaml: SnapshotAssertion, +) -> None: + """--no-merge should prevent duplicate workspace roots from being combined.""" + config_file = tmp_path / ".vcspull.yaml" + config_file.write_text( + """~/study/c/: + repo1: url1 +~/study/c/: + repo2: url2 +""", + encoding="utf-8", + ) + + with caplog.at_level(logging.WARNING): + format_config_file( + str(config_file), + write=True, + format_all=False, + merge_roots=False, + ) + + text = caplog.text + assert "skipping merge" in text + yaml_text = config_file.read_text(encoding="utf-8") + assert yaml_text == snapshot_yaml(name="format-config-file-no-merge") + + +def test_format_config_merges_duplicate_roots_when_writing( + tmp_path: pathlib.Path, + snapshot_yaml: SnapshotAssertion, +) -> None: + """When merging, formatted file should contain combined repositories.""" + config_file = tmp_path / ".vcspull.yaml" + config_file.write_text( + """~/study/c/: + repo1: url1 +~/study/c/: + repo2: url2 +""", + encoding="utf-8", + ) + + format_config_file(str(config_file), write=True, format_all=False) + + yaml_text = config_file.read_text(encoding="utf-8") + assert yaml_text == snapshot_yaml(name="format-config-file-merge") + + +class FmtIntegrationFixture(t.NamedTuple): + """Fixture for parametrized fmt CLI integration tests.""" + + test_id: str + cli_args: list[str] + expected_log_fragment: str + snapshot_name: str + + +FMT_CLI_FIXTURES: list[FmtIntegrationFixture] = [ + FmtIntegrationFixture( + test_id="merge-default", + cli_args=["fmt", "-f", "{config}", "--write"], + expected_log_fragment="Merged", + snapshot_name="fmt-cli-merge", + ), + FmtIntegrationFixture( + test_id="no-merge", + cli_args=["fmt", "-f", "{config}", "--write", "--no-merge"], + expected_log_fragment="skipping merge", + snapshot_name="fmt-cli-no-merge", + ), +] + + +@pytest.mark.parametrize( + list(FmtIntegrationFixture._fields), + FMT_CLI_FIXTURES, + ids=[fixture.test_id for fixture in FMT_CLI_FIXTURES], +) +def test_fmt_cli_integration( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + snapshot_yaml: SnapshotAssertion, + test_id: str, + cli_args: list[str], + expected_log_fragment: str, + snapshot_name: str, +) -> None: + """Run vcspull fmt via CLI to ensure duplicate handling respects flags.""" + config_file = tmp_path / ".vcspull.yaml" + config_file.write_text( + """~/study/c/: + repo1: url1 +~/study/c/: + repo2: url2 +""", + encoding="utf-8", + ) + + monkeypatch.chdir(tmp_path) + + formatted_args = [ + arg.format(config=str(config_file)) if "{config}" in arg else arg + for arg in cli_args + ] + + with caplog.at_level(logging.INFO), contextlib.suppress(SystemExit): + cli(formatted_args) + + yaml_text = config_file.read_text(encoding="utf-8") + assert yaml_text == snapshot_yaml(name=snapshot_name) + assert expected_log_fragment in caplog.text diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..3aa4072a --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,27 @@ +"""Shared pytest fixtures for snapshot testing.""" + +from __future__ import annotations + +import pytest +from syrupy.assertion import SnapshotAssertion +from syrupy.extensions.json import JSONSnapshotExtension +from syrupy.extensions.single_file import SingleFileSnapshotExtension, WriteMode + + +class YamlSnapshotExtension(SingleFileSnapshotExtension): + """Snapshot extension that persists plain-text YAML files.""" + + file_extension = "yaml" + _write_mode = WriteMode.TEXT + + +@pytest.fixture +def snapshot_json(snapshot: SnapshotAssertion) -> SnapshotAssertion: + """JSON-formatted snapshot assertions.""" + return snapshot.with_defaults(extension_class=JSONSnapshotExtension) + + +@pytest.fixture +def snapshot_yaml(snapshot: SnapshotAssertion) -> SnapshotAssertion: + """YAML-formatted snapshot assertions.""" + return snapshot.with_defaults(extension_class=YamlSnapshotExtension) diff --git a/uv.lock b/uv.lock index c2605dc8..e1422d51 100644 --- a/uv.lock +++ b/uv.lock @@ -1161,6 +1161,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/d9/52/1064f510b141bd54025f9b55105e26d1fa970b9be67ad766380a3c9b74b0/starlette-0.50.0-py3-none-any.whl", hash = "sha256:9e5391843ec9b6e472eed1365a78c8098cfceb7a74bfd4d6b1c0c0095efb3bca", size = 74033, upload-time = "2025-11-01T15:25:25.461Z" }, ] +[[package]] +name = "syrupy" +version = "5.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c1/90/1a442d21527009d4b40f37fe50b606ebb68a6407142c2b5cc508c34b696b/syrupy-5.0.0.tar.gz", hash = "sha256:3282fe963fa5d4d3e47231b16d1d4d0f4523705e8199eeb99a22a1bc9f5942f2", size = 48881, upload-time = "2025-09-28T21:15:12.783Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/9d/9a/6c68aad2ccfce6e2eeebbf5bb709d0240592eb51ff142ec4c8fbf3c2460a/syrupy-5.0.0-py3-none-any.whl", hash = "sha256:c848e1a980ca52a28715cd2d2b4d434db424699c05653bd1158fb31cf56e9546", size = 49087, upload-time = "2025-09-28T21:15:11.639Z" }, +] + [[package]] name = "tomli" version = "2.3.0" @@ -1323,6 +1335,7 @@ dev = [ { name = "sphinx-inline-tabs" }, { name = "sphinxext-opengraph" }, { name = "sphinxext-rediraffe" }, + { name = "syrupy" }, { name = "types-colorama" }, { name = "types-pyyaml" }, { name = "types-requests" }, @@ -1355,6 +1368,7 @@ testing = [ { name = "pytest-mock" }, { name = "pytest-rerunfailures" }, { name = "pytest-watcher" }, + { name = "syrupy" }, ] typings = [ { name = "types-colorama" }, @@ -1398,6 +1412,7 @@ dev = [ { name = "sphinx-inline-tabs" }, { name = "sphinxext-opengraph" }, { name = "sphinxext-rediraffe" }, + { name = "syrupy" }, { name = "types-colorama" }, { name = "types-pyyaml" }, { name = "types-requests" }, @@ -1427,6 +1442,7 @@ testing = [ { name = "pytest-mock" }, { name = "pytest-rerunfailures" }, { name = "pytest-watcher" }, + { name = "syrupy" }, ] typings = [ { name = "types-colorama" },