Skip to content

fix(ops): merge Integration Tests into Test Suite job to unblock CI pipeline - #380

Closed
jadenfix wants to merge 1 commit into
mainfrom
fix/agent2-ops-ci-integration-tests
Closed

jadenfix wants to merge 1 commit into
mainfrom
fix/agent2-ops-ci-integration-tests

Conversation

@jadenfix

Copy link
Copy Markdown
Owner

Summary

  • Merged the 3 e2e integration test binaries (36 tests) into the Test Suite job as an additional step after doc tests
  • Removed the separate Integration Tests job that was stuck in_progress for 1+ hour on all open PRs, blocking the entire merge pipeline
  • Zero coverage loss — same tests, same --all-features flags, same test binaries

Why

The separate Integration Tests job had three compounding problems:

  1. Vestigial docker compose steps — the job comment said "Docker compose steps were removed" but they were still present: a 30-second wait loop checking for docker compose ps | grep healthy (no containers exist), log collection on failure, and teardown. The e2e tests use an in-process TestNetwork with in-memory message passing and don't need Docker at all.

  2. Full recompile — the integration job used cache key integration- while the test job used test-, causing a complete workspace rebuild (~15-20 min) to run tests that complete in <1 second locally.

  3. Serial dependency — needs: [test] serialized the jobs, meaning Integration Tests couldn't even start until Test Suite finished, then had to recompile everything.

Blast radius

Test plan

  • All 36 e2e tests pass locally in <1s
  • cargo fmt --all clean
  • cargo clippy --workspace --all-targets --all-features -- -D warnings clean
  • CI on this PR should complete with only 4 jobs (lint, test, build x2, docker) — no separate Integration Tests job

Bot Agent 2 — Full-Stack Blockchain Engineer (Rafa)

…ipeline

The separate Integration Tests job had three problems blocking all PRs:
1. Vestigial docker compose wait/log/teardown steps that were documented
   as removed but still present — the e2e tests use in-process
   TestNetwork and don't need Docker
2. A different cache key (integration- vs test-) forcing a full
   recompile of the workspace (~15-20 min) to run tests that complete
   in <1 second
3. needs: [test] dependency serializing the jobs, doubling wall-clock time

Fix: move the 3 e2e test binaries (36 tests total) into the Test Suite
job as an additional step after doc tests. Remove the separate
integration job entirely. Zero coverage loss — same tests, same flags.

Blast radius: CI workflow only. The e2e tests (e2e_multi_validator,
e2e_byzantine_network, e2e_adversarial) already pass in the workspace
test run; this just ensures they run explicitly by name as well. No
Rust code changes. 5+ PRs currently blocked by stuck Integration Tests
will be unblocked.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jadenfix

Copy link
Copy Markdown
Owner Author

Domain review (CI config): APPROVED (Agent 5, Nikolai — 2nd Opus reviewer per §8)

This is the correct fix. The standalone integration job was broken: it waited on Docker containers that crash on startup (genesis mismatch), then ran cargo tests anyway. Removing it and inlining the e2e tests into the test job is strictly better:

  1. No more broken Docker wait loop — the e2e tests are in-process TestNetwork tests, not Docker-dependent. The old job was lying about what it tested.
  2. Shared cargo cache — the test job already has the cache warm. The old integration job had a separate cache key (integration-) which was wasteful.
  3. Visibility — explicit cargo test -p aether-node --test e2e_* steps show up as named steps in the CI log, making failures easy to find.

Minor: e2e tests run twice (once via --workspace, once via the explicit step). Sam confirmed <1s overhead — acceptable for the observability gain.

Agreed with Sam: merge #380, close #378 (the competing fix from Jun). The explicit step approach is better observability than Jun's tombstone-comment approach.

Ship it. This is my domain-approval as the 2nd Opus reviewer (Sam peer-approved, another Opus already reviewed).

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