Skip to content

test: Add the evmone test command - #1685

Open
chfast wants to merge 1 commit into
masterfrom
test/evmone-test-cmd
Open

test: Add the evmone test command#1685
chfast wants to merge 1 commit into
masterfrom
test/evmone-test-cmd

Conversation

@chfast

@chfast chfast commented Aug 31, 2026

Copy link
Copy Markdown
Member

One evmone test command runs both state and blockchain fixtures, deciding which format each fixture is when it runs it: _info.fixture-format names it, and a fixture without one is recognised by the shape only that format has.

What the tool cannot run is treated three ways, because a fixture directory holds more than fixtures. A format it does not run, engine fixtures above all, is skipped. A file holding no fixture at all is skipped whole, which is what EEST's shared pre-allocation is. Anything else it cannot recognise, in a file whose other fixtures are tests, is a fault in the file and fails.

Collection reads no fixture, so a directory costs a walk. A file is one test and reports one result per fixture in it.

evmone-statetest and evmone-blockchaintest are deleted, and CI runs the fixtures through the one command instead.

🤖 Generated with Claude Code

https://claude.ai/code/session_016UHPAGwcwXMjqhTLpT31K7

@codspeed-hq

codspeed-hq Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 129 untouched benchmarks


Comparing test/evmone-test-cmd (9fba008) with master (20c5b22)

Open in CodSpeed

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.72131% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.00%. Comparing base (20c5b22) to head (9fba008).

Files with missing lines Patch % Lines
test/utils/test_driver.cpp 93.81% 1 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1685      +/-   ##
==========================================
- Coverage   98.00%   98.00%   -0.01%     
==========================================
  Files         180      179       -1     
  Lines       16288    16319      +31     
  Branches     3736     3762      +26     
==========================================
+ Hits        15963    15993      +30     
+ Misses        247      244       -3     
- Partials       78       82       +4     
Flag Coverage Δ
eest-develop 82.70% <64.70%> (-4.94%) ⬇️
eest-develop-gmp 26.26% <38.25%> (+0.36%) ⬆️
eest-legacy 17.24% <39.34%> (+0.39%) ⬆️
eest-libsecp256k1 28.44% <38.25%> (+0.36%) ⬆️
eest-stable 82.67% <64.70%> (-4.93%) ⬇️
evmone-unittests 94.29% <96.72%> (+<0.01%) ⬆️

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

