Skip to content

test(builder): cover Builder lifecycle regressions - #9932

Merged
nflaig merged 15 commits into
ChainSafe:unstablefrom
krisoshea-eth:krisoshea/lod-45-test-01-add-missing-tests-from-cli-01-sign-01-api-01
Sep 9, 2026
Merged

nflaig merged 15 commits into
ChainSafe:unstablefrom
krisoshea-eth:krisoshea/lod-45-test-01-add-missing-tests-from-cli-01-sign-01-api-01

Conversation

@krisoshea-eth

@krisoshea-eth krisoshea-eth commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Motivation

The Builder readiness, genesis polling, identity, and cancellation paths still lacked focused regression coverage after the initial Builder implementation. This PR adds that coverage without changing runtime behavior.

Changes

  • Cover syncing, EL-offline, non-ok, unreachable, optimistic, and eventual-ready transitions.
  • Prove cancellation during readiness and genesis polling stops further requests and timers.
  • Keep node-version lookup failure non-fatal.
  • Cover expected not-found genesis responses and unexpected genesis failures without noisy stack traces.
  • Distinguish an empty successful Builder lookup, a non-active Builder status, and a failed beacon node response.
  • Preserve useful typed API error detail on status lookup failure.
  • Cover cancellation while waiting for an unknown Builder to appear.
  • Extend the Builder API stub with getGenesis and getNodeVersionV2.

This is a test-only change. It does not alter polling intervals, logs, API contracts, public exports, or Builder runtime behavior.

Historical validation base: f22c5ce63e, including merged #9832 and #9964.

Reviewed head on 2026-09-07: 9898d54ca9 (source unchanged in this pass).

Project issue: krisoshea-eth/lodestar#45.

Testing

Previously recorded validation with Node 24.13.0 and pnpm 11.0.0:

  • 29 focused readiness, genesis, and identity tests
  • all 40 Builder unit tests
  • 8 existing Builder CLI and fee-recipient parser tests
  • Builder package type-check
  • Builder package lint
  • git diff --check

The Builder coverage comprises 7 readiness tests, 3 genesis tests, 19 identity tests, 5 BuilderStatusTracker tests, and 6 signer tests. Existing CLI tests continue to cover zero fee-recipient and unscheduled Gloas validation.

Earlier CI results belong to their recorded heads. See the Checks tab for runs on the current head; absent runs or runs awaiting approval must not be treated as passing. This review did not refresh the branch from unstable.

AI assistance disclosure

  • External Contributors: I have read the contributor guidelines and disclosed my usage of AI below.

AI assistance was used for codebase research, test drafting, validation, and review. I reviewed and revised the submitted tests and PR text, made manual edits and technical decisions, and take responsibility for the final result.

@krisoshea-eth
krisoshea-eth marked this pull request as ready for review August 27, 2026 15:47
@krisoshea-eth
krisoshea-eth requested a review from a team as a code owner August 27, 2026 15:47
@markolazic01

markolazic01 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

maybe change the title here, Gate A is an unknown term for external viewers.

@krisoshea-eth krisoshea-eth changed the title test(builder): cover remaining Gate A lifecycle regressions test(builder): cover Builder lifecycle regressions Sep 2, 2026
@markolazic01

Copy link
Copy Markdown
Contributor

LGTM

@nflaig nflaig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @krisoshea-eth and sorry for the delay

@nflaig
nflaig merged commit 2c42b1a into ChainSafe:unstable Sep 9, 2026
21 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Lodestar Team Coordination Sep 9, 2026
@wemeetagain

Copy link
Copy Markdown
Member

🎉 This PR is included in v1.48.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants