Skip to content

Commit dd5ba36

Browse files
committed
tests/cli(add,discover): Cover merge opt-out paths
why: The new `--no-merge` flag must be regression-tested for both add and discover commands. what: - extend the CLI test suites with merge/no-merge parametrized fixtures - normalize log output and verify config contents when duplicates are kept separate
1 parent 54aa40a commit dd5ba36

File tree

2 files changed

+145
-12
lines changed

2 files changed

+145
-12
lines changed

tests/cli/test_add.py

Lines changed: 90 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import typing as t
99

1010
import pytest
11+
from syrupy.assertion import SnapshotAssertion
1112

1213
from vcspull.cli.add import add_repo, handle_add_command
1314
from vcspull.util import contract_user_home
@@ -314,12 +315,47 @@ def test_add_repo_creates_new_file(
314315
assert "newrepo" in config["./"]
315316

316317

317-
def test_add_repo_merges_duplicate_workspace_roots(
318+
class AddDuplicateMergeFixture(t.NamedTuple):
319+
"""Fixture describing duplicate merge toggles for add_repo."""
320+
321+
test_id: str
322+
merge_duplicates: bool
323+
expected_repo_names: set[str]
324+
expected_warning: str | None
325+
326+
327+
ADD_DUPLICATE_MERGE_FIXTURES: list[AddDuplicateMergeFixture] = [
328+
AddDuplicateMergeFixture(
329+
test_id="merge-on",
330+
merge_duplicates=True,
331+
expected_repo_names={"repo1", "repo2", "pytest-docker"},
332+
expected_warning=None,
333+
),
334+
AddDuplicateMergeFixture(
335+
test_id="merge-off",
336+
merge_duplicates=False,
337+
expected_repo_names={"repo2", "pytest-docker"},
338+
expected_warning="Duplicate workspace root",
339+
),
340+
]
341+
342+
343+
@pytest.mark.parametrize(
344+
list(AddDuplicateMergeFixture._fields),
345+
ADD_DUPLICATE_MERGE_FIXTURES,
346+
ids=[fixture.test_id for fixture in ADD_DUPLICATE_MERGE_FIXTURES],
347+
)
348+
def test_add_repo_duplicate_merge_behavior(
349+
test_id: str,
350+
merge_duplicates: bool,
351+
expected_repo_names: set[str],
352+
expected_warning: str | None,
318353
tmp_path: pathlib.Path,
319354
monkeypatch: MonkeyPatch,
320355
caplog: t.Any,
356+
snapshot: SnapshotAssertion,
321357
) -> None:
322-
"""Duplicate workspace roots are merged without losing repositories."""
358+
"""Duplicate workspace roots log appropriately based on merge toggle."""
323359
import yaml
324360

325361
caplog.set_level(logging.INFO)
@@ -347,16 +383,21 @@ def test_add_repo_merges_duplicate_workspace_roots(
347383
path=str(tmp_path / "study/python/pytest-docker"),
348384
workspace_root_path="~/study/python/",
349385
dry_run=False,
386+
merge_duplicates=merge_duplicates,
350387
)
351388

352-
with config_file.open() as fh:
353-
merged_config = yaml.safe_load(fh)
389+
with config_file.open(encoding="utf-8") as fh:
390+
config_after = yaml.safe_load(fh)
391+
392+
assert "~/study/python/" in config_after
393+
repos = config_after["~/study/python/"]
394+
assert set(repos.keys()) == expected_repo_names
354395

355-
assert "~/study/python/" in merged_config
356-
repos = merged_config["~/study/python/"]
357-
assert set(repos.keys()) == {"repo1", "repo2", "pytest-docker"}
396+
if expected_warning is not None:
397+
assert expected_warning in caplog.text
358398

359-
assert "Merged" in caplog.text
399+
normalized_log = caplog.text.replace(str(config_file), "<config>")
400+
snapshot.assert_match({"test_id": test_id, "log": normalized_log})
360401

361402

362403
class PathAddFixture(t.NamedTuple):
@@ -371,6 +412,8 @@ class PathAddFixture(t.NamedTuple):
371412
expected_url_kind: str # "remote" or "path"
372413
override_name: str | None
373414
expected_warning: str | None
415+
merge_duplicates: bool
416+
preexisting_yaml: str | None
374417

375418

376419
PATH_ADD_FIXTURES: list[PathAddFixture] = [
@@ -384,6 +427,8 @@ class PathAddFixture(t.NamedTuple):
384427
expected_url_kind="remote",
385428
override_name=None,
386429
expected_warning=None,
430+
merge_duplicates=True,
431+
preexisting_yaml=None,
387432
),
388433
PathAddFixture(
389434
test_id="path-interactive-accept",
@@ -395,6 +440,8 @@ class PathAddFixture(t.NamedTuple):
395440
expected_url_kind="remote",
396441
override_name="project-alias",
397442
expected_warning=None,
443+
merge_duplicates=True,
444+
preexisting_yaml=None,
398445
),
399446
PathAddFixture(
400447
test_id="path-interactive-decline",
@@ -406,6 +453,8 @@ class PathAddFixture(t.NamedTuple):
406453
expected_url_kind="remote",
407454
override_name=None,
408455
expected_warning=None,
456+
merge_duplicates=True,
457+
preexisting_yaml=None,
409458
),
410459
PathAddFixture(
411460
test_id="path-no-remote",
@@ -417,6 +466,28 @@ class PathAddFixture(t.NamedTuple):
417466
expected_url_kind="path",
418467
override_name=None,
419468
expected_warning="Unable to determine git remote",
469+
merge_duplicates=True,
470+
preexisting_yaml=None,
471+
),
472+
PathAddFixture(
473+
test_id="path-no-merge",
474+
remote_url="https://github.com/example/no-merge",
475+
assume_yes=True,
476+
prompt_response=None,
477+
dry_run=False,
478+
expected_written=True,
479+
expected_url_kind="remote",
480+
override_name=None,
481+
expected_warning="Duplicate workspace root",
482+
merge_duplicates=False,
483+
preexisting_yaml="""
484+
~/study/python/:
485+
repo1:
486+
repo: git+https://example.com/repo1.git
487+
~/study/python/:
488+
repo2:
489+
repo: git+https://example.com/repo2.git
490+
""",
420491
),
421492
]
422493

@@ -436,9 +507,12 @@ def test_handle_add_command_path_mode(
436507
expected_url_kind: str,
437508
override_name: str | None,
438509
expected_warning: str | None,
510+
merge_duplicates: bool,
511+
preexisting_yaml: str | None,
439512
tmp_path: pathlib.Path,
440513
monkeypatch: MonkeyPatch,
441514
caplog: t.Any,
515+
snapshot: SnapshotAssertion,
442516
) -> None:
443517
"""CLI path mode prompts and adds repositories appropriately."""
444518
caplog.set_level(logging.INFO)
@@ -451,6 +525,9 @@ def test_handle_add_command_path_mode(
451525

452526
config_file = tmp_path / ".vcspull.yaml"
453527

528+
if preexisting_yaml is not None:
529+
config_file.write_text(preexisting_yaml, encoding="utf-8")
530+
454531
expected_input = prompt_response if prompt_response is not None else "y"
455532
monkeypatch.setattr("builtins.input", lambda _: expected_input)
456533

@@ -463,6 +540,7 @@ def test_handle_add_command_path_mode(
463540
workspace_root_path=None,
464541
dry_run=dry_run,
465542
assume_yes=assume_yes,
543+
merge_duplicates=merge_duplicates,
466544
)
467545

468546
handle_add_command(args)
@@ -473,6 +551,10 @@ def test_handle_add_command_path_mode(
473551
assert "Found new repository to import" in log_output
474552
assert contracted_path in log_output
475553

554+
normalized_log = log_output.replace(str(config_file), "<config>")
555+
normalized_log = normalized_log.replace(str(repo_path), "<repo_path>")
556+
snapshot.assert_match({"test_id": test_id, "log": normalized_log})
557+
476558
if dry_run:
477559
assert "skipped (dry-run)" in log_output
478560

tests/cli/test_discover.py

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import typing as t
77

88
import pytest
9+
from syrupy.assertion import SnapshotAssertion
910

1011
from vcspull.cli.discover import discover_repos
1112

@@ -41,6 +42,8 @@ class DiscoverFixture(t.NamedTuple):
4142
preexisting_config: dict[str, t.Any] | None
4243
user_input: str | None
4344
expected_workspace_labels: set[str] | None
45+
merge_duplicates: bool
46+
preexisting_yaml: str | None
4447

4548

4649
DISCOVER_FIXTURES: list[DiscoverFixture] = [
@@ -59,6 +62,8 @@ class DiscoverFixture(t.NamedTuple):
5962
preexisting_config=None,
6063
user_input=None,
6164
expected_workspace_labels={"~/code/"},
65+
merge_duplicates=True,
66+
preexisting_yaml=None,
6267
),
6368
DiscoverFixture(
6469
test_id="discover-recursive",
@@ -76,6 +81,8 @@ class DiscoverFixture(t.NamedTuple):
7681
preexisting_config=None,
7782
user_input=None,
7883
expected_workspace_labels={"~/code/"},
84+
merge_duplicates=True,
85+
preexisting_yaml=None,
7986
),
8087
DiscoverFixture(
8188
test_id="discover-dry-run",
@@ -91,6 +98,8 @@ class DiscoverFixture(t.NamedTuple):
9198
preexisting_config=None,
9299
user_input=None,
93100
expected_workspace_labels=None,
101+
merge_duplicates=True,
102+
preexisting_yaml=None,
94103
),
95104
DiscoverFixture(
96105
test_id="discover-default-config",
@@ -106,6 +115,8 @@ class DiscoverFixture(t.NamedTuple):
106115
preexisting_config=None,
107116
user_input=None,
108117
expected_workspace_labels={"~/code/"},
118+
merge_duplicates=True,
119+
preexisting_yaml=None,
109120
),
110121
DiscoverFixture(
111122
test_id="discover-workspace-normalization",
@@ -125,6 +136,8 @@ class DiscoverFixture(t.NamedTuple):
125136
},
126137
user_input=None,
127138
expected_workspace_labels={"~/code/"},
139+
merge_duplicates=True,
140+
preexisting_yaml=None,
128141
),
129142
DiscoverFixture(
130143
test_id="discover-interactive-confirm",
@@ -140,6 +153,8 @@ class DiscoverFixture(t.NamedTuple):
140153
preexisting_config=None,
141154
user_input="y",
142155
expected_workspace_labels={"~/code/"},
156+
merge_duplicates=True,
157+
preexisting_yaml=None,
143158
),
144159
DiscoverFixture(
145160
test_id="discover-interactive-abort",
@@ -155,6 +170,32 @@ class DiscoverFixture(t.NamedTuple):
155170
preexisting_config=None,
156171
user_input="n",
157172
expected_workspace_labels=None,
173+
merge_duplicates=True,
174+
preexisting_yaml=None,
175+
),
176+
DiscoverFixture(
177+
test_id="discover-no-merge",
178+
repos_to_create=[
179+
("repo3", "git+https://github.com/user/repo3.git"),
180+
],
181+
recursive=False,
182+
workspace_override=None,
183+
dry_run=False,
184+
yes=True,
185+
expected_repo_count=2,
186+
config_relpath=".vcspull.yaml",
187+
preexisting_config=None,
188+
user_input=None,
189+
expected_workspace_labels={"~/code/"},
190+
merge_duplicates=False,
191+
preexisting_yaml="""
192+
~/code/:
193+
repo1:
194+
repo: git+https://example.com/repo1.git
195+
~/code/:
196+
repo2:
197+
repo: git+https://example.com/repo2.git
198+
""",
158199
),
159200
]
160201

@@ -263,9 +304,12 @@ def test_discover_repos(
263304
preexisting_config: dict[str, t.Any] | None,
264305
user_input: str | None,
265306
expected_workspace_labels: set[str] | None,
307+
merge_duplicates: bool,
308+
preexisting_yaml: str | None,
266309
tmp_path: pathlib.Path,
267310
monkeypatch: MonkeyPatch,
268311
caplog: t.Any,
312+
snapshot: SnapshotAssertion,
269313
) -> None:
270314
"""Test discovering repositories from filesystem."""
271315
import logging
@@ -291,7 +335,9 @@ def test_discover_repos(
291335
target_config_file.parent.mkdir(parents=True, exist_ok=True)
292336
config_argument = str(target_config_file)
293337

294-
if preexisting_config is not None:
338+
if preexisting_yaml is not None:
339+
target_config_file.write_text(preexisting_yaml, encoding="utf-8")
340+
elif preexisting_config is not None:
295341
import yaml
296342

297343
target_config_file.write_text(
@@ -310,8 +356,13 @@ def test_discover_repos(
310356
workspace_root_override=workspace_override,
311357
yes=yes,
312358
dry_run=dry_run,
359+
merge_duplicates=merge_duplicates,
313360
)
314361

362+
if preexisting_yaml is not None or not merge_duplicates:
363+
normalized_log = caplog.text.replace(str(target_config_file), "<config>")
364+
snapshot.assert_match({"test_id": test_id, "log": normalized_log})
365+
315366
if dry_run:
316367
# In dry-run mode, config file should not be created/modified
317368
if expected_repo_count == 0:
@@ -383,8 +434,8 @@ def test_discover_config_load_edges(
383434

384435
if mode == "non_dict":
385436
monkeypatch.setattr(
386-
"vcspull.cli.discover.ConfigReader._from_file",
387-
lambda _path: ["invalid"],
437+
"vcspull.cli.discover.DuplicateAwareConfigReader.load_with_duplicates",
438+
lambda _path: (["invalid"], {}),
388439
)
389440
else: # mode == "exception"
390441

@@ -393,7 +444,7 @@ def _raise(_path: pathlib.Path) -> t.NoReturn:
393444
raise ValueError(error_message)
394445

395446
monkeypatch.setattr(
396-
"vcspull.cli.discover.ConfigReader._from_file",
447+
"vcspull.cli.discover.DuplicateAwareConfigReader.load_with_duplicates",
397448
_raise,
398449
)
399450

0 commit comments

Comments
 (0)