Skip to content

Commit e2b36e8

Browse files
committed
fix: address final copilot review feedback
1 parent fa3405a commit e2b36e8

4 files changed

Lines changed: 23 additions & 21 deletions

File tree

src/specify_cli/shared_infra.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def load_speckit_manifest(
2323
manifest = IntegrationManifest.load("speckit", project_path)
2424
manifest.version = version
2525
return manifest
26-
except (ValueError, FileNotFoundError) as exc:
26+
except (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) as exc:
2727
if console is not None:
2828
console.print(
2929
f"[yellow]Warning:[/yellow] Could not read shared infrastructure "

tests/integrations/test_cli.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,23 @@ def test_shared_infra_warns_when_manifest_cannot_be_loaded(self, tmp_path, capsy
271271
assert "Could not read shared infrastructure manifest" in captured.out
272272
assert "A new shared manifest will be created" in captured.out
273273

274+
def test_shared_infra_warns_when_manifest_cannot_be_decoded(self, tmp_path, capsys):
275+
"""Non-UTF-8 shared manifests warn before falling back to a new manifest."""
276+
from specify_cli import _install_shared_infra
277+
278+
project = tmp_path / "bad-shared-manifest-encoding-test"
279+
project.mkdir()
280+
integrations_dir = project / ".specify" / "integrations"
281+
integrations_dir.mkdir(parents=True)
282+
manifest_path = integrations_dir / "speckit.manifest.json"
283+
manifest_path.write_bytes(b"\xff\xfe\x00")
284+
285+
_install_shared_infra(project, "sh")
286+
287+
captured = capsys.readouterr()
288+
assert "Could not read shared infrastructure manifest" in captured.out
289+
assert "A new shared manifest will be created" in captured.out
290+
274291
def test_shared_infra_no_warning_when_forced(self, tmp_path, capsys):
275292
"""No skip warning when force=True (all files overwritten)."""
276293
from specify_cli import _install_shared_infra

tests/integrations/test_integration_subcommand.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,10 @@ def test_list_shows_multi_install_safe_status(self, tmp_path):
8181
assert result.exit_code == 0
8282
assert "Multi-install" in result.output
8383
assert "Safe" in result.output
84-
rows = {
85-
cells[0]: cells
86-
for line in result.output.splitlines()
87-
if line.startswith("│")
88-
for cells in ([cell.strip() for cell in line.split("│")[1:-1]],)
89-
if cells and cells[0]
90-
}
91-
assert rows["claude"][4] == "yes"
92-
assert rows["copilot"][4] == "no"
84+
claude_row = next(line for line in result.output.splitlines() if line.startswith("│ claude"))
85+
copilot_row = next(line for line in result.output.splitlines() if line.startswith("│ copilot"))
86+
assert claude_row.rstrip().endswith("│ yes │")
87+
assert copilot_row.rstrip().endswith("│ no │")
9388

9489

9590
# ── install ──────────────────────────────────────────────────────────

tests/integrations/test_registry.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,6 @@
3131
]
3232

3333

34-
def _multi_install_safe_install_orders() -> list[tuple[str, str]]:
35-
safe_keys = _multi_install_safe_keys()
36-
return [
37-
(first, second)
38-
for first in safe_keys
39-
for second in safe_keys
40-
if first != second
41-
]
42-
43-
4434
def _multi_install_safe_keys() -> list[str]:
4535
return sorted(
4636
key
@@ -240,7 +230,7 @@ def test_safe_context_files_do_not_overlap_other_command_dirs(self, first, secon
240230
f"commands directory {_integration_commands_dir(first)!r}"
241231
)
242232

243-
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_install_orders())
233+
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
244234
def test_safe_integrations_have_disjoint_manifests(
245235
self,
246236
tmp_path,

0 commit comments

Comments
 (0)