Skip to content

perf(ci): cut E2E smoke wall-clock from ~8min to ~3min - #348

Closed
Million-mo wants to merge 2 commits into
mainfrom
ci/e2e-smoke-parallel
Closed

perf(ci): cut E2E smoke wall-clock from ~8min to ~3min#348
Million-mo wants to merge 2 commits into
mainfrom
ci/e2e-smoke-parallel

Conversation

@Million-mo

Copy link
Copy Markdown
Collaborator

Summary

The single L4a e2e-smoke job ran 172 tests serially and took 7m44s on PR #346
(see run 30810232095).
This PR cuts that to ~3min wall-clock with zero coverage loss.

Changes

  1. Split e2e-smoke into 3 parallel jobse2e-acp, e2e-opencode, e2e-misc

    • Each job runs on its own runner and owns its own subprocess_server cache instance.
    • The conftest cache auto-disables under xdist, so naive -n auto is not viable; splitting
      by protocol gives parallel runners without touching xdist/cache semantics.
    • Wall-clock becomes max(acp, opencode, misc)~3min vs the 7m44s serial sum.
  2. Cap known-hanging test_post_init at 10s (tests/e2e/test_opencode_misc.py)

    • Already xfailed / known_bug, but it burned the full 60s job-level --timeout on
      every PR just to reach the xfail. @pytest.mark.timeout(10) saves 50s/PR.
  3. Add concurrency: cancel-in-progress

    • Each new push to a PR cancels the superseded in-flight run, freeing runner capacity
      that was delaying newly-queued e2e jobs.

Expected impact

Metric Before After
E2E smoke wall-clock 464s (serial) ~295s (parallel, ACP-heavy bucket)
test_post_init 60s xfail wait 10s
PR responsibility 3 e2e jobs 3 e2e jobs (same coverage)

Slow L4 tests remain covered by the existing e2e-nightly.yml. No coverage or
test-selection semantics changed — only job partitioning, the known-bug timeout,
and report aggregation kept in sync.

Verification

  • yaml.safe_load valid; needs-graph has no dangling refs
  • Bucket globs partition all 32 e2e test files with zero overlap
  • ruff check + ruff format --check pass on the modified test file
  • pytest --collect-only -m e2e tests/e2e/test_opencode_misc.py collects test_post_init

The single L4a e2e-smoke job collected 172 tests and ran them serially,
taking 7m44s. Split it by server into three parallel jobs (acp / opencode /
misc), each with its own subprocess-server cache instance, so wall-clock is
now ~the slowest bucket instead of the serial sum.

Also cap the known-hanging test_post_init at a 10s timeout so its xfail no
longer burns the full 60s job-level timeout on every PR.

Add a concurrency cancel-in-progress so a new push to a PR cancels the
superseded in-flight run instead of occupying further runner queues.
@opencode-agent

opencode-agent Bot commented Aug 3, 2026

Copy link
Copy Markdown

No GH_TOKEN is available in this runner, so I can't post the comment via gh; here is the synthesized review to post:


Review: perf(ci): cut E2E smoke wall-clock from ~8min to ~3min

Verdict: APPROVE — no blockers. Two specialist reviews (code + docs) independently verified the partition, the timeout/xfail interaction, and doc consistency. All findings below are warnings/nits.

