-
Notifications
You must be signed in to change notification settings - Fork 114
fix: create target dir for explicit --target claude; content hash fallback when .git absent #763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
dc0c416
088ffc1
06ca5fd
1e00ffd
c706be7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -993,3 +993,123 @@ def test_ghes_host_skips_ssh_attempt(self, mock_run): | |
| assert all("git@" not in arg for arg in only_cmd), ( | ||
| f"Expected HTTPS-only URL for GHES host, got: {only_cmd}" | ||
| ) | ||
|
|
||
|
|
||
| class TestExplicitTargetDirCreation: | ||
| """Verify --target creates root_dir even when auto_create=False (GH bug fix).""" | ||
|
|
||
| def setup_method(self): | ||
| self._tmpdir = tempfile.mkdtemp() | ||
| self.project_root = Path(self._tmpdir) | ||
|
|
||
| def teardown_method(self): | ||
| import shutil | ||
| shutil.rmtree(self._tmpdir, ignore_errors=True) | ||
|
|
||
| def test_explicit_target_creates_dir_for_auto_create_false(self): | ||
| """When _explicit is set, target dirs are created even if auto_create=False.""" | ||
| from apm_cli.integration.targets import KNOWN_TARGETS | ||
|
|
||
| claude = KNOWN_TARGETS["claude"] | ||
| assert claude.auto_create is False | ||
|
|
||
| # Simulate the fixed loop logic: create dir when _explicit is set | ||
| _explicit = "claude" | ||
| _targets = [claude] | ||
| for _t in _targets: | ||
| if not _t.auto_create and not _explicit: | ||
| continue | ||
| _target_dir = self.project_root / _t.root_dir | ||
| if not _target_dir.exists(): | ||
| _target_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| assert (self.project_root / ".claude").is_dir() | ||
|
|
||
| def test_auto_detect_skips_dir_for_auto_create_false(self): | ||
| """Without _explicit, auto_create=False targets don't get dirs created.""" | ||
| from apm_cli.integration.targets import KNOWN_TARGETS | ||
|
|
||
| claude = KNOWN_TARGETS["claude"] | ||
| assert claude.auto_create is False | ||
|
|
||
| _explicit = None | ||
| _targets = [claude] | ||
| for _t in _targets: | ||
| if not _t.auto_create and not _explicit: | ||
| continue | ||
| _target_dir = self.project_root / _t.root_dir | ||
| if not _target_dir.exists(): | ||
| _target_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| assert not (self.project_root / ".claude").exists() | ||
|
|
||
| def test_auto_create_true_always_creates_dir(self): | ||
| """auto_create=True targets create dir regardless of _explicit.""" | ||
| from apm_cli.integration.targets import KNOWN_TARGETS | ||
|
|
||
| copilot = KNOWN_TARGETS["copilot"] | ||
| assert copilot.auto_create is True | ||
|
|
||
| for _explicit in [None, "copilot"]: | ||
| import shutil | ||
| shutil.rmtree(self.project_root / copilot.root_dir, ignore_errors=True) | ||
|
|
||
| _targets = [copilot] | ||
| for _t in _targets: | ||
| if not _t.auto_create and not _explicit: | ||
| continue | ||
| _target_dir = self.project_root / _t.root_dir | ||
| if not _target_dir.exists(): | ||
| _target_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| assert (self.project_root / ".github").is_dir(), ( | ||
| f"auto_create=True should create dir when _explicit={_explicit!r}" | ||
| ) | ||
|
|
||
|
|
||
| class TestContentHashFallback: | ||
| """Verify content-hash fallback when .git is removed from installed packages.""" | ||
|
|
||
| def test_hash_match_skips_redownload(self): | ||
| """Content hash verification allows skipping re-download.""" | ||
| from apm_cli.utils.content_hash import compute_package_hash, verify_package_hash | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| pkg_dir = Path(tmpdir) / "pkg" | ||
| pkg_dir.mkdir() | ||
| (pkg_dir / "file.txt").write_text("hello") | ||
| content_hash = compute_package_hash(pkg_dir) | ||
|
|
||
| assert verify_package_hash(pkg_dir, content_hash) is True | ||
|
|
||
| def test_hash_mismatch_triggers_redownload(self): | ||
| """Mismatched content hash means re-download should proceed.""" | ||
| from apm_cli.utils.content_hash import verify_package_hash | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| pkg_dir = Path(tmpdir) / "pkg" | ||
| pkg_dir.mkdir() | ||
| (pkg_dir / "file.txt").write_text("original") | ||
|
|
||
| assert verify_package_hash(pkg_dir, "sha256:badhash") is False | ||
|
|
||
| def test_missing_content_hash_skips_fallback(self): | ||
| """When locked dep has no content_hash, the fallback guard prevents | ||
| verify_package_hash from being called.""" | ||
| from apm_cli.utils.content_hash import verify_package_hash | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| pkg_dir = Path(tmpdir) / "pkg" | ||
| pkg_dir.mkdir() | ||
| (pkg_dir / "file.txt").write_text("data") | ||
|
|
||
| # Simulate the guard logic from install.py: | ||
| # if _pd_locked_chk.content_hash and _pd_path.is_dir(): | ||
| content_hash = None # no content_hash recorded in lockfile | ||
| fallback_triggered = False | ||
| if content_hash and pkg_dir.is_dir(): | ||
| fallback_triggered = verify_package_hash(pkg_dir, content_hash) | ||
|
|
||
|
Comment on lines
+1070
to
+1112
|
||
| assert not fallback_triggered, ( | ||
| "Fallback must not trigger when content_hash is None" | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests duplicate the production directory-creation loop inline (re-implementing the
_explicit/auto_createcondition) instead of exercisingapm install/_install_apm_dependenciesbehavior. This makes the tests brittle (they can pass even if install.py regresses) and doesn't assert the CLI-facing bug fix. Consider invoking the install command viaCliRunnerwith--target claude(mocking downloads/integration as needed) and asserting.claude/is created, or factoring the loop into a helper and testing that helper directly.