Skip to content

test: Move the fixture runners into evmone::testutils - #1676

Merged
chfast merged 3 commits into
masterfrom
test/runners-in-testutils
Aug 28, 2026
Merged

test: Move the fixture runners into evmone::testutils#1676
chfast merged 3 commits into
masterfrom
test/runners-in-testutils

Conversation

@chfast

@chfast chfast commented Aug 26, 2026

Copy link
Copy Markdown
Member

Now that the runners no longer use gtest, they can live in evmone::testutils instead of in the
test binaries. statetest_runner.cpp moves unchanged; blockchaintest_runner.cpp only swaps its
own header for blockchaintest.hpp, where run_blockchain_tests() now sits next to
load_blockchain_tests() — that retires blockchaintest_runner.hpp. run_state_test() ends up
defined in the same target as the statetest.hpp declaration it implements. The point is the
build configuration: evmone::testutils is built by EVMONE_TOOLS alone, where GTest is never
fetched, so this is what lets the evmone CLI reach the runners later. Configuring with
EVMONE_TOOLS=ON, EVMONE_TESTING=OFF builds both runner objects into testutils with no mention of
GTest; before this commit that configuration had no runners at all.

The second commit takes what the move enables. The suites the coverage job runs, unit and
integration, reach little of the runners: blockchaintest_runner.cpp sat at 61% line coverage,
with validate_block()'s rejections and the state dumps never executed. EEST does reject blocks
on a green run, but it never reports a disagreement, so the verdicts below a rejection are
unreachable from it. evmone-unittests already links testutils, so five tests can now
drive both runners directly with fixtures broken on purpose, taking the two files to 75% and 96%
lines and the blockchain runner to 100% of functions. tx_invalid_nonce moves out of the
integration tests, where the remaining cases stay as end-to-end smoke; the unit version asserts on
the reported failure's fields rather than regex-matching a subprocess's output.

@codspeed-hq

codspeed-hq Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 129 untouched benchmarks


Comparing test/runners-in-testutils (649799b) with master (6c20562)

Open in CodSpeed

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.86%. Comparing base (6c20562) to head (649799b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1676      +/-   ##
==========================================
+ Coverage   97.56%   97.86%   +0.29%     
==========================================
  Files         174      176       +2     
  Lines       15876    15995     +119     
  Branches     3646     3666      +20     
==========================================
+ Hits        15490    15654     +164     
+ Misses        289      255      -34     
+ Partials       97       86      -11     
Flag Coverage Δ
eest-develop 87.66% <ø> (ø)
eest-develop-gmp 26.08% <0.00%> (-0.20%) ⬇️
eest-legacy 16.82% <0.00%> (-0.13%) ⬇️
eest-libsecp256k1 28.31% <0.00%> (-0.22%) ⬇️
eest-stable 87.66% <ø> (ø)
evmone-unittests 93.88% <100.00%> (+0.53%) ⬆️

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

Components Coverage Δ
core 96.19% <ø> (+0.07%) ⬆️
tooling 92.89% <ø> (+2.20%) ⬆️
tests 99.81% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
test/blockchaintest/blockchaintest.cpp 88.70% <ø> (ø)
test/unittests/blockchaintest_runner_test.cpp 100.00% <100.00%> (ø)
test/unittests/statetest_runner_test.cpp 100.00% <100.00%> (ø)
test/utils/blockchaintest_runner.cpp 87.80% <ø> (ø)
test/utils/statetest_runner.cpp 92.98% <ø> (ø)

... and 2 files 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/runners-in-testutils branch from 375057b to 1913fbe Compare August 26, 2026 14:40
Both runners stopped needing gtest, so they can live in the library the
tools already link instead of in the test binaries. run_state_test() is now
defined next to the declaration in statetest.hpp rather than across the
target boundary from it, and run_blockchain_tests() joins
load_blockchain_tests() in blockchaintest.hpp, which retires
blockchaintest_runner.hpp.

evmone::testutils is built by EVMONE_TOOLS alone, where GTest is never
fetched, so this is what lets the evmone CLI reach the runners later.
@chfast
chfast force-pushed the test/runners-in-testutils branch from d0dc651 to 25c9084 Compare August 26, 2026 19:31
@chfast
chfast requested a balanced review from Copilot August 26, 2026 21:26

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

Moves fixture runners into evmone::testutils, enabling tooling builds without GTest dependencies.

Changes:

  • Relocates state and blockchain runners into testutils.
  • Exposes the blockchain runner through blockchaintest.hpp.
  • Adds direct unit coverage for runner failure paths.

Reviewed changes

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

Show a summary per file
File Description
test/utils/statetest_runner.cpp Adds the relocated state-test runner.
test/utils/CMakeLists.txt Builds both runners into testutils.
test/utils/blockchaintest.hpp Declares the blockchain runner API.
test/utils/blockchaintest_runner.cpp Uses the shared blockchain-test header.
test/unittests/statetest_runner_test.cpp Tests state-runner failure reporting.
test/unittests/blockchaintest_runner_test.cpp Tests validation, RLP, and state-reporting paths.
test/unittests/CMakeLists.txt Registers the new unit tests.
test/statetest/CMakeLists.txt Removes the runner from the executable sources.
test/integration/statetest/tx/invalid_nonce.json Removes the migrated nonce fixture.
test/integration/statetest/CMakeLists.txt Removes the migrated integration test.
test/blockchaintest/CMakeLists.txt Removes locally compiled runner files.
test/blockchaintest/blockchaintest.cpp Includes the shared runner declaration.
test/blockchaintest/blockchaintest_runner.hpp Retires the redundant runner header.

💡 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/runners-in-testutils branch 2 times, most recently from ee811cc to 3b9718a Compare August 27, 2026 10:24
The suites the coverage job runs, unit and integration, reach little of the
runners: blockchaintest_runner.cpp sat at 61% lines, with validate_block()'s
rejections and the state dumps never executed. EEST does reject blocks on a
green run, but never reports a disagreement, so the verdicts below a
rejection are unreachable from it.

Both runners now live in evmone::testutils, which evmone-unittests links,
so a test can drive them directly with a fixture broken on purpose. This
lifts them to 75% and 96%. tx_invalid_nonce moves out of the integration
tests, which keep the remaining cases as end-to-end smoke.
@chfast
chfast force-pushed the test/runners-in-testutils branch from 3b9718a to 2c0bc97 Compare August 28, 2026 08:02
@chfast
chfast requested a balanced review from Copilot August 28, 2026 08:21

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 12 out of 13 changed files in this pull request and generated 1 comment.

Comment thread test/unittests/statetest_runner_test.cpp Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@chfast
chfast merged commit 37d490b into master Aug 28, 2026
25 checks passed
@chfast
chfast deleted the test/runners-in-testutils branch August 28, 2026 10:23
chfast added a commit that referenced this pull request Aug 28, 2026
`EVMONE_TESTING` defaults to `OFF`, and that is the only configuration
where GTest is never
fetched — but every job funnels through a `build` command that passes
`-DEVMONE_TESTING=ON`, so
the default has never been built in CI. Nothing would notice if the code
built under
`EVMONE_TOOLS` grew a dependency on GTest, which matters now that
`evmone::testutils` carries the
fixture runners and #1676 relies on them staying GTest-free.

`gcc-min` and `clang-min` now configure the default first, build it, run
`evmone run` once to
check the tool works at all, and then reconfigure with testing on in the
same directory. Measured
locally: the first pass compiles 54 objects, the second 111, and **none
of the 54 is recompiled** —
165 distinct in total, the same as a single testing-on build. So this
costs an extra CMake
configure and a smoke test, not a second build.
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