-
Notifications
You must be signed in to change notification settings - Fork 177
Description
We now have 3 classes of test:
- Regular tests.
- Computationally intensive tests (
./tests/benchmark/compute
[*]) - State intensive tests (
./tests/benchmark/state/
).
( [*] These paths don't reflect the current state, but the proposal here: #2208 )
The requirement, as I understand, is:
fill
should only fill regular tests by default (same forexecute
).fill -m benchmarks
should only run tests from./tests/benchmarks/compute/
(same forexecute
).execute -m stateful
should only execute tests from./tests/benchmarks/state/
(fill is currently not required/realistically possible).
This requirement is implemented (via pytest markers) here:
- chore(benchmark): don't fill benchmark tests by default #1920, and
- feat: add bloatnet marker support #2169 (WIP).
Using pytest markers for this case results in a lot of very complex logic, and, as it stands in #2169, adds two additional modifications of pytest_collect_modifyitems()
in two conftests under ./tests/
.
There seems to be a much simpler solution: Just collect tests from the applicable sub-directories. This will:
- Result in much simpler pytest logic.
- Result in much faster test collection times.
This can be achieved by modifying the pytest ini file testpaths
config for the fill
and execute
commands. This config tells pytest where to search for tests.
UX and Corresponding Required Pytest Flags
I'd suggest adding a --mode
flag, which is None
by default.
Relevant pytest config and docs:
testpaths
-o
:-o OVERRIDE_INI, --override-ini=OVERRIDE_INI Override ini option with "option=value" style, e.g. `-o xfail_strict=True -o cache_dir=cache`.
-ignore
:--ignore=path Ignore path during collection (multi-allowed)
- EIP testing (default behavior):
should result in no changes to
fill
testpaths
but automatically add these pytest flags:fill --ignore=tests/benchmarks execute --ignore=tests/benchmarks
- Compute benchmark tests:
is equivalent to modifying
fill --mode=compute
testpaths
via these flags:Also possible:fill -o "testpaths=tests/benchmarks/compute" execute -o "testpaths=tests/benchmarks/compute"
fill -o "testpaths=tests/benchmarks/instructions,tests/benchmarks/scenarios,tests/benchmarks/precompiles"
- State benchmark tests:
is equivalent to modifying
fill --mode=compute
testpaths
via these flags:fill -o "testpaths=tests/benchmarks/state" execute -o "testpaths=tests/benchmarks/state"
Implementation
Two options, I think only the second works:
- Add the flag to the
execute_fill
plugin. But I don't think this works as testpaths are so critical to pytest config, that it won't take effect early enough. But it'd be less code and cleaner. - Modify the behavior via the EEST CLI interface to our pytest commands in https://github.com/ethereum/execution-spec-tests/tree/a2f28413de99d10d349844872a860a340cb2f345/src/cli/pytest_commands . This would mean adding a new Processor class and appending the relevant flags based on the value of
--mode
. As a reference, you can see how--watch
was added in ✨ feat(fill): watch mode #2173.