-
Notifications
You must be signed in to change notification settings - Fork 63
Migrate seismic e2e tests from subprocess to in-process node harness #338
Copy link
Copy link
Closed
Description
Context
PR #336 fixed immediate lifecycle issues (orphaned processes, port conflicts, retry storms) in the seismic integration test. However, the underlying test architecture still shells out to cargo run --bin seismic-reth, which is fundamentally brittle and slow.
Current problems
- Recompilation at test time —
cargo run --bin seismic-rethreinvokes the compiler inside the test (utils.rs:47). Even though the test binary itself was just compiled, thecargo runtriggers a separate compilation of theseismic-rethbinary, adding significant time especially in CI. - Hardcoded HTTP RPC port —
url()returns127.0.0.1:8545(utils.rs:132), a latent source of conflicts if anything else binds that port. - Stdout parsing for readiness — the test waits for
"Starting consensus engine"in stdout to know the node is up; any log format change silently breaks this. - Silent hangs on read errors —
read_lineerrors in the spawned stdout/stderr tasks panic silently (theJoinHandleis never awaited), causing the test to hang until the nextest timeout. - No isolation — all tests share the same filesystem paths and ports; parallel execution is impossible.
Proposed improvement
Upstream reth provides reth_e2e_test_utils which starts a full node in-process via NodeBuilder::new(config).testing_node(executor). Migrating to this pattern would:
- Eliminate recompilation — the node is compiled once as part of the test binary, not again at runtime via
cargo run. - Remove port conflicts — RPC is in-process, no TCP ports needed.
- Improve isolation — each test gets its own
TempDatabase. - Give direct access — call into node internals (pool, provider, payload builder) instead of going through HTTP RPC.
- Simplify lifecycle — no process management, stdout parsing, or
kill_on_drop.
The target API would look like:
let (mut nodes, _tasks, wallet) = setup::<SeismicNode>(
1, Arc::new(chain_spec), false, eth_payload_attributes,
).await?;
let mut node = nodes.pop().unwrap();
let raw_tx = TransactionTestContext::transfer_tx_bytes(1, wallet.inner).await;
let tx_hash = node.rpc.inject_tx(raw_tx).await?;
let payload = node.advance_block().await?;This may require implementing additional traits on SeismicNode to satisfy the NodeBuilder test infrastructure.
Intermediate step
Before the full migration, the hardcoded port 8545 can be addressed by parsing the bound port from reth's startup logs and passing it to url() dynamically.
References
- PR test: fix seismic integration failure lifecycle #336 (lifecycle fixes): test: fix seismic integration failure lifecycle #336
- Upstream e2e pattern:
crates/ethereum/node/tests/e2e/eth.rs - Current test harness:
crates/seismic/node/src/utils.rs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels