Skip to content

test(integration): add live GitLab generic install smoke#1663

Open
abhinavgautam01 wants to merge 4 commits into
microsoft:mainfrom
abhinavgautam01:fix-1229-live-gitlab-smoke
Open

test(integration): add live GitLab generic install smoke#1663
abhinavgautam01 wants to merge 4 commits into
microsoft:mainfrom
abhinavgautam01:fix-1229-live-gitlab-smoke

Conversation

@abhinavgautam01

Copy link
Copy Markdown
Contributor

Description

Adds an opt-in live smoke test for apm install gitlab.com/<group>/<repo> so the generic git backend is covered end-to-end against GitLab. The test validates package materialization under apm_modules/ and confirms apm.lock.yaml records host: gitlab.com plus a full resolved commit SHA.

The smoke is gated behind APM_LIVE_GENERIC_PACKAGE because the fixture suggested in the issue is not currently public. Scheduled/manual CI wiring is included so maintainers can enable it by setting that repository variable to a stable public APM-shaped GitLab repo.

Fixes #1229

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Validation run:

uv run --python 3.12 ruff check pyproject.toml tests/integration/test_live_generic_gitlab_install.py
uv run --python 3.12 pytest tests/integration/test_live_generic_gitlab_install.py -m live_generic -o addopts= -v --tb=short
git diff --check

Focused pytest collected the live smoke and skipped locally because APM_LIVE_GENERIC_PACKAGE is not set, since the GitLab repo suggested one from the issue is not available publicly:

gitlab.com/microsoft-apm-fixtures/smoke-pkg -> GitLab API returns 404 Project Not Found

Spec conformance (OpenAPM v0.1)

If this PR changes behaviour that an OpenAPM v0.1 req-XXX covers, confirm the three-step ritual (see CONTRIBUTING.md "Adding or changing a normative requirement"):

  • Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
    (new/changed anchor + prose + Appendix C
    row).

  • Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml
    updated.

  • Test edit: a @pytest.mark.req("req-XXX") test under
    tests/spec_conformance/ added or extended.

  • CONFORMANCE.{md,json} regenerated via
    uv run --extra dev python -m tests.spec_conformance.gen_statement
    and committed.

  • N/A -- this PR does not change OpenAPM-observable behaviour.

Copilot AI review requested due to automatic review settings June 4, 2026 16:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a scheduled/manual live smoke test to validate that apm install can clone and lock a real GitLab-hosted package via the generic git backend.

Changes:

  • Introduces a new live integration test that installs a configured GitLab repo and asserts lockfile stamping with a full commit SHA.
  • Registers a new live_generic pytest marker.
  • Extends the release workflow to run this smoke test on schedule / workflow_dispatch.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
tests/integration/test_live_generic_gitlab_install.py New live smoke test exercising the generic git clone path against GitLab and validating lockfile fields.
pyproject.toml Adds live_generic marker so pytest recognizes the new selection.
.github/workflows/build-release.yml Runs the new smoke test in scheduled/manual workflow runs using repo variable configuration.

Comment thread tests/integration/test_live_generic_gitlab_install.py Outdated
Comment thread tests/integration/test_live_generic_gitlab_install.py Outdated
Comment thread tests/integration/test_live_generic_gitlab_install.py Outdated
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 19, 2026
@github-actions

Copy link
Copy Markdown

APM Review Panel: ship_with_followups

PR #1663 adds live GitLab smoke-test scaffolding but is permanently dormant (fixture missing, wrong backend named) -- ship with three concrete follow-ups before closing issue #1229.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Three panelists converge on the single most consequential finding: the test added here never runs. APM_LIVE_GENERIC_PACKAGE is unset in every existing CI workflow, vars.APM_LIVE_GENERIC_PACKAGE is not configured in the repository, and the fixture repo gitlab.com/microsoft-apm-fixtures/smoke-pkg does not exist. The test-coverage-expert carries the strongest signal here via two evidence blocks (outcome: missing, portability-by-manifest): the generic-git install path has zero automated regression coverage post-merge, and a six-month drift in the backend would go undetected until a user reports it. OSS growth hacker and devx-ux-expert corroborate independently. This is the single action that converts the PR from scaffolding into value.

