Skip to content

test(integration): extract bringUpRPCNode bring-up helper#467

Merged
bdchatham merged 1 commit into
mainfrom
chore/integration-bringup-rpc-node-helper
Jul 10, 2026
Merged

test(integration): extract bringUpRPCNode bring-up helper#467
bdchatham merged 1 commit into
mainfrom
chore/integration-bringup-rpc-node-helper

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

What

Extracts the fresh-RPC-node bring-up tailCreateNode → WaitReady → WaitCaughtUp → WaitEVMServing with per-gate progress logs — that was duplicated verbatim in provision's node loop and in release_test.go. What's duplicated is the gate protocol + ordering + teardown-registration contract, not incidental line-shape, so the two sites now share one named helper (past CLAUDE.md's "three similar lines" bar because it's a correctness-coupling that must move in lockstep).

The helper (harness_test.go, next to provision)

func bringUpRPCNode(ctx, t, c, hc, nodeSpec sei.NodeSpec) (*sei.Node, error)
  • Error-returning coreprovision propagates up its chain; release wraps in t.Fatal. Same error-core / caller-picks-severity seam as the existing runTaskWait/runTaskComplete.
  • Per-gate logging lives inside the helper — deliberately, so "running"/"caught up"/"EVM serving" is emitted before the next gate starts and a multi-minute stall stays localizable from the pod log while it hangs.
  • Returns node non-nil once created, even on a later gate failure, so the caller still registers it for teardown (if node != nil { append }). Nil only when CreateNode itself fails — matching the prior pre-append-then-wait semantics exactly.

Behavior

Preserving except one approved delta: release now emits the 3 per-gate log lines it lacked (replacing its single combined line) — better stall localization. Error text, %w wrap chains, gate ordering, and teardown registration are unchanged at both sites. (release also gains a %w chain, strictly improving errors.Is/As with no output change.)

The four post-perturbation liveness assertions (post-release / -resync / -fault / -load) are intentionally left inline — a distinct concept with heterogeneous gates and fatal/non-fatal polarity; unifying them would need severity+gate-subset flags, the opposite of what the extraction is for.

Verification

  • gofmt -l, go vet -tags integration, golangci-lint run --build-tags integration — all clean (0 issues)
  • /xreview merge gate: RESOLVED — two blinded independent lenses: idiomatic-reviewer (COMPATIBLE / reads native) + systems-engineer as assigned dissenter, which attacked all six invariants (teardown-registration equivalence, error-text byte-identity, real-time log ordering, hc-hoist inertness, scope, compile) and RATIFIED. Two advisory idiom nits (param-shadow rename, t.Fatal(err)) folded in.

Third and final piece of the integration-harness cleanup arc (after #465 fan-out/docs, #466 seiload lint).

🤖 Generated with Claude Code

The fresh-RPC-node bring-up tail (CreateNode -> WaitReady -> WaitCaughtUp
-> WaitEVMServing, with per-gate progress logs) was duplicated verbatim
in provision's node loop and in release_test.go. What's duplicated is the
gate protocol + ordering + teardown-registration contract, not incidental
line-shape, so extract a named helper both sites share:

- bringUpRPCNode(ctx, t, c, hc, nodeSpec) (*sei.Node, error): error-
  returning core; logs each gate as it passes (kept INSIDE the helper so a
  stall stays localizable in real time); returns the node non-nil once
  created even on a later gate failure, so callers still register it for
  teardown.
- provision propagates the error up its chain; release wraps it in
  t.Fatal. Same error-core / caller-picks-severity seam as runTaskWait.

Behavior-preserving except one approved delta: release now emits the three
per-gate log lines it lacked (replacing one combined line) — better stall
localization. Error text, gate ordering, and teardown semantics unchanged.

The four post-perturbation liveness assertions (post-release/-resync/-fault
/-load) are intentionally left inline — distinct concept, heterogeneous
gates and fatal/non-fatal polarity.

Reviewed via /xreview (idiom + adversarial behavior-preservation, both
RATIFY). gofmt, go vet -tags integration, golangci-lint -tags integration
all clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-harness refactor only; gate order, error wrapping, and teardown registration behavior are preserved by design.

Overview
Pulls the duplicated RPC follower bring-up sequence (CreateNodeWaitReadyWaitCaughtUpWaitEVMServing) into bringUpRPCNode in harness_test.go, used from provision’s node loop and TestRelease.

The helper owns per-gate progress logs (“running”, “caught up”, “EVM serving”) and returns the sei.Node non-nil after a successful create even when a later gate fails, so callers can still append it for teardown. release now gets those three log lines (replacing one combined line); WaitRESTServing stays release-only after bring-up.

Callers keep their own failure style: provision returns wrapped errors; release uses t.Fatal(err).

Reviewed by Cursor Bugbot for commit 46a60eb. Bugbot is set up for automated code reviews on this repo. Configure here.

@bdchatham bdchatham merged commit 059dd64 into main Jul 10, 2026
5 checks passed
@bdchatham bdchatham deleted the chore/integration-bringup-rpc-node-helper branch July 10, 2026 19:07
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