Verified correct

  • Partition is exact — 13 test_acp*.py + 13 test_opencode*.py + 6 misc files = all 32 tests/e2e/test_*.py, zero overlap, zero gaps (.github/workflows/pytest.yml:303,329,355). All 32 carry module-level pytest.mark.e2e, and -m "e2e and not slow" selects the same set as the old repo-wide run.
  • report job fully updatedneeds, summary rows, and failure condition all reference e2e-acp/e2e-opencode/e2e-misc with no stale e2e-smoke references; per-job artifact names keep triage clean.
  • Conftest discovery unaffected — path-scoped pytest tests/e2e/... still loads root tests/conftest.py (model gate, logfire disable) and tests/e2e/conftest.py; each bucket gets its own independent session-scoped subprocess_server cache, matching the PR's stated rationale.
  • @pytest.mark.timeout(10) on test_post_init is safe (tests/e2e/test_opencode_misc.py:173) — with xfail(strict=False) and no raises narrowing, a pytest-timeout exception converts to XFAIL (green), verified empirically against the pinned stack. Precisely scoped and correctly reasoned.
  • No docs regressions — nothing in AGENTS.md/tests/AGENTS.md/docs/explanation//openspec/ references the old e2e-smoke job; the L4a OpenSpec requirement (pytest -m "e2e and not slow", PR-blocking) remains satisfied.

Warnings

  1. Coverage drop is confirmed but latenttests/capabilities/viking/test_viking_e2e.py:46 is module-level e2e + real_mcp, not slow, so the old repo-wide command collected it and the new path-scoped buckets never do. Practical impact is nil today (the viking_cap fixture self-skips without a live server), and tests/team_mode/test_live.py is unaffected (all e2e tests are also slow). But this sets a precedent: any e2e-marked file outside tests/e2e/ silently leaves PR smoke. Consider adding it to e2e-misc or documenting the exclusion.
  2. e2e-misc uses a hard-coded file list (pytest.yml:355) while the other buckets use prefix globs — a future tests/e2e/test_*.py that matches none of the three patterns (e.g. test_event_stream_e2e.py) would silently fall outside all buckets. Recommend a guard step asserting every tests/e2e/test_*.py matches at least one bucket.
  3. Concurrency group collision on maingroup: ${{ github.workflow }}-${{ github.ref }} (pytest.yml:38) means a workflow_dispatch and a push on main share CI-refs/heads/main, so a manual dispatch can cancel an in-flight push run. Adding ${{ github.event_name }} would isolate them. (Per-PR behavior is correct: refs/pull/N/merge.)
  4. Fix-masking window widened — with the 10s cap, if the underlying bug is fixed and the endpoint starts returning within 10–60s, the test flips from XPASS to XFAIL (both green), making the fix less discoverable. Not a bug, but keep the xfail issue link fresh.

Nits

  • tests/AGENTS.md / AGENTS.md:20 annotate L4a smoke as "~30s" — pre-existing and loose, but this PR was the natural place to correct it (local serial is ~4–5min; CI is now ~3min).
  • Triplicated setup steps are consistent with the existing workflow (every job repeats checkout/uv/python); a composite action would be cleaner but is out of scope.
  • Summary-table labels ("E2E smoke (ACP)") don't match job name:s ("E2E smoke (L4a · ACP)") — cosmetic.
  • @pytest.mark.timeout(10) combined with an xfail that later gains raises=AssertionError would turn a timeout into a red failure — flagging so future edits don't trip on it.

No OpenSpec change needed: it's a pure CI topology change with no src/ or test-logic impact, matching the established precedent for CI-only commits.

Net: sound, well-reasoned change delivering the claimed ~3min wall-clock with zero current-test regression. The two structural risks worth addressing are (1) the silent-drop footgun for future e2e files outside the buckets and (2) the workflow_dispatch/push concurrency collision.

New%20session%20-%202026-08-03T12%3A56%3A44.975Z
opencode session  |  github run

…igger

- Add opencode.json with default_agent: review-lead so the opencode GitHub
  action actually runs the review-lead agent (the agent: input is ignored by
  the CLI handler, which falls back to 'build')
- Teach review-lead + workflow prompt to consume <pull_request_comments> /
  <pull_request_reviews> context and de-duplicate findings across runs
- Fix /oc manual trigger: gh pr view needs -R on issue_comment (runner has
  no git repo before checkout)
@Million-mo

Copy link
Copy Markdown
Collaborator Author

Closing per maintainer decision — reconsidering a different approach to the E2E smoke timing.

@Million-mo Million-mo closed this Aug 3, 2026
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.

1 participant