Skip to content

refactor(ci): run geth benchmark CI only on benchmarks/** branches - #3249

Merged
spencer-tb merged 3 commits into
ethereum:forks/amsterdamfrom
marioevz:split-benchmarks-ci
Jul 29, 2026
Merged

refactor(ci): run geth benchmark CI only on benchmarks/** branches#3249
spencer-tb merged 3 commits into
ethereum:forks/amsterdamfrom
marioevz:split-benchmarks-ci

Conversation

@marioevz

Copy link
Copy Markdown
Member

Description

Benchmark CI previously coupled an external, spec-matched geth dependency into checks that ran on (nearly) every PR: benchmark.yaml's bench-gas job fills fixtures with geth and re-verifies them against EELS, so whenever the EELS spec moved ahead of the pinned geth ref, unrelated spec PRs failed the benchmark job (see #3214: EELS on devnet-8 vs geth on glamsterdam-devnet-7).

This PR separates benchmarking so that mainline development is never blocked by an external client, while the geth-backed work moves to dedicated benchmarks/** branches:

  • test_benchmarking.py defaults to EELS. The benchmark-plugin fills previously hardcoded --evm-bin (defaulting to geth's evm). They now omit --evm-bin when EVM_BIN is unset, falling through to fill's in-repo EELS t8n; setting EVM_BIN still targets a specific binary. This removes the only reason those tests needed an external client.
  • Plugin unit tests fold into the normal unit-test suite. The --ignore=.../test_benchmarking.py is removed from just test-tests, so the benchmark-plugin tests now run as part of the standard unit tests on every forks/** PR via test.yaml — on EELS, with no geth. The redundant test-tests-bench recipe is deleted.
  • test.yaml behaves differently on benchmarks/**. On those branches it builds the pinned geth and sets EVM_BIN, so the same benchmark tests run against a real client; on every other branch the build step is skipped and the tests use EELS. (EVM_BIN is read only by test_benchmarking.py, so this has no effect on the rest of the suite.)
  • benchmark.yaml runs only on benchmarks/**. It now holds the geth-backed sanity checks (bench-gas cross-check, bench-opcode, bench-opcode-config) and the fixture-artifact build. These branches pin a spec version together with a matching geth ref, so the geth↔EELS cross-check is expected to agree there.

Net effect: forks/** PRs get full benchmark-plugin coverage on EELS with no external dependency, and the geth cross-check/fills run where geth and the spec are deliberately kept in sync. This is the first step of a larger benchmarking separation; creating the benchmarks/amsterdam branch and the release/publish flow are follow-ups.

Files changed:

  • packages/testing/.../filler/tests/test_benchmarking.py — EELS-by-default _evm_bin_args() helper.
  • .github/workflows/test.yaml — add benchmarks/**; conditional geth build + EVM_BIN in test-tests.
  • .github/workflows/benchmark.yaml — scope to benchmarks/**; drop unit-tests (now covered by test.yaml).
  • Justfile — un-ignore the benchmark tests in test-tests; delete test-tests-bench.
  • docs/getting_started/verifying_changes.md — update the benchmark row.

Related Issues or PRs

#3244

Checklist

  • Ran fast static checks to avoid CI fails, see Code Standards & Verifying Changes: just static
  • PR title has the form <type>(<area>): <title>, where <type> and <area> come from an appropriate C-<type>, respectively A-<area>, label. The title should match the target squash commit message.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.41%. Comparing base (c69d54b) to head (d1c4be6).
⚠️ Report is 4 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #3249   +/-   ##
================================================
  Coverage            93.41%   93.41%           
================================================
  Files                  625      625           
  Lines                37035    37035           
  Branches              3386     3386           
================================================
  Hits                 34597    34597           
  Misses                1668     1668           
  Partials               770      770           
Flag Coverage Δ
unittests 93.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@spencer-tb spencer-tb 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.

LGTM!

cc @danceratopz for when you are back

@spencer-tb
spencer-tb merged commit 46364f7 into ethereum:forks/amsterdam Jul 29, 2026
21 checks passed
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.

2 participants