Skip to content

Enable Amsterdam spec tests - #2537

Open
Baltoli wants to merge 1 commit into
eip-8024-dupn-swapn-exchangefrom
bruce/enable-amsterdam-spec-tests
Open

Baltoli wants to merge 1 commit into
eip-8024-dupn-swapn-exchangefrom
bruce/enable-amsterdam-spec-tests

Conversation

@Baltoli

@Baltoli Baltoli commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

The inverse of #2535. That PR pinned the combined Amsterdam fixture bundle and switched the MONAD_NEXT Amsterdam spec job off behind a blanket BlockchainTests.* exclusion, with a WILL_FAIL guard so ctest stayed green while nothing ran. Now that every EIP in that bundle is implemented on this stack (#2439#2499#2512#2469#2407), the suite runs for real, so this drops the scaffolding.

Change

  • exclude/MONAD_NEXT_amsterdam.cmake: replace the blanket BlockchainTests.* entry with the single EIP-4844 blob fixture Monad cannot run.
  • CMakeLists.txt: remove the WILL_FAIL guard and its comment, since the guard can never fire again.

The feature PRs underneath leave the exclusion file alone. The fixtures are generated with the whole fork active, so opening or filtering the suite part-way up the stack either runs zero tests with the guard disarmed (which is what broke CI on #2512) or fails on the EIPs not yet in the tree (#2469). Enabling belongs in one place, here.

Measured

Against tests-monad_eip7708+7843+7981+7997+8024+8246@v0.2.0 on this branch, unfiltered:

fixtures 711
pass 697
skip 13
fail 1

The single failure is the blob fixture already excluded, so the ctest job passes as configured. The skips are the for_monad_tentomonad_nextattime15k fork-transition fixtures, whose network has no revision_map.hpp entry — a harness gap that predates this stack.

Sits on top of #2407.

🤖 Generated with Claude Code

https://claude.ai/code/session_019VKRBQPBtf5k1JbBH28hVe

@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from 39a1d28 to c153a4d Compare September 4, 2026 14:29
ryankeleti
ryankeleti previously approved these changes Sep 4, 2026
Copilot AI lite review requested due to automatic review settings September 7, 2026 09:24
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from c153a4d to b8b7c06 Compare September 7, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped to test configuration and removes now-obsolete scaffolding, with only a minor comment typo noted.

Pull request overview

Enables the MONAD_NEXT Amsterdam spec suite to run normally by narrowing the exclusion list to the single known-incompatible fixture and removing the temporary WILL_FAIL scaffolding that existed only to keep ctest green when zero tests were selected.

Changes:

  • Replace the blanket BlockchainTests.* exclusion with a single excluded Amsterdam fixture.
  • Remove the WILL_FAIL guard in the Amsterdam spec job now that the suite is intended to execute.

Verdict: CORRECT

🤖 Generated with Claude Code

File summaries
File Description
test/ethereum_test/exclude/MONAD_NEXT_amsterdam.cmake Narrows the exclusion list from a blanket skip to a single excluded fixture.
test/ethereum_test/CMakeLists.txt Removes the WILL_FAIL workaround so the test job reports real failures/zero-test runs.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/ethereum_test/exclude/MONAD_NEXT_amsterdam.cmake Outdated
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from b8b7c06 to d7c1248 Compare September 7, 2026 09:41
@Baltoli Baltoli changed the title test: run the MONAD_NEXT Amsterdam spec suite unconditionally Enable Amsterdam spec tests Sep 7, 2026
@Baltoli
Baltoli requested a review from andreaslyn as a code owner September 7, 2026 12:25
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from d7c1248 to 575f9a8 Compare September 7, 2026 12:25
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from 575f9a8 to fcf9330 Compare September 7, 2026 14:25
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from fcf9330 to 2a0e95f Compare September 8, 2026 10:02
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from 2a0e95f to 28564b9 Compare September 9, 2026 10:23
@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from 989f612 to 27e4d58 Compare September 10, 2026 10:51
ryankeleti
ryankeleti previously approved these changes Sep 10, 2026
@brett-monad
brett-monad force-pushed the bruce/enable-amsterdam-spec-tests branch from 27e4d58 to cb81a64 Compare September 10, 2026 13:59
@ryankeleti
ryankeleti force-pushed the bruce/enable-amsterdam-spec-tests branch from cb81a64 to 31bf8b1 Compare September 10, 2026 18:40
@ryankeleti
ryankeleti force-pushed the bruce/enable-amsterdam-spec-tests branch from 31bf8b1 to 5c68195 Compare September 10, 2026 19:18
dhil
dhil previously approved these changes Sep 11, 2026
@brett-monad
brett-monad force-pushed the bruce/enable-amsterdam-spec-tests branch 2 times, most recently from 1279aac to eef8e3e Compare September 11, 2026 12:18
dhil
dhil previously approved these changes Sep 11, 2026
@brett-monad
brett-monad force-pushed the bruce/enable-amsterdam-spec-tests branch 2 times, most recently from 24de545 to ca1499e Compare September 11, 2026 18:51
dhil
dhil previously approved these changes Sep 11, 2026
@Baltoli
Baltoli dismissed dhil’s stale review September 14, 2026 12:29

The merge-base changed after approval.

@Baltoli
Baltoli force-pushed the bruce/enable-amsterdam-spec-tests branch from ca1499e to 50a9fc0 Compare September 14, 2026 12:31
@brett-monad
brett-monad force-pushed the bruce/enable-amsterdam-spec-tests branch from 50a9fc0 to c040306 Compare September 15, 2026 16:34
Drop the blanket BlockchainTests.* entry from exclude/MONAD_NEXT_amsterdam.cmake,
leaving the exclusion list empty, and drop the WILL_FAIL scaffolding around the
job and the comment explaining it. The guard existed to keep ctest green while
the whole bundle was excluded, which the runner's "No tests were run" check
turned into a non-zero exit. With every Amsterdam EIP in the pinned bundle now
implemented on the commits below, the guard can never fire again and the job
runs like every other spec suite.

The feature commits underneath leave the exclusion file alone. The fixtures are
generated with the whole fork active, so opening the suite (or filtering it)
part-way up the stack either runs zero tests with the guard disarmed or fails
on the EIPs not yet in the tree; enabling belongs in one place, here.

Measured against tests-monad_eip7708+7843+7981+7997+8024+8246@v0.3.0 on this
branch: 723 fixtures, 710 pass, 13 skip, 0 fail. The v0.3.0 bundle no longer
contains type-3 (blob) transactions, so the eip7981 blob fixture that v0.2.0
needed excluded is gone. The skips are the for_monad_tentomonad_nextattime15k
fork-transition fixtures, whose network has no revision_map.hpp entry; that
harness gap predates this change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ttcfi1kTMXgxbhZ9hF52s3
@brett-monad
brett-monad force-pushed the bruce/enable-amsterdam-spec-tests branch from c040306 to ca34cc2 Compare September 15, 2026 20:27
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.

5 participants