Refactor test-integration.sh to fixture-driven pytest discovery (eliminate manual test wiring)

[danielmeppiel]

Problem

scripts/test-integration.sh is a 665-line bash script that explicitly enumerates ~20 pytest tests/integration/test_*.py invocations, one per file. Adding a new integration test file requires editing the script. Forgetting to wire a file means the test silently never runs.

This is not a hypothetical risk -- it just bit us:

• tests/integration/test_skill_install.py was never wired into the script. Two existing tests (test_reinstall_same_skill_is_idempotent, and the new test_reinstall_does_not_leak_apm_pin_to_deploy_targets from PR fix: exclude .apm-pin from skill deploy copytree #1153) silently rotted until I noticed and patched it in PR fix: exclude .apm-pin from skill deploy copytree #1153 (commit 941c6942).
• No CI workflow runs the full integration script. .github/workflows/ci-integration.yml only invokes test_runtime_smoke.py; the other ~19 files run only on manual release-validation execution. Most "integration coverage" runs on the honor system.

Real constraints (what's legitimate)

The script does several things that genuinely need orchestration:

• Builds a PyInstaller binary (or reuses CI artifact at ./dist/$BINARY_NAME/apm)
• Symlinks the binary into $PATH so tests exercise the shipped binary, not in-process python -m apm_cli
• Sets up runtimes (copilot/codex/llm) by invoking apm runtime setup
• Resolves GitHub tokens via github-token-helper.sh

These are real concerns. Per-file pytest enumeration is not.

What's wrong (per Python testing world-class standards)

1. Manual wiring is the bug we just hit. Pytest's whole value-prop is auto-discovery (pytest <dir> finds and runs everything matching test_*.py). The bash script defeats that.
2. Bash is the wrong layer for test selection, reporting, and exit codes. The 20 if pytest ... ; then log_success ; else log_error ; exit 1 ; fi blocks reimplement what pytest does natively, and worse: a single test file failure aborts the whole run via set -e, masking subsequent failures.
3. No marker-based gating. Different files need different env (ADO_APM_PAT, GITHUB_APM_PAT, APM_RUN_INTEGRATION_TESTS=1, runtime presence). Today this is encoded as conditional bash branches; it should be @pytest.mark.requires_ado_pat etc., selectable via pytest -m.
4. The "built binary" constraint doesn't justify bash sprawl. It's a session-scoped fixture, not a loop.

Proposed pattern

# tests/integration/conftest.py
@pytest.fixture(scope="session")
def apm_binary():
    # build once OR read APM_BINARY_PATH from env (CI), prepend to PATH
    ...

# pyproject.toml
[tool.pytest.ini_options]
markers = [
    "requires_apm_binary",
    "requires_github_apm_pat",
    "requires_ado_pat",
    "requires_runtime_codex",
    "requires_runtime_copilot",
    "requires_runtime_llm",
]

# Per test file (replaces today's pytestmark skipif chains):
pytestmark = [pytest.mark.requires_apm_binary, pytest.mark.requires_github_apm_pat]

scripts/test-integration.sh shrinks to ~50 lines: detect platform, build/locate binary, set up runtimes, export tokens, then one invocation:

pytest tests/integration/ -v --tb=short \
  $( [[ -z "${ADO_APM_PAT:-}" ]] && echo '-m "not requires_ado_pat"' )

Pytest handles discovery, selection, reporting, exit codes. Bash handles env setup. Each layer does what it's good at.

Acceptance criteria

• scripts/test-integration.sh no longer enumerates individual pytest test_*.py calls; it invokes pytest tests/integration/ with marker selection.
• A new test file dropped into tests/integration/ with the right markers automatically runs without script edits.
• All currently-wired integration tests still run (no regressions).
• Conditional gating (ADO PAT, GitHub PAT, runtime presence) preserved via markers + skipif fixtures, not bash branches.
• CI integration workflow (.github/workflows/ci-integration.yml) optionally extended to run the full marker-filtered suite (currently only runs smoke).

Out of scope

• Migrating tests off the built binary to in-process invocation (that's a separate, larger discussion).
• Adding new test coverage (this is a pure refactor).
• Changing github-token-helper.sh or runtime-setup commands.

References

• PR fix: exclude .apm-pin from skill deploy copytree #1153 (where the wiring gap was discovered and patched in commit 941c6942)
• scripts/test-integration.sh:336-664 (the enumerated pytest blocks)

[github-actions]

Triage decision

accept

Proposed labels

area/testing
area/ci-cd
type/refactor
status/accepted
priority/low

Milestone

null

Suggested next action

Implement the conftest.py session fixture and pyproject.toml marker declarations first, then annotate each integration test file and reduce scripts/test-integration.sh to a single pytest tests/integration/ invocation.

Suggested issue comment

Thanks for filing this with such clear evidence -- the silent test-wiring gap in `test_skill_install.py` is exactly the kind of failure mode that fixture-driven discovery is designed to prevent.

The panel accepts this as a straightforward refactor. The proposed pattern is idiomatic pytest: a `session`-scoped `apm_binary` fixture in `tests/integration/conftest.py` handles the binary build-or-locate step once, and explicit markers (`requires_apm_binary`, `requires_github_apm_pat`, `requires_ado_pat`, `requires_runtime_*`) in `pyproject.toml` replace the conditional bash branches. Pytest handles discovery, selection, reporting, and exit codes; bash handles env setup. Each layer does what it is good at.

The scope note is well-drawn: `github-token-helper.sh`, runtime-setup commands, and the binary-vs-in-process question are all out of scope here.

Companion issue #1168 owns the audit of the 38 currently-unwired files and the actual script swap; this issue covers the marker/fixture infrastructure that unblocks that work.

Suggested implementation order:
1. Add markers to `pyproject.toml` `[tool.pytest.ini_options]`.
2. Write the `apm_binary` session fixture in `tests/integration/conftest.py`.
3. Annotate each currently-wired test file with the appropriate `pytestmark`.
4. Verify all wired tests pass under `pytest tests/integration/ -m "requires_apm_binary and requires_github_apm_pat"`.

Once this lands, #1168 can proceed with the audit sweep and script swap.

Per-lens notes (collapsed)

DevX UX Expert -- User-Need Reviewer

The issue maps to a clear contributor workflow pain point: adding a new integration test file today requires editing a 665-line bash script, and forgetting to do so causes silent test rot -- as already happened with test_skill_install.py. The proposed fixture/marker pattern is idiomatic pytest and directly addresses the root cause (manual wiring). The framing aligns with standard Python project conventions; no mental-model mismatch with the pip/pytest contributor experience APM targets.

Supply Chain Security Expert -- Risk-Surface Reviewer

No supply chain, lockfile, marketplace, MCP config, signing, or auth surface is touched by this refactor. Token handling and runtime setup remain in the bash layer (explicitly out of scope). No risk-surface implication; theme/security and theme/governance are not required.

OSS Growth Hacker -- Contributor-Tone Reviewer

Not activated -- Author is a COLLABORATOR with established contribution history; tone-tuning for a first-time contributor is not applicable.

Python Architect -- Architecture Reviewer

Design is complete and ready to implement. Session-scoped apm_binary fixture correctly amortizes the binary build cost across the full suite. The marker set covers all current env-gating cases. The pyproject.toml change is additive. The main cross-cutting impact is annotating ~20 currently-wired test files, which is mechanical. The companion issue (#1168) correctly scopes the harder work -- auditing 38 unwired files and surfacing real bugs -- as a separate gate before the script swap lands. No design doc needed; accept.

Doc Writer -- Documentation Reviewer

Not activated -- Pure testing infrastructure refactor with no user-facing CLI surface or documentation change. No area/docs-site secondary label needed.

APM CEO -- Triage Arbiter

Clean type/refactor on area/testing + area/ci-cd. The infra-only exception applies -- no theme/* label is required since this touches only CI and test tooling with no product surface implication. The phased approach (infrastructure in this issue, audit + swap in #1168) is the right risk management call: it avoids mixing marker-gap noise with real-bug noise on the first discovery run. Priority is priority/low -- this is a DX quality improvement that unblocks correct CI coverage, not a user-facing feature or a bug fix. Accept with milestone null pending milestone list confirmation.

{
  "decision": "accept",
  "decision_detail": "",
  "theme": null,
  "areas": ["area/testing", "area/ci-cd"],
  "type": "type/refactor",
  "status": "status/accepted",
  "priority": "priority/low",
  "preserved_labels": [],
  "milestone": null,
  "next_action": "Implement the conftest.py session fixture and pyproject.toml marker declarations first, then annotate each integration test file and reduce scripts/test-integration.sh to a single pytest tests/integration/ invocation.",
  "comment_markdown": "Thanks for filing this with such clear evidence -- the silent test-wiring gap in test_skill_install.py is exactly the kind of failure mode that fixture-driven discovery is designed to prevent.\n\nThe panel accepts this as a straightforward refactor. The proposed pattern is idiomatic pytest: a session-scoped apm_binary fixture in tests/integration/conftest.py handles the binary build-or-locate step once, and explicit markers in pyproject.toml replace the conditional bash branches. Pytest handles discovery, selection, reporting, and exit codes; bash handles env setup.\n\nCompanion issue #1168 owns the audit of the 38 currently-unwired files and the actual script swap; this issue covers the marker/fixture infrastructure that unblocks that work."
}

---

Triage status: agentic proposal pending human ratification.
Silence is approval. Maintainers can:

• Override any label or milestone above by editing it directly --
  human edits are authoritative and will not be reverted on
  subsequent runs.
• Re-trigger triage by applying the status/needs-triage label, or
  by removing status/triaged to enroll the issue in the next
  daily sweep.

Posted by the Triage Panel workflow. See .apm/skills/apm-triage-panel for the panel skill.

Note

🔒 Integrity filter blocked 42 items

The following items were blocked because they don't meet the GitHub integrity level.

• [FEATURE] Extended MCP Server Configuration in apm.yml #20 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Allow install to download and unpack a GitHub release automatically #514 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Skills from .well-known URIs #554 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• Feature: per-target agent frontmatter transformation during deployment #581 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• bug: apm run start picks wrong runtime — system PATH stub takes priority over APM-managed binary, codex v0.116+ incompatible with GitHub Models #605 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Add ability to specify a different configuration file name or path #612 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• bug: apm install plugin@marketplace validation bypasses registry proxy #615 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Renovate supporting apm.yml #639 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• Align CONTRIBUTING, Python formatting (Black/isort), and CI #645 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Copilot CLI user-scope instructions: can apm install -g write copilot-instructions.md #650 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• Marketplace add source parity with the Anthropic spec #676 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] PEP 8 Support #690 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] URL-based marketplace work Extension #692 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Create root AGENTS.md documenting all CI-enforced rules for coding agents #695 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Kiro Support #702 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• [FEATURE] Installation via NPM registry #732 list_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• ... and 26 more items

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Triage Panel · ● 789.1K · ◷