Skip to content

test: Check why a state test transaction was rejected - #1621

Merged
chfast merged 1 commit into
masterfrom
test/statetest-reject-reason
Aug 10, 2026
Merged

test: Check why a state test transaction was rejected#1621
chfast merged 1 commit into
masterfrom
test/statetest-reject-reason

Conversation

@chfast

@chfast chfast commented Aug 9, 2026

Copy link
Copy Markdown
Member

A state test with an expectException only checked that the transaction was rejected, not that it was rejected for the stated reason, so an implementation failing a rule for the wrong cause passed.

Compare the rejection against the fixture's exception. Where the specs draw a distinction evmone does not, or draw it elsewhere, the alternative names are listed in one table in test/utils/error_matching.cpp.

The legacy retesteth exception names are mapped to the spec ones there too, so both runners compare one vocabulary.

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.73%. Comparing base (049d111) to head (04fdfcb).

Files with missing lines Patch % Lines
test/utils/error_matching.cpp 95.45% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1621      +/-   ##
==========================================
- Coverage   97.75%   97.73%   -0.02%     
==========================================
  Files         170      171       +1     
  Lines       15557    15565       +8     
  Branches     3596     3604       +8     
==========================================
+ Hits        15207    15213       +6     
  Misses        263      263              
- Partials       87       89       +2     
Flag Coverage Δ
eest-develop 88.69% <80.00%> (-0.01%) ⬇️
eest-develop-gmp 26.70% <76.66%> (+0.41%) ⬆️
eest-legacy 17.01% <26.66%> (-0.10%) ⬇️
eest-libsecp256k1 28.90% <76.66%> (+0.40%) ⬆️
eest-stable 88.69% <80.00%> (-0.01%) ⬇️
evmone-unittests 93.31% <0.00%> (-0.06%) ⬇️

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

Components Coverage Δ
core 96.08% <ø> (ø)
tooling 92.07% <96.66%> (-0.08%) ⬇️
tests 99.80% <ø> (ø)
Files with missing lines Coverage Δ
test/statetest/statetest_runner.cpp 92.45% <100.00%> (+0.61%) ⬆️
test/utils/blockchaintest_loader.cpp 96.15% <100.00%> (-0.60%) ⬇️
test/utils/statetest.hpp 80.00% <ø> (ø)
test/utils/statetest_loader.cpp 92.55% <100.00%> (+0.02%) ⬆️
test/utils/error_matching.cpp 95.45% <95.45%> (ø)

... and 1 file with indirect coverage changes

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

@chfast
chfast force-pushed the test/statetest-reject-reason branch from 9ba5dc6 to 19d0bc2 Compare August 9, 2026 23:17
@chfast
chfast force-pushed the test/statetest-reject-reason branch from 19d0bc2 to 3d555c5 Compare August 9, 2026 23:47
@chfast
chfast force-pushed the test/statetest-reject-reason branch from 3d555c5 to 3ddecd8 Compare August 10, 2026 06:17
@chfast
chfast changed the base branch from master to state/eest-error-names August 10, 2026 06:24
@chfast
chfast force-pushed the state/eest-error-names branch from 17551b0 to feadd8a Compare August 10, 2026 06:30
@chfast
chfast force-pushed the test/statetest-reject-reason branch 2 times, most recently from 13146b5 to 528d969 Compare August 10, 2026 06:47
Base automatically changed from state/eest-error-names to master August 10, 2026 07:01
@chfast
chfast force-pushed the test/statetest-reject-reason branch 4 times, most recently from 75ef6f3 to 3271aac Compare August 10, 2026 12:45
@chfast
chfast requested a balanced review from Copilot August 10, 2026 13:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Validates transaction rejection reasons and centralizes legacy exception-name mapping across state and blockchain tests.

Changes:

  • Records and checks expected state-test exceptions.
  • Adds shared exception mapping and alias matching.
  • Reuses mapping in the blockchain-test loader.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/utils/statetest.hpp Stores expected exception names.
test/utils/statetest_loader.cpp Loads normalized exceptions.
test/utils/exceptions.hpp Declares exception utilities.
test/utils/exceptions.cpp Implements mapping and matching.
test/utils/CMakeLists.txt Builds the new utilities.
test/utils/blockchaintest_loader.cpp Shares legacy mapping.
test/statetest/statetest_runner.cpp Checks rejection reasons.

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

Comment thread test/utils/error_matching.cpp
Comment thread test/utils/blockchaintest_loader.cpp
@chfast
chfast force-pushed the test/statetest-reject-reason branch from 3271aac to 29faa6c Compare August 10, 2026 13:16
A state test with an `expectException` only checked that the transaction was
rejected, not that it was rejected for the stated reason, so an implementation
failing a rule for the wrong cause passed.

Compare the rejection against the fixture's exception. Where the specs draw a
distinction evmone does not, or draw it elsewhere, the alternative names are
listed in one table in test/utils/error_matching.cpp.

The retesteth vocabulary of the ethereum/tests fixtures is mapped to the spec
names there too, moved out of the blockchain-test loader and extended with the
transaction-level spellings, so both runners compare one vocabulary.
@chfast
chfast force-pushed the test/statetest-reject-reason branch from 29faa6c to 04fdfcb Compare August 10, 2026 13:54
@chfast
chfast requested a balanced review from Copilot August 10, 2026 14:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@chfast
chfast merged commit 8ec683f into master Aug 10, 2026
23 of 25 checks passed
@chfast
chfast deleted the test/statetest-reject-reason branch August 10, 2026 14:51
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