Components Coverage Δ
core 96.19% <ø> (ø)
tooling 93.91% <95.58%> (+0.05%) ⬆️
tests 99.81% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
test/unittests/test_driver_test.cpp 100.00% <100.00%> (ø)
test/utils/test_collector.cpp 100.00% <100.00%> (ø)
test/utils/test_driver.hpp 100.00% <100.00%> (ø)
tools/evmone/main.cpp 97.02% <100.00%> (+0.67%) ⬆️
test/utils/test_driver.cpp 95.91% <93.81%> (-2.92%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Adds a unified evmone test command for state and blockchain fixtures.

Changes:

  • Adds lazy fixture collection, classification, filtering, and execution.
  • Exposes per-case state and blockchain loaders.
  • Updates empty/skipped-run exit semantics and integration coverage.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/evmone/main.cpp Adds the test subcommand.
test/utils/test_driver.hpp Defines the nothing-verified exit code.
test/utils/test_driver.cpp Handles fully skipped runs.
test/utils/test_collector.hpp Declares collection settings and API.
test/utils/test_collector.cpp Classifies and runs mixed fixtures.
test/utils/statetest.hpp Exposes single-case loading.
test/utils/statetest_runner.cpp Documents empty-post behavior.
test/utils/statetest_loader.cpp Implements single-case loading.
test/utils/CMakeLists.txt Builds the collector.
test/utils/blockchaintest.hpp Exposes single-case loading.
test/utils/blockchaintest_loader.cpp Publishes the case loader.
test/unittests/test_driver_test.cpp Tests skipped-run semantics.
test/integration/testcmd/one_unsupported_case.json Covers mixed supported and unsupported cases.
test/integration/testcmd_fault/unrecognised_case.json Covers malformed fixture cases.
test/integration/testcmd_cases/case_after_exception.json Covers continuation after exceptions.
test/integration/CMakeLists.txt Adds command-level integration tests.

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

Comment thread test/utils/test_collector.cpp Outdated
Comment thread tools/evmone/main.cpp Outdated
Comment thread test/utils/test_collector.cpp Outdated
@chfast
chfast force-pushed the test/evmone-test-cmd branch 4 times, most recently from 2cfb252 to 3460a62 Compare August 31, 2026 20:52
@chfast
chfast requested a balanced review from Copilot August 31, 2026 20:54

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.

Suppressed comments (1)

test/utils/test_collector.cpp:152

  • When -k matches no cases in a directory, every collected file returns normally and is counted as passed, so the command exits successfully despite executing no fixture. This also makes filtering behavior depend on whether the same fixture was named as a file (which correctly returns NOTHING_VERIFIED). Mark a file with no runnable selected cases as skipped so an empty directory selection propagates NOTHING_VERIFIED.
    // A fixture file in which this tool ran nothing it was asked for is skipped, not passed.
    // TODO: A file whose cases -k all deselected still passes, as it did before this command
    //   existed, so a filter which matches nothing reports a tree of passing tests. Skip it
    //   instead, and an empty selection reaches NOTHING_VERIFIED on its own.
    if (!any_ran && !declined.empty())
        throw UnsupportedTestFeature{declined.front().second};

@chfast
chfast force-pushed the test/evmone-test-cmd branch 4 times, most recently from 0080993 to cdc9e3d Compare September 1, 2026 08:22
@chfast
chfast requested a balanced review from Copilot September 1, 2026 09:33

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

Suppressed comments (1)

test/utils/test_collector.cpp:148

  • With a directory root, -k matching no case leaves each collected file as passed, so the command exits successfully despite verifying nothing; the same filter on a named file correctly returns NOTHING_VERIFIED. Treat a fixture file where no selected case ran as skipped so an entirely empty selection produces the non-success exit code.
    // A fixture file in which this tool ran nothing it was asked for is skipped, not passed.
    // TODO: A file whose cases -k all deselected still passes, as it did before this command
    //   existed, so a filter which matches nothing reports a tree of passing tests. Skip it
    //   instead, and an empty selection reaches NOTHING_VERIFIED on its own.
    if (!any_ran && !declined.empty())
        throw UnsupportedTestFeature{declined.front().second};

Comment thread test/utils/statetest_runner.cpp Outdated
@chfast
chfast force-pushed the test/evmone-test-cmd branch 6 times, most recently from e9d827d to 1d8a9ca Compare September 1, 2026 13:25
@chfast
chfast force-pushed the test/evmone-test-cmd branch 2 times, most recently from 81b6c18 to 4686ff5 Compare September 2, 2026 05:51
chfast added a commit that referenced this pull request Sep 2, 2026
The module collects the tests a run is given, and #1685 is about to make
it collect them from inside a file too, not only find the files. A pure
rename, peeled out of #1685 so the real change there stops showing as a
54%-similar rewrite of this file.
@chfast
chfast force-pushed the test/one-test-per-file branch 11 times, most recently from 418522b to eea90ca Compare September 7, 2026 11:25
chfast added a commit that referenced this pull request Sep 7, 2026
Naming a file used to collect one test per fixture in it, which naming a
directory could not afford: enumerating fixtures means loading every
file, and a release is gigabytes. The two forms counted, filtered and
skipped differently, and only one of them scaled.

A file is now one test either way, in both tools. Nothing is read until
a test runs, so what `--collect-only` lists no longer depends on how the
path was given.

Peeled out of #1685 to land the behaviour change on its own: a few tests
change meaning here, which is worth reading apart from the command that
replaces these tools.
Base automatically changed from test/one-test-per-file to master September 7, 2026 12:22
@chfast
chfast force-pushed the test/evmone-test-cmd branch 3 times, most recently from 980a2f6 to 1753600 Compare September 7, 2026 12:44
@chfast
chfast requested a balanced review from Copilot September 7, 2026 12:58

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.

🟢 Approval recommended

The implementation and regression coverage are consistent, with only a non-blocking documentation wording issue remaining.

Review details
  • Files reviewed: 28/28 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread test/utils/test_driver.hpp Outdated

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.

🟢 Approval recommended

The unified runner behavior, migration, failure handling, and edge cases are consistently implemented and covered.

Review details
  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

One `evmone test` command runs both state and blockchain fixtures,
deciding which format each fixture is when it runs it:
`_info.fixture-format` names it, and a fixture without one is
recognised by the shape only that format has.

What the tool cannot run is treated three ways, because a fixture
directory holds more than fixtures. A format it does not run, engine
fixtures above all, is skipped. A file holding no fixture at all is
skipped whole, which is what EEST's shared pre-allocation is. Anything
else it cannot recognise, in a file whose other fixtures are tests, is
a fault in the file and fails.

Collection reads no fixture, so a directory costs a walk. A file is one
test and reports one result per fixture in it.

evmone-statetest and evmone-blockchaintest are deleted, and CI runs the
fixtures through the one command instead.

Claude-Session: https://claude.ai/code/session_016UHPAGwcwXMjqhTLpT31K7
@chfast
chfast force-pushed the test/evmone-test-cmd branch from 5b4e907 to 9fba008 Compare September 7, 2026 15:28
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