Skip to content

Ensure state tests only run against the intended fork #1470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

cburgdorf
Copy link
Contributor

@cburgdorf cburgdorf commented Nov 15, 2018

What was wrong?

The statetest fixtures have different definitions
for each fork and we must ensure to only run
test against against the intended fork (e.g run
Constantinople state tests against Constantinople VM).

We can not rely on pytest -k matching for that
as that matches against an identification string that
includes the path and name of the test file which in
some cases also contains fork names. A test file may
be named "ConstantinopleSomething.json" but still
contains individual definitions per fork.

How was it fixed?

Add a mechanism so that the intended fork can be passed as --fork and then internally filter the generated fixtures against that.

Notice that this does only apply to state tests as these are the only test files expands into multiple test cases per fork.

The following table compares the number of executed tests per fork based on this new scheme.

State test Run before Run after
Frontier 1033 942
Homestead 2196 2031
EIP150 1281 1112
EIP158 1177 1168
Byzantium 4714 4714

Cute Animal Picture

put a cute animal picture link inside the parentheses

The statetest fixtures have different definitions
for each fork and we must ensure to only run
test against against the intended fork (e.g run
Constantinople state tests against Constantinople VM).

We can not rely on `pytest -k` matching for that
as that matches against an identification string that
includes the path and name of the test file which in
some cases also contains fork names. A test file may
be named "ConstantinopleSomething.json" but still
contains individual definitions per fork.
@cburgdorf cburgdorf force-pushed the christoph/tests/do-not-rely-on-k-matching branch from ede82e7 to ba309fd Compare November 15, 2018 13:18
@cburgdorf cburgdorf changed the title Just trying something Ensure state tests only run against the intended fork Nov 15, 2018
@cburgdorf cburgdorf merged commit b4ca90b into ethereum:master Nov 15, 2018
@pipermerriam
Copy link
Member

It's pretty nice that this resulted in a non-trivial drop in total number of tests we run. 👍

veox added a commit to veox/trinity that referenced this pull request May 16, 2019
Mostly a copy-paste from `py-evm` codebase; relevant PRs that
introduced the functionality there:

ethereum/py-evm#1470
ethereum/py-evm#1577
veox added a commit to veox/trinity that referenced this pull request May 20, 2019
Mostly a copy-paste from `py-evm` codebase; relevant PRs that
introduced the functionality there:

ethereum/py-evm#1470
ethereum/py-evm#1577
veox added a commit to veox/trinity that referenced this pull request May 21, 2019
Mostly a copy-paste from `py-evm` codebase; relevant PRs that
introduced the functionality there:

ethereum/py-evm#1470
ethereum/py-evm#1577
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