Auth-expert and python-architect independently surface the wrong-backend naming: AuthResolver.classify_host('gitlab.com') returns HostInfo(kind='gitlab'), which backend_for() dispatches via _BACKEND_BY_KIND to GitLabBackend -- not GenericGitBackend. Every docstring in the new module says otherwise, and a stale sentence in GenericGitBackend's class docstring compounds the confusion. Strategically, this matters because it affects scope: if issue #1229 targets the true GenericGitBackend path (Gitea/Gogs/Forgejo), this PR partially misses the target and a separate fixture host is required. The maintainer must confirm intent in #1229 before activating CI. Separately, auth-expert and supply-chain-expert converge on two independent isolation gaps in _env_with_isolated_home: GITLAB_APM_PAT/GITLAB_TOKEN are not stripped (silently converting the anonymous smoke test to authenticated), and ambient runner tokens (GITHUB_TOKEN, ACTIONS_RUNTIME_TOKEN) are forwarded wholesale to the subprocess materializing external content. Both are pre-activation defects -- currently harmless because the test never runs, but must be resolved before the first live CI run.

Overall the panel is well-aligned. Performance-expert correctly self-deactivated. No panelist returned a blocking finding. The contribution from an external collaborator is valuable: the test structure is clean, the CI scaffold is reusable, and the three gaps above are correctable in follow-up PRs. The recommended shipping path is to merge the scaffolding now, open a tracked follow-up for fixture creation (which simultaneously activates CI and closes #1229), and batch the auth-isolation and docstring fixes into that same PR.

Dissent. No hard disagreements between panelists. Auth-expert and python-architect reach the same wrong-backend conclusion from orthogonal entry points (auth resolution vs. class dispatch), which strengthens the finding rather than creating tension. The only latent framing gap is scope: OSS growth hacker treats the PR as validating the GitLab path (correct at the user level) while auth-expert notes GenericGitBackend (Gitea/Gogs/Forgejo) remains unexercised (correct at the backend level). These are both true simultaneously; the maintainer resolves it by confirming issue #1229's intended scope.

Aligned with: Portable-by-manifest (scaffold exists, no live coverage yet). Secure-by-default (weakened in test env -- token-forwarding gap pre-activation). Governed-by-policy (minor gap -- runtime skip anti-pattern invisible to test_marker_registry_sync). Multi-harness/multi-host (directly serves -- first live scaffold for non-GitHub/non-ADO install path). OSS-community-driven (external contributor closing a tracked gap, high amplification potential). Pragmatic-as-npm (minor ergonomics debt on marker predicate pattern).

Growth signal. Once gitlab.com/microsoft-apm-fixtures/smoke-pkg exists and APM_LIVE_GENERIC_PACKAGE is set in CI, the first green scheduled run is a concrete story beat: APM continuously smoke-tests GitLab installs. The README already claims install from anywhere; a CHANGELOG entry paired with live CI coverage converts that claim into verifiable proof. This is the highest-leverage signal for the non-GitHub OSS developer segment skeptical of Microsoft-branded tooling. Priority: create the fixture immediately post-merge, link the activation PR in issue #1229, and use the first green run as the outreach moment.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 Test-only PR, no prod class changes; nit on docstring naming GenericGitBackend when gitlab.com dispatches to GitLabBackend.
CLI Logging Expert 0 1 2 No stdout content assertions beyond exit code; NO_COLOR via setdefault not hard-set; untruncated stdout in failure message.
DevX UX Expert 0 1 2 Good contributor ergonomics; live_generic not in default addopts exclusion is a forward-looking gap.
Supply Chain Security Expert 0 2 2 Uses vars (not secrets) correctly; no expected-SHA pin and ambient CI tokens leak to subprocess via os.environ.copy().
OSS Growth Hacker 0 2 1 Good vendor-neutral signal; test is inert until fixture exists -- CHANGELOG entry and fixture-tracking issue complete the story.
Auth Expert 0 2 1 gitlab.com resolves to GitLabBackend not GenericGitBackend; GITLAB_APM_PAT/GITLAB_TOKEN not stripped from subprocess env.
Doc Writer 0 2 2 live_generic marker row missing from integration-testing.md; APM_LIVE_GENERIC_PACKAGE and APM_LIVE_GENERIC_HOST undocumented.
Test Coverage Expert 0 3 1 Well-structured test permanently dormant; no CI sets the env var; idempotency gap and runtime-skip anti-pattern.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [Test Coverage Expert + OSS Growth Hacker + DevX UX Expert] Create the gitlab.com/microsoft-apm-fixtures/smoke-pkg fixture repo and set APM_LIVE_GENERIC_PACKAGE in CI to activate the smoke test. -- Without the fixture and env var the CI step is permanently dormant and issue Add live e2e smoke install for gitlab.com (generic git host) #1229 remains unclosed. Two test-coverage-expert evidence blocks (outcome: missing, portability-by-manifest) make this the highest-signal gap in the panel. Everything else in this list is gated behind this step.

  2. [Auth Expert + Python Architect] Fix wrong-backend docstrings in the new test module and the stale GenericGitBackend class docstring; confirm issue Add live e2e smoke install for gitlab.com (generic git host) #1229 scope before CI activation. -- gitlab.com dispatches to GitLabBackend (kind=gitlab) not GenericGitBackend (kind=generic). If Add live e2e smoke install for gitlab.com (generic git host) #1229 targets the true GenericGitBackend path (Gitea/Gogs/Forgejo), a separate fixture host is required and this PR only partially addresses the issue.

  3. [Auth Expert + Supply Chain Security Expert] Harden _env_with_isolated_home: strip GITLAB_APM_PAT, GITLAB_TOKEN, GITHUB_TOKEN, and ACTIONS_RUNTIME_TOKEN before the subprocess is spawned. -- These vars silently change the code path or violate least-privilege when forwarded to a subprocess materializing external content. Pre-activation defects: harmless now because the test never runs, but they invalidate what the test proves once live.

  4. [Test Coverage Expert] Replace the runtime pytest.skip() guard on APM_LIVE_GENERIC_PACKAGE with a collection-time pytestmark predicate. -- The runtime skip inside _configured_package() violates the marker-registry pattern test_marker_registry_sync.py enforces. APM_LIVE_GENERIC_PACKAGE is absent from its gate_env_vars set so the anti-pattern is invisible to CI governance today.

  5. [Doc Writer + OSS Growth Hacker] Add the live_generic marker row to integration-testing.md, document APM_LIVE_GENERIC_PACKAGE and APM_LIVE_GENERIC_HOST, and add a CHANGELOG [Unreleased] entry tracking issue Add live e2e smoke install for gitlab.com (generic git host) #1229. -- Contributors cannot discover the live_generic tier or the env vars needed locally. A CHANGELOG entry converts the README's install-from-anywhere claim into a verifiable milestone.

Architecture

classDiagram
    direction LR
    class DependencyReference {
        <<ValueObject>>
        +repo_url str
        +host str
        +is_virtual bool
        +parse(raw) DependencyReference
        +to_canonical() str
    }
    class HostInfo {
        <<ValueObject>>
        +host str
        +kind str
        +api_base str
    }
    class HostBackend {
        <<Protocol>>
        +host_info HostInfo
        +kind str
        +is_generic bool
        +build_clone_https_url(dep_ref, token) str
        +build_commits_api_url(dep_ref, ref) str
    }
    class GitLabBackend {
        <<ConcreteStrategy>>
        +kind str
        +is_generic bool
        +build_clone_https_url(dep_ref, token) str
        +build_commits_api_url(dep_ref, ref) str
    }
    class GenericGitBackend {
        <<ConcreteStrategy>>
        +kind str
        +is_generic bool
        +build_commits_api_url(dep_ref, ref) None
    }
    class AuthResolver {
        <<Strategy>>
        +classify_host(host, port) HostInfo
    }
    class backend_for {
        <<Factory>>
        +backend_for(dep_ref, auth_resolver) HostBackend
    }
    class LockedDependency {
        <<ValueObject>>
        +repo_url str
        +host str
        +resolved_commit str
        +from_dependency_ref(dep_ref, resolved_commit) LockedDependency
    }
    class LiveGenericGitLabTest {
        <<IOBoundary>>
        +_configured_package() DependencyReference
        +_write_consumer_project(project, ref)
        +_env_with_isolated_home(home) dict
        +_read_lockfile(project) dict
        +_locked_dep(lockfile, expected) dict
        +test_live_gitlab_generic_install()
    }
    HostBackend <|.. GitLabBackend
    HostBackend <|.. GenericGitBackend
    AuthResolver ..> HostInfo : returns
    backend_for ..> AuthResolver : calls classify_host
    backend_for ..> GitLabBackend : kind=gitlab for gitlab.com
    backend_for ..> GenericGitBackend : kind=generic for other hosts
    GitLabBackend *-- HostInfo
    GenericGitBackend *-- HostInfo
    LiveGenericGitLabTest ..> DependencyReference : parse and to_canonical
    LiveGenericGitLabTest ..> LockedDependency : validates lockfile shape
    LiveGenericGitLabTest ..> GitLabBackend : exercises via subprocess apm install
    note for GitLabBackend "classify_host(gitlab.com) -> kind=gitlab dispatched via _BACKEND_BY_KIND"
    note for LiveGenericGitLabTest "Docstring says GenericGitBackend; actual runtime path is GitLabBackend"
    class LiveGenericGitLabTest:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    CI["CI: schedule or workflow_dispatch\nbuild-release.yml"] --> PYTEST
    PYTEST["EXEC: uv run pytest test_live_generic_gitlab_install.py\n-m live_generic -o addopts=''"]
    PYTEST --> CFG["_configured_package()\nreads APM_LIVE_GENERIC_PACKAGE env"]
    CFG --> ENVCHECK{APM_LIVE_GENERIC_PACKAGE set?}
    ENVCHECK -->|no| SKIP["pytest.skip() -- test skipped"]
    ENVCHECK -->|yes| PARSE["DependencyReference.parse(raw)\nmodels/dependency/reference.py"]
    PARSE --> HOSTCHECK{dep.host == expected_host?}
    HOSTCHECK -->|no| FAILHOST["pytest.fail() -- wrong host"]
    HOSTCHECK -->|yes| WRITE["FS: _write_consumer_project()\nconsumer/apm.yml via yaml.safe_dump"]
    WRITE --> RUN["EXEC: subprocess.run apm_binary_path install\ntimeout=240s, capture_output=True, check=False"]
    RUN --> CLASSIFY["AuthResolver.classify_host(gitlab.com)\ncore/auth.py -> HostInfo(kind=gitlab)"]
    CLASSIFY --> DISPATCH["backend_for(dep_ref, auth_resolver)\ndeps/host_backends.py -> GitLabBackend"]
    DISPATCH --> CLONEURL["GitLabBackend.build_clone_https_url(dep_ref, token)\ntoken from GITLAB_APM_PAT if set"]
    CLONEURL --> CLONE["NET: git clone (gitlab.com/redacted)
    CLONE --> MATERIALIZE["FS: consumer/apm_modules/group/repo/\nvalidate apm.yml in clone"]
    MATERIALIZE --> LOCKWRITE["FS: consumer/apm.lock.yaml\nresolved_commit = 40-char SHA"]
    LOCKWRITE --> RC{returncode == 0?}
    RC -->|no| FAILRC["assert False with stdout and stderr context"]
    RC -->|yes| MANIFESTS["assert apm_modules rglob apm.yml not empty"]
    MANIFESTS --> READLOCK["FS: _read_lockfile(project)\nyaml.safe_load apm.lock.yaml"]
    READLOCK --> LOCKEDDEP["_locked_dep(lockfile, dep)\nO(N) scan match on host + repo_url"]
    LOCKEDDEP --> FOUND{locked entry found?}
    FOUND -->|no| FAILLOCK["assert False: dep missing from lockfile"]
    FOUND -->|yes| SHACHECK["assert _FULL_SHA_RE.match(resolved_commit)\n^[0-9a-f]{40}$"]
    SHACHECK --> PASS["test PASSED"]
Loading

Recommendation

Ship the scaffolding: zero blocking findings, clean test structure, and the CI reuse pattern is correct. Three follow-ups must be tracked explicitly before issue #1229 is closed: (1) create the fixture repo and activate CI -- the test is inert without it; (2) fix wrong-backend docstrings and confirm #1229 scope -- if the real target is GenericGitBackend a different fixture host is needed; (3) harden _env_with_isolated_home by stripping GitLab and ambient CI tokens before the first live run. The doc and marker-registry gaps should land in the same activation PR rather than accumulating as drift. Tag follow-up (1) as blocking issue #1229 closure.


Full per-persona findings

Python Architect

  • [nit] Test docstring references GenericGitBackend; gitlab.com actually dispatches to GitLabBackend at tests/integration/test_live_generic_gitlab_install.py:110
    AuthResolver.classify_host('gitlab.com') returns HostInfo(kind='gitlab'), dispatched via _BACKEND_BY_KIND to GitLabBackend. The module and function docstrings say "through GenericGitBackend" -- factually wrong.
    Suggested: Change 'through GenericGitBackend' to 'through GitLabBackend (kind=gitlab)'. In host_backends.py remove the stale 'GitLab is currently classified as generic' sentence.

  • [nit] Design patterns: procedural helper decomposition is correct for this scope
    Used in this PR: none -- straight-line procedural test helpers. Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope.

CLI Logging Expert

  • [recommended] No stdout content assertions -- exit code 0 is the only CLI output check at tests/integration/test_live_generic_gitlab_install.py:129
    The test verifies filesystem state but never asserts anything about CLI output. If apm install silently regresses on output format, the test stays green.
    Suggested: After the returncode assert, add: assert dep.host in result.stdout or 'installed' in result.stdout.lower() to pin at least one expected output pattern.

  • [nit] NO_COLOR set via setdefault, inconsistent with GIT_TERMINAL_PROMPT above at tests/integration/test_live_generic_gitlab_install.py:80
    Suggested: Change env.setdefault("NO_COLOR", "1") to env["NO_COLOR"] = "1"

  • [nit] Untruncated result.stdout in failure message -- can be megabytes for a 240s clone at tests/integration/test_live_generic_gitlab_install.py:132
    Suggested: Replace result.stdout with result.stdout[-2000:] and result.stderr with result.stderr[-2000:]

DevX UX Expert

  • [recommended] live_generic not in default addopts exclusion; a future live_generic-only test will bleed into normal CI runs at pyproject.toml:152
    addopts excludes 'benchmark' and 'live' but not 'live_generic'. Current test is safe because it also carries pytest.mark.live, but a contributor adding a live_generic-only test as a template would silently hit the network in unguarded CI jobs.
    Suggested: Extend addopts to "-m 'not benchmark and not live and not live_generic'", or add a note in the marker description.

  • [nit] Skip message shows var name but no example value at tests/integration/test_live_generic_gitlab_install.py:47
    Suggested: pytest.skip(f"{_LIVE_PACKAGE_ENV} not set -- e.g. APM_LIVE_GENERIC_PACKAGE=gitlab.com/<group>/<repo>")

  • [nit] APM_LIVE_GENERIC_HOST: gitlab.com in workflow is redundant with _DEFAULT_HOST in test code
    Suggested: Remove the APM_LIVE_GENERIC_HOST line from the workflow; re-add only when a non-default host variant is wired in.

Supply Chain Security Expert

  • [recommended] Fixture repo resolved at HEAD with no expected-SHA pin -- circular trust at tests/integration/test_live_generic_gitlab_install.py:131
    The test asserts resolved_commit is a 40-char hex SHA but never compares it to a known-good value. A compromised push to the fixture repo would produce a fresh SHA and the test would pass.
    Suggested: Pin APM_LIVE_GENERIC_PACKAGE to a ref@sha form and assert locked.get('resolved_commit') == EXPECTED_SHA.

  • [recommended] os.environ.copy() forwards ambient Actions tokens to subprocess materializing untrusted external content at tests/integration/test_live_generic_gitlab_install.py:72
    GITHUB_TOKEN and ACTIONS_RUNTIME_TOKEN from the runner reach the install process. Violates least-privilege.
    Suggested: Build subprocess env from an explicit allowlist (PATH, TMPDIR, LANG, HOME, GIT_TERMINAL_PROMPT, NO_COLOR, APM_E2E_TESTS) rather than os.environ.copy().

  • [nit] Assertion failure prints raw captured stdout/stderr, potentially surfacing auth diagnostics outside Actions secret masking at tests/integration/test_live_generic_gitlab_install.py:120

  • [nit] No content-level assertion on installed files -- a compromised fixture with extra files passes at tests/integration/test_live_generic_gitlab_install.py:123

OSS Growth Hacker

  • [recommended] No CHANGELOG entry leaves GitLab community outreach value on the table at CHANGELOG.md
    README claims install from anywhere -- GitHub, GitLab, Bitbucket. A CHANGELOG entry converts that claim into a verifiable milestone.
    Suggested: Add under [Unreleased] > Added: 'Live smoke-test infrastructure for apm install gitlab.com/<group>/<repo> via the generic git backend; runs on schedule/dispatch once APM_LIVE_GENERIC_PACKAGE is configured. (test(integration): add live GitLab generic install smoke #1663, closes Add live e2e smoke install for gitlab.com (generic git host) #1229 when fixture is online)'

  • [recommended] Fixture repo does not exist; CI step is permanently inert without a follow-up action
    gitlab.com/microsoft-apm-fixtures/smoke-pkg doesn't exist and vars.APM_LIVE_GENERIC_PACKAGE is unset. Infrastructure may sit dormant indefinitely.
    Suggested: Open a follow-up issue: (1) create fixture repo, (2) set APM_LIVE_GENERIC_PACKAGE in Actions repo variables, (3) confirm smoke test passes on scheduled run.

  • [nit] CI step is silent about its always-skip state at .github/workflows/build-release.yml
    Suggested: # Unset = test gracefully skips; create the fixture repo and set this var to activate.

Auth Expert

  • [recommended] gitlab.com routes to GitLabBackend (kind=gitlab), not GenericGitBackend; test does not cover GenericGitBackend code path at tests/integration/test_live_generic_gitlab_install.py:1
    AuthResolver.classify_host('gitlab.com') returns HostInfo(kind='gitlab') -> GitLabBackend. True GenericGitBackend path (Gitea/Gogs/Forgejo) remains unexercised. If Add live e2e smoke install for gitlab.com (generic git host) #1229 targets GenericGitBackend specifically, a different fixture host is required.
    Suggested: Update module docstring and test name to say 'GitLab backend path (kind=gitlab)' not 'generic git backend'.

  • [recommended] _env_with_isolated_home does not strip GITLAB_APM_PAT or GITLAB_TOKEN; test may silently exercise authenticated GitLab access at tests/integration/test_live_generic_gitlab_install.py:76
    AuthResolver._resolve_token for kind='gitlab' checks GITLAB_APM_PAT then GITLAB_TOKEN. If either is in the runner environment, the subprocess authenticates and the smoke no longer validates anonymous access.
    Suggested: Explicitly pop GITLAB_APM_PAT and GITLAB_TOKEN from the copied env dict.

  • [nit] Stale docstring in GenericGitBackend claims 'GitLab is currently classified as generic' at src/apm_cli/deps/host_backends.py:439

Doc Writer

  • [recommended] live_generic marker row missing from integration-testing.md marker registry table
    The PR registers live_generic in pyproject.toml but adds no row to the marker table. Contributors cannot discover this tier.
    Suggested: Add a row: | live_generic | Tests that install from a live generic git host; skips unless APM_LIVE_GENERIC_PACKAGE is set | export APM_LIVE_GENERIC_PACKAGE=gitlab.com/<group>/<repo> |

  • [recommended] APM_LIVE_GENERIC_PACKAGE and APM_LIVE_GENERIC_HOST are undocumented contributor-facing env vars
    The primary knobs for enabling live_generic tests locally and in CI appear nowhere in the docs.

  • [nit] live marker description says 'real GitHub repos' but now also covers GitLab via live_generic
    Suggested: Change to: 'Tests that hit real remote repos via live network cloning; deselected by default'

  • [nit] No CHANGELOG [Unreleased] entry for the live_generic test tier tracking issue Add live e2e smoke install for gitlab.com (generic git host) #1229

Test Coverage Expert

  • [recommended] Test is permanently dormant: no CI workflow sets APM_LIVE_GENERIC_PACKAGE
    Issue Add live e2e smoke install for gitlab.com (generic git host) #1229 remains undefended. Six-month drift in the backend would go undetected until a user reports it.
    Proof (missing at e2e): .github/workflows/build-release.yml -- proves: apm install gitlab.com path runs automatically in CI before a user encounters a regression [portability-by-manifest, devx]

  • [recommended] No idempotency assertion: running apm install twice on the generic git backend path is not checked at tests/integration/test_live_generic_gitlab_install.py:111
    Lockfile-determinism user promise undefended for this backend path. Grepped tests/integration/ -- test_lock_e2e.py::test_lock_is_idempotent uses local deps only.
    Proof (missing at e2e): tests/integration/test_live_generic_gitlab_install.py -- proves: apm install produces the same apm.lock.yaml on a second consecutive run [portability-by-manifest, devx]

  • [recommended] APM_LIVE_GENERIC_PACKAGE guard is a runtime pytest.skip() inside test body, not a collection-time marker predicate at tests/integration/test_live_generic_gitlab_install.py:44
    Violates the marker-registry pattern. APM_LIVE_GENERIC_PACKAGE absent from test_marker_registry_sync.py gate_env_vars so anti-pattern is invisible to CI governance.
    Suggested: Add requires_live_generic_fixture to _MARKER_CHECKS in conftest.py and add pytest.mark.requires_live_generic_fixture to pytestmark.
    Proof (missing at static): tests/integration/conftest.py -- proves: live_generic test skips at collection time with a clear reason [devx]

  • [nit] assert locked.get('host') == dep.host at line 142 is a tautology given non-None return from _locked_dep at tests/integration/test_live_generic_gitlab_install.py:142
    Suggested: Replace with assert locked.get('repo_url') == dep.repo_url
    Proof (missing at static): tests/integration/test_live_generic_gitlab_install.py -- proves: lockfile entry explicitly records the correct repo_url [portability-by-manifest]

Performance Expert -- inactive

The three changed files touch CI configuration and a new integration test only; none intersect the cache, deps, install phases, pipeline, resolve, or perf-harness fast-path surfaces.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

Generated by PR Review Panel for issue #1663 · sonnet46 15.6M ·

@github-actions github-actions Bot removed the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 19, 2026
@abhinavgautam01

Copy link
Copy Markdown
Contributor Author

@sergio-sisternes-epam thanks for the panel-review.
Addressed the repo-actionable panel follow-ups.

Changes include:

  • corrected the GitLabBackend vs GenericGitBackend wording in the live test and stale GenericGitBackend docstring
  • moved APM_LIVE_GENERIC_PACKAGE to a collection-time marker gate
  • hardened the subprocess environment by stripping GitLab/GitHub/Actions tokens and avoiding ambient git config
  • added required APM_LIVE_GENERIC_EXPECTED_SHA pin validation
  • added truncated failure output, basic output/content assertions, repo_url assertion and second-install lockfile idempotency coverage
  • documented live_generic / fixture env vars and added the changelog entry

Validation passed locally:

  • Ruff passed
  • helper + marker registry tests: 9 passed
  • live smoke test skips cleanly while fixture vars are unset
  • full unit suite passed

The remaining activation step is external maintainer/admin work: create the public GitLab fixture repo and set APM_LIVE_GENERIC_PACKAGE + APM_LIVE_GENERIC_EXPECTED_SHA as repo variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add live e2e smoke install for gitlab.com (generic git host)

3 participants