Skip to content

python/arco/tests/integration/test_e2e.py is never executed by any CI workflow #365

Description

@ethan-tyler

Severity: Low · Category: testing

Affected code

  • .github/workflows/ci.yml:187-192

Problem

The python-tests job enumerates test paths explicitly rather than running the configured testpaths:

      - name: Run Python unit tests
        run: uv run python -m pytest tests/unit -v
        working-directory: python/arco
      - name: Run Python CLI API tests
        run: uv run python -m pytest tests/integration/test_cli_api.py -v
        working-directory: python/arco

python/arco/tests/integration/ contains two files — test_cli_api.py and test_e2e.py. Only the first is named. test_e2e.py (202 lines, 8 tests at lines 58, 69, 80, 95, 117, 135, 169, 188) is fully hermetic: it uses tmp_path and MonkeyPatch fixtures, contains no pytest.mark.skip, no pytest.mark.integration, and no network/credential dependency. python/arco/pyproject.toml:62-65 sets testpaths = ["tests"] with addopts = ["-ra", "-q", "--strict-markers", "--strict-config"] and no deselection, so a bare pytest would collect and run it; the workflow simply never asks for it. Grep across .github/workflows/ and scripts/ finds no other invocation.

Why it matters

Eight tests covering asset discovery, dependency extraction, partition extraction, manifest generation, manifest git context, manifest fingerprint stability, and the deploy/validate CLI paths are dead weight — they can rot into failing state indefinitely without gating a merge. Manifest fingerprint stability in particular is a cross-language contract with the Rust side; a silent break there is exactly the kind of thing an unrun test is supposed to catch.

Failure scenario

A change to the manifest builder alters field ordering, so test_manifest_fingerprint_stable (test_e2e.py:135) would fail. CI runs pytest tests/unit and pytest tests/integration/test_cli_api.py, neither of which collects that test, so the PR merges green. The fingerprint divergence surfaces later as a mismatch between Python-produced manifests and Rust-side expectations.

Suggested fix

Replace the two enumerated pytest invocations with a single uv run python -m pytest -v (which honors testpaths = ["tests"]), or add an explicit third step uv run python -m pytest tests/integration/test_e2e.py -v. Run it once first — if it currently fails, that is itself the finding.

Verification notes

Existing mitigations found (do not fully close it): Partial, and it materially deflates the stated impact rather than the fact. tests/unit IS run by CI and already covers most of what the orphaned e2e tests exercise: tests/unit/test_discovery.py (9 tests incl. multi-asset discovery and sorted keys), tests/unit/test_manifest_builder.py (9 tests incl. build-with-assets, code_version_id generation, git extraction, git error handling, dirty-repo detection), and tests/unit/test_cli_commands.py / test_cli_main.py. Most importantly, the cross-language canonical-form contract the claim leans on is guarded by python/arco/tests/unit/test_canonical_json_vectors.py::test_canonical_json_golden_vectors, which runs in CI. So a field-ordering change in the manifest builder would very likely be caught by the golden-vector test regardless of test_e2e.py. The e2e file's genuinely unique coverage is narrow: end-to-end deploy --dry-run and validate against a real on-disk project tree, and dependency/partition extraction through the full discovery→definition path.

Caveat / scope: The FAILURE SCENARIO as written is overstated: test_manifest_fingerprint_stable (test_e2e.py:135) only asserts that two consecutive builder.build(assets) calls produce identical canonical JSON after deleting deployedAt and codeVersionId — it is a determinism/idempotence check within one process, not a golden-value fingerprint pinned against the Rust side. A field-ordering change would keep both runs equal and this test would still pass. The real Python↔Rust canonical contract lives in the CI-run test_canonical_json_vectors.py. The defect (file never executed by CI) is certain; the "silent cross-language break" consequence is not well supported. Recommend reporting it as pure testing hygiene — e.g. change ci.yml:190-191 to pytest tests/integration -v or drop to a bare pytest honoring testpaths.

Found during a staff-level audit of origin/main @ c3c0867. Mechanism confirmed by an independent adversarial verifier.

Metadata

Metadata

Assignees

No one assigned

    Labels

    severity: lowHygiene / low impacttestingTest coverage or test quality

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions