fix: CI integration test corrections#109
Merged
danielmeppiel merged 3 commits intomainfrom Feb 26, 2026
Merged
Conversation
- Fix _auto_install_virtual_package to handle subdirectory paths correctly (don't append .prompt.md when path already has an extension) - Add unit tests for subdirectory auto-install support - Update test repos: ComposioHQ/awesome-claude-skills → anthropics/skills - Correct skill integration assertions: .github/agents/ → .github/skills/ - Rename test_skill_compile.py → test_skill_integration.py with accurate class/method names reflecting that skills integrate at install, not compile - Add unauthenticated API fallback in setup-codex.sh - Remove stale metadata assertions in test_auto_integration.py - Update guardrailing test to use github/awesome-copilot references
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates APM’s CI integration tests (and a small part of ScriptRunner) to reflect the current “skills-first” integration strategy: skills are copied to .github/skills/ at install time (not compiled into .agent.md), .prompt.md usage is being phased out in tests, and real upstream skills are sourced from anthropics/skills.
Changes:
- Update ScriptRunner prompt discovery + virtual package auto-install to support virtual subdirectory skills and use canonical install paths.
- Replace ComposioHQ skill references with
anthropics/skillsacross integration tests; add a new integration suite verifying.github/skills/integration and compile non-modification. - Adjust e2e/integration tests away from SKILL→agent expectations and remove prompt-metadata injection assertions.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_script_runner.py | Adds unit coverage for SKILL.md discovery and virtual subdirectory auto-install. |
| src/apm_cli/core/script_runner.py | Extends discovery to find SKILL.md and updates virtual package install flow to use DependencyReference.get_install_path() and subdirectory downloads. |
| tests/integration/test_skill_integration.py | New integration tests validating skill install integrates into .github/skills/ and compile doesn’t modify integrated skills. |
| tests/integration/test_skill_install.py | Updates skill source refs and assertions to expect .github/skills/{name}/SKILL.md rather than generated agent files. |
| tests/integration/test_skill_compile.py | Removes obsolete integration tests that assumed SKILL.md → .agent.md compilation. |
| tests/integration/test_mixed_deps.py | Updates mixed-dependency tests to use anthropics/skills and validate .github/skills/ integration. |
| tests/integration/test_guardrailing_hero_e2e.py | Switches step 3 to install an instructions virtual file and updates assertions around AGENTS.md content. |
| tests/integration/test_auto_integration.py | Updates expectations to verify prompt integration copies content verbatim (no metadata injection). |
| scripts/runtime/setup-codex.sh | Adds a fallback unauthenticated GitHub API call when an authenticated “latest release” lookup yields no tag. |
Comments suppressed due to low confidence (5)
tests/integration/test_skill_integration.py:90
- This test runs
apm installbut does not assert on the subprocess return code. Because it laterpytest.skip()s when expected files are missing, an installation failure (network, auth, repo change) can be silently treated as a passing test. Capture the result and assert success (or skip based on an explicit/expected failure condition) before comparing file contents.
subprocess.run(
[apm_command, "install", "anthropics/skills/skills/brand-guidelines"],
cwd=temp_project,
capture_output=True,
text=True,
timeout=120
)
tests/integration/test_skill_integration.py:156
test_compile_does_not_modify_skillsdoesn't assert thatapm install/apm compilesucceeded, so a failing command could still lead to a passing test if the skill file isn't present or unchanged. Also, usingst_mtimecan be flaky on filesystems with coarse timestamp resolution;st_mtime_ns(or content hashing) is a more robust way to detect modifications.
subprocess.run(
[apm_command, "install", "anthropics/skills/skills/brand-guidelines"],
cwd=temp_project,
capture_output=True,
text=True,
timeout=120
)
skill_integrated = temp_project / ".github" / "skills" / "brand-guidelines" / "SKILL.md"
if skill_integrated.exists():
# Record modification time
mtime_before = skill_integrated.stat().st_mtime
# Run compile
subprocess.run(
[apm_command, "compile"],
cwd=temp_project,
capture_output=True,
text=True,
timeout=60
)
# Skill file should not be modified by compile
mtime_after = skill_integrated.stat().st_mtime
assert mtime_before == mtime_after, "Compile should not modify skill integrated at install"
tests/integration/test_skill_install.py:118
- This test asserts on stdout content but doesn't assert the install command succeeded. If the install fails (auth/rate limit/etc.), stdout/stderr may still contain the searched strings and produce misleading results. Assert
result.returncode == 0(and include stderr in the assertion message) before checking output text.
This issue also appears on line 153 of the same file.
result = subprocess.run(
[apm_command, "install", "anthropics/skills/skills/brand-guidelines", "--verbose"],
cwd=temp_project,
capture_output=True,
text=True,
timeout=120
)
# Check for skill detection message
assert "Claude Skill" in result.stdout or "SKILL.md detected" in result.stdout
tests/integration/test_skill_install.py:165
- This test ignores the install subprocess return code and then
pytest.skip()s if the skill path doesn't exist, which can hide unexpected install failures as skips. Capture the subprocess result and either assert success or skip only for explicitly recognized conditions (e.g., 404/not found).
subprocess.run(
[apm_command, "install", "anthropics/skills/skills/skill-creator", "--verbose"],
cwd=temp_project,
capture_output=True,
text=True,
timeout=120
)
skill_path = temp_project / "apm_modules" / "anthropics" / "skills" / "skills" / "skill-creator"
if not skill_path.exists():
pytest.skip("skill-creator not available")
tests/integration/test_skill_install.py:209
- The class/docstring imply VSCode integration is disabled when
.github/is missing, but the CLI now auto-creates.github/(as the standard skills root) and this test expects.github/skills/...to exist. Consider renaming the class/test and updating the docstring to reflect the actual behavior being validated (auto-creation + skill integration).
class TestSkillInstallWithoutVSCodeTarget:
"""Test skill installation when VSCode is not the target."""
def test_skill_install_without_github_folder(self, tmp_path, apm_command):
"""Skill installs but no agent.md generated without .github/ folder."""
project_dir = tmp_path / "no-vscode-project"
- Assert returncode == 0 in test_skill_integration.py instead of silently skipping on missing files (masks real install failures) - Update guardrailing test docstring to reflect actual flow (no longer claims to mirror README line-for-line) - Add assertion that instruction file is actually downloaded, not just the directory
SebastienDegodez
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is fixing integration tests in CI: