-
Notifications
You must be signed in to change notification settings - Fork 371
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: #1616 )
The requirement, as I understand, is:
fillshould only fill regular tests by default (same forexecute).fill -m benchmarksshould only run tests from./tests/benchmarks/compute/(same forexecute).execute -m statefulshould 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 execution-spec-tests#1920, and
- feat: add bloatnet marker support execution-spec-tests#2169 (WIP).
Using pytest markers for this case results in a lot of very complex logic, and, as it stands in ethereum/execution-spec-tests#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
filltestpathsbut automatically add these pytest flags:fill --ignore=tests/benchmarks execute --ignore=tests/benchmarks - Compute benchmark tests:
is equivalent to modifying
fill --mode=computetestpathsvia 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=computetestpathsvia 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_fillplugin. 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--watchwas added in ✨ feat(fill): watch mode execution-spec-tests#2173.