Skip to content

test: Replace gtest assertions in the fixture runners - #1673

Open
chfast wants to merge 1 commit into
masterfrom
test/report-without-gtest
Open

test: Replace gtest assertions in the fixture runners#1673
chfast wants to merge 1 commit into
masterfrom
test/report-without-gtest

Conversation

@chfast

@chfast chfast commented Aug 23, 2026

Copy link
Copy Markdown
Member

evmone-statetest and evmone-blockchaintest use gtest for two unrelated
jobs: as a test registration/reporting framework, and as an assertion
vocabulary inside the runners. This is the first step of removing it,
covering only the second job.

TestReport replaces the assertions. The runners take it by reference and
record into it:

report.start_case(test.name);                 // the fixture's own test name
const auto in_case = report.at(evmc::to_string(rev), '/', case_index);
report.check_eq("state root", state_root, expected.state_hash);
report.fail("transaction validity", "unexpected valid transaction: " + ec.message());

A failure is reported the way pytest reports one, nested and name-first:

tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_contract_create[fork_Prague-state_test]:
  Prague/0:
    state root:
      actual   0x52d14b553a8578ef2c4f2574061ba0140de743e87fb0429e0faffc48b94b1e75
      expected 0xdededededededededededededededededededededededededededededededede

The binaries still drive the tests; a small bridge turns the collected
failures into gtest failures and goes away with the driver.

Notes

There are no macros. std::source_location is not needed because the
check names what it compares in the fixture's own terms, which locates it
better than a C++ line does; and detail built inside the failing branch is
as lazy as the streamed gtest messages were, which matters because one
site formats the whole result state and another reads the error of a
variant that holds one only when the check fails. Without macros the
report is a parameter rather than a global, so nothing prevents running
cases concurrently later.

Behaviour is unchanged. Every former ASSERT_* became an explicit
return with the same scope, and every EXPECT_* a non-returning check.
A green fixture run does not exercise any of this, so the polarity of all
40 sites was reviewed against the original rather than inferred from the
suites passing.

One gtest behaviour is deliberately not preserved: failures are collected
and reported when the test ends instead of printed as they fire, so a run
that is killed loses the failures of the test in flight. The window is one
JSON file, and pytest buffers the same way.

Testing

ctest 1221/1221. Against EEST tests@v20.0.1: state tests 8172/8172,
blockchain tests 8612 passed with the same 6 skipped as before. Instruction
counts are unchanged (343.25 G vs 343.39 G on the state suite).

@codspeed-hq

codspeed-hq Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 129 untouched benchmarks


Comparing test/report-without-gtest (20f7340) with master (c8d6705)

Open in CodSpeed

@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.34799% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.52%. Comparing base (c8d6705) to head (20f7340).

Files with missing lines Patch % Lines
test/blockchaintest/blockchaintest_runner.cpp 55.12% 26 Missing and 9 partials ⚠️
test/statetest/statetest_runner.cpp 82.35% 2 Missing and 1 partial ⚠️
test/utils/test_report.cpp 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1673      +/-   ##
==========================================
- Coverage   97.72%   97.52%   -0.21%     
==========================================
  Files         171      174       +3     
  Lines       15666    15860     +194     
  Branches     3625     3641      +16     
==========================================
+ Hits        15310    15467     +157     
- Misses        269      295      +26     
- Partials       87       98      +11     
Flag Coverage Δ
eest-develop 87.62% <53.24%> (-0.98%) ⬇️
eest-develop-gmp 26.31% <12.08%> (-0.21%) ⬇️
eest-legacy 16.94% <23.44%> (-0.15%) ⬇️
eest-libsecp256k1 28.55% <12.08%> (-0.24%) ⬇️
eest-stable 87.62% <53.24%> (-0.98%) ⬇️
evmone-unittests 93.30% <77.28%> (-0.15%) ⬇️

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

Components Coverage Δ
core 96.11% <ø> (ø)
tooling 90.30% <74.35%> (-1.66%) ⬇️
tests 99.81% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
test/blockchaintest/blockchaintest.cpp 88.70% <100.00%> (+0.99%) ⬆️
test/statetest/statetest.cpp 91.02% <100.00%> (+0.48%) ⬆️
test/unittests/test_report_test.cpp 100.00% <100.00%> (ø)
test/utils/statetest.hpp 73.33% <ø> (-6.67%) ⬇️
test/utils/test_report.hpp 100.00% <100.00%> (ø)
test/utils/test_report.cpp 88.23% <88.23%> (ø)
test/statetest/statetest_runner.cpp 87.71% <82.35%> (-4.74%) ⬇️
test/blockchaintest/blockchaintest_runner.cpp 73.70% <55.12%> (-9.82%) ⬇️
🚀 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/report-without-gtest branch 6 times, most recently from e44c90b to 81fad7d Compare August 24, 2026 18:20
@chfast
chfast requested a balanced review from Copilot August 25, 2026 11:34

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

Replaces GoogleTest assertions inside fixture runners with structured TestReport failure collection while retaining GoogleTest as the test driver.

Changes:

  • Adds scoped, structured failure reporting and rendering.
  • Migrates state and blockchain runners to explicit checks and failures.
  • Bridges collected failures to GoogleTest and adds unit coverage.

Reviewed changes

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

Show a summary per file
File Description
test/utils/test_report.hpp Defines reporting API and failure model.
test/utils/test_report.cpp Formats structured failures.
test/utils/statetest.hpp Adds report parameter to state runner.
test/utils/CMakeLists.txt Builds reporting utilities.
test/unittests/test_report_test.cpp Tests reporting and rendering.
test/unittests/CMakeLists.txt Registers report tests.
test/statetest/statetest.cpp Bridges state-test reports to GoogleTest.
test/statetest/statetest_runner.cpp Replaces runner assertions with report checks.
test/blockchaintest/blockchaintest.cpp Bridges blockchain reports to GoogleTest.
test/blockchaintest/blockchaintest_runner.hpp Adds report parameter to blockchain runner.
test/blockchaintest/blockchaintest_runner.cpp Migrates blockchain assertions to reporting.

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

@chfast
chfast force-pushed the test/report-without-gtest branch 3 times, most recently from 22c8f79 to 231ca83 Compare August 25, 2026 15:22
The state and blockchain test runners used gtest only as an assertion
vocabulary. Replace it with TestReport, which the runners take by
reference and record into: start_case() names the fixture's test, at()
the place within it, check()/check_eq() what is compared, fail() what did
not hold. The binaries still drive the tests, passing a sink that turns
each failure into a gtest one as it is recorded, so a run that dies
part-way still reports what it found. A failure is reported the way
pytest reports one:

    <test name>:
      Prague/0:
        state root:
          actual   0x5f8c...
          expected 0x9e21...

No macros, so no global report to reach from one and no stringified
expressions: a check is named in the fixture's terms instead. Detail too
expensive to format unless a check fails is passed as a callable, run
before the failure is recorded so that the sink sees it whole; one site
dumps the entire result state that way.
@chfast
chfast force-pushed the test/report-without-gtest branch from 231ca83 to 20f7340 Compare August 25, 2026 15:55
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