Skip to content

test(evm_tools): revive the opcode-count test against in-repo inputs - #3336

Closed
mkzung wants to merge 1 commit into
ethereum:forks/amsterdamfrom
mkzung:fix/revive-count-opcodes-test
Closed

mkzung wants to merge 1 commit into
ethereum:forks/amsterdamfrom
mkzung:fix/revive-count-opcodes-test

Conversation

@mkzung

@mkzung mkzung commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes #3326, taking the second option there: rewrite rather than delete, so the
opcode-count tracer gets its first coverage.

The test asked for root_relative, defined in a sibling tree, and pointed at
fixtures/evm_tools_testdata, which no download step provides. It now uses the
client_clis transition inputs. Fixture 3 is the only alloc there with code, 0x600140,
so the call runs PUSH1 then BLOCKHASH. Those inputs are a plain transition rather than a
state test, so the run names the fork instead of passing --state-test.

The --ignore in the spec-tools target goes with it. Only one target carries it on
forks/amsterdam; the issue mentions two, which holds on another branch.

One thing I did not decide: test_execution_specs.py imports Berlin while this names
Frontier. The count is the same either way, but say if Berlin is the intended fork.

Testing

uv run pytest -n 4 tests/evm_tools: 42 passed, with the file collected for the first
time. I restored the pre-change file from git and confirmed it still errors with
fixture 'root_relative' not found, and flipping the expected count to PUSH1: 2 fails,
so the assertion really reads the tracer.

Static analysis: ruff check, ruff format --check, codespell, vulture,
ethereum-spec-lint and mypy all pass.

Claude Code was used for implementation assistance, covering the test rewrite and this
description. All changes have been reviewed, understood and manually tested by me.

The test never ran: it wanted a `root_relative` fixture from a sibling tree
and data no download step provides, so the Justfile ignored it outright.

Fixture 3 is the only client_clis alloc with code, `0x600140`, so the call
runs PUSH1 then BLOCKHASH. A plain transition, not a state test.

Closes ethereum#3326.
@marioevz
marioevz requested review from marioevz and removed request for danceratopz August 13, 2026 17:34
@codecov

codecov Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.50%. Comparing base (4f17c37) to head (8b2ec15).
⚠️ Report is 14 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #3336   +/-   ##
================================================
  Coverage            93.50%   93.50%           
================================================
  Files                  624      624           
  Lines                37070    37070           
  Branches              3394     3394           
================================================
  Hits                 34661    34661           
  Misses                1653     1653           
  Partials               756      756           
Flag Coverage Δ
unittests 93.50% <ø> (ø)

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.

@marioevz

Copy link
Copy Markdown
Member

Hi @mkzung, thank you for putting the effort of addressing the issue with your PR. We currently are undergoing a big refactor of this file and many others in this same folder in #3307, and unfortunately I've rebased your changes and the fix is not compatible with the it, and is going to cause conflicts during the rebase. I think the best course of action for now is to close this PR, and try to address this issue directly in #3307. Again, thanks for this effort! Closing for now.

@marioevz marioevz closed this Aug 13, 2026
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.

chore(testing): delete or revive test_count_opcodes.py (never runnable since introduction)

2 participants