Skip to content

Commit 13bb6d2

Browse files
feat,test(consume): use regex for --sim.limit; don't use xdist if --sim.parallelism=1 (#1221)
* chore(deps): add pytest-regex as a dependency * feat(consume): use `--regex` for hive `--sim.limit` args * feat(consume): add `--sim.limit` flag to consume * tests(consume): add tests for `--sim.limit` in collect-only mode * chore(tests): fix typo in docstring * docs: update changelog * fix(consume): add quotes around argument value in help string * docs: move this pr's entry from v4 to unreleased * refactor(consume): specify the type of `--sim.limit` to `SimLimitBehavior` * tests(consume): fix-up tests following rebase on #1237 * tests(consume): try to fix macOS fails * test(consume): print output upon non-clean fill exit code * test(consume): fix tox -pytest fails * test(gentest): add debug output in case of unclean fill exit * fix(consume): rebase import issue --------- Co-authored-by: Mario Vega <marioevz@gmail.com>
1 parent 3dc21a8 commit 13bb6d2

File tree

11 files changed

+293
-16
lines changed

11 files changed

+293
-16
lines changed

docs/CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Test fixtures for use by clients are available for each release on the [Github r
1717
- ✨ Allow `consume direct --collect-only` without specifying a fixture consumer binary on the command-line ([#1237](https://github.com/ethereum/execution-spec-tests/pull/1237)).
1818
- ✨ Report the (resolved) fixture tarball URL and local fixture cache directory when `consume`'s `--input` flag is a release spec or URL [#1239](https://github.com/ethereum/execution-spec-tests/pull/1239).
1919
- ✨ EOF Container validation tests (`eof_test`) now generate container deployment state tests, by wrapping the EOF container in an init-container and sending a deploy transaction ([#783](https://github.com/ethereum/execution-spec-tests/pull/783), [#1233](https://github.com/ethereum/execution-spec-tests/pull/1233)).
20+
- ✨ Use regexes for Hive's `--sim.limit` argument and don't use xdist if `--sim.parallelism==1` in the `eest/consume-rlp` and `eest/consume-rlp` simulators ([#1220](https://github.com/ethereum/execution-spec-tests/pull/1220)).
2021

2122
### 📋 Misc
2223

@@ -292,8 +293,8 @@ Test fixtures for use by clients are available for each release on the [Github r
292293
- 🔀 Refactor `gentest` to use `ethereum_test_tools.rpc.rpc` by adding to `get_transaction_by_hash`, `debug_trace_call` to `EthRPC` ([#568](https://github.com/ethereum/execution-spec-tests/pull/568)).
293294
- ✨ Write a properties file to the output directory and enable direct generation of a fixture tarball from `fill` via `--output=fixtures.tgz`([#627](https://github.com/ethereum/execution-spec-tests/pull/627)).
294295
- 🔀 `ethereum_test_tools` library has been split into multiple libraries ([#645](https://github.com/ethereum/execution-spec-tests/pull/645)).
295-
- ✨ Add the consume engine simulator and refactor the consume simulator suite. ([#691](https://github.com/ethereum/execution-spec-tests/pull/691)).
296-
- 🐞 Prevents forcing consume to use stdin as an input when running from hive. ([#701](https://github.com/ethereum/execution-spec-tests/pull/701)).
296+
- ✨ Add the consume engine simulator and refactor the consume simulator suite ([#691](https://github.com/ethereum/execution-spec-tests/pull/691)).
297+
- 🐞 Prevents forcing consume to use stdin as an input when running from hive ([#701](https://github.com/ethereum/execution-spec-tests/pull/701)).
297298

298299
### 📋 Misc
299300

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ dependencies = [
5151
"questionary @ git+https://github.com/tmbo/questionary@ff22aeae1cd9c1c734f14329934e349bec7873bc",
5252
"prompt_toolkit>=3.0.48,<4", # ensure we have a new enough version for ipython
5353
"ethereum-rlp>=0.1.3,<0.2",
54+
"pytest-regex>=0.2.0,<0.3",
5455
]
5556

5657
[project.urls]

src/cli/gentest/tests/test_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,4 @@ def get_mock_context(self: StateTestProvider) -> dict:
9898

9999
## Fill ##
100100
fill_result = runner.invoke(fill, ["-c", "pytest.ini", "--skip-evm-dump", output_file])
101-
assert fill_result.exit_code == 0
101+
assert fill_result.exit_code == 0, f"Fill command failed:\n{fill_result.output}"

src/cli/pytest_commands/consume.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@
1414

1515
def handle_hive_env_flags(args: List[str]) -> List[str]:
1616
"""Convert hive environment variables into pytest flags."""
17-
env_var_mappings = {
18-
"HIVE_TEST_PATTERN": ["-k"],
19-
"HIVE_PARALLELISM": ["-n"],
20-
}
21-
for env_var, pytest_flag in env_var_mappings.items():
22-
value = os.getenv(env_var)
23-
if value is not None:
24-
args.extend(pytest_flag + [value])
17+
# handle hive --sim.limit arg
18+
hive_test_pattern = os.getenv("HIVE_TEST_PATTERN")
19+
if hive_test_pattern and ("--regex" not in args and "--sim.limit" not in args):
20+
args += ["--sim.limit", hive_test_pattern]
21+
hive_parallelism = os.getenv("HIVE_PARALLELISM")
22+
# handle hive --sim.parallelism arg
23+
if hive_parallelism not in [None, "", "1"] and "-n" not in args:
24+
args += ["-n", str(hive_parallelism)]
2525
if os.getenv("HIVE_RANDOM_SEED") is not None:
2626
warnings.warn("HIVE_RANDOM_SEED is not yet supported.", stacklevel=2)
2727
if os.getenv("HIVE_LOGLEVEL") is not None:

src/pytest_plugins/consume/consume.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""A pytest plugin providing common functionality for consuming test fixtures."""
22

3+
import re
34
import sys
45
import tarfile
56
from dataclasses import dataclass
@@ -127,6 +128,56 @@ def download_and_extract(url: str, base_directory: Path) -> Tuple[bool, Path]:
127128
return already_cached, extract_to / "fixtures"
128129

129130

131+
class SimLimitBehavior:
132+
"""Represents options derived from the `--sim.limit` argument."""
133+
134+
def __init__(self, pattern: str, collectonly: bool = False): # noqa: D107
135+
self.pattern = pattern
136+
self.collectonly = collectonly
137+
138+
@staticmethod
139+
def _escape_id(pattern: str) -> str:
140+
"""
141+
Escape regex char in the pattern; prepend and append '.*' (for `fill` IDs).
142+
143+
The `pattern` is prefixed and suffixed with a wildcard match to allow `fill`
144+
test case IDs to be specified, otherwise the full `consume` test ID must be
145+
specified.
146+
"""
147+
return f".*{re.escape(pattern)}.*"
148+
149+
@classmethod
150+
def from_string(cls, pattern: str) -> "SimLimitBehavior":
151+
"""
152+
Parse the `--sim.limit` argument and return a `SimLimitBehavior` instance.
153+
154+
If `pattern`:
155+
- Is "collectonly", enable collection mode without filtering.
156+
- Starts with "collectonly:", enable collection mode and use the rest as a regex pattern.
157+
- Starts with "id:", treat the rest as a literal test ID and escape special regex chars.
158+
- Starts with "collectonly:id:", enable collection mode with a literal test ID.
159+
"""
160+
if pattern == "collectonly":
161+
return cls(pattern=".*", collectonly=True)
162+
163+
if pattern.startswith("collectonly:id:"):
164+
literal_id = pattern[len("collectonly:id:") :]
165+
if not literal_id:
166+
raise ValueError("Empty literal ID provided.")
167+
return cls(pattern=cls._escape_id(literal_id), collectonly=True)
168+
169+
if pattern.startswith("collectonly:"):
170+
return cls(pattern=pattern[len("collectonly:") :], collectonly=True)
171+
172+
if pattern.startswith("id:"):
173+
literal_id = pattern[len("id:") :]
174+
if not literal_id:
175+
raise ValueError("Empty literal ID provided.")
176+
return cls(pattern=cls._escape_id(literal_id))
177+
178+
return cls(pattern=pattern)
179+
180+
130181
def pytest_addoption(parser): # noqa: D103
131182
consume_group = parser.getgroup(
132183
"consume", "Arguments related to consuming fixtures via a client"
@@ -173,6 +224,22 @@ def pytest_addoption(parser): # noqa: D103
173224
"The --html flag can be used to specify a different path."
174225
),
175226
)
227+
consume_group.addoption(
228+
"--sim.limit",
229+
action="store",
230+
dest="sim_limit",
231+
type=SimLimitBehavior.from_string,
232+
default=SimLimitBehavior(".*"),
233+
help=(
234+
"Filter tests by either a regex pattern or a literal test case ID. To match a "
235+
"test case by its exact ID, prefix the ID with `id:`. The string following `id:` "
236+
"will be automatically escaped so that all special regex characters are treated as "
237+
"literals. Without the `id:` prefix, the argument is interpreted as a Python regex "
238+
"pattern. To see which test cases are matched, without executing them, prefix with "
239+
'`collectonly:`, e.g. `--sim.limit "collectonly:.*eip4788.*fork_Prague.*"`. '
240+
"To list all available test case IDs, set the value to `collectonly`."
241+
),
242+
)
176243

177244

178245
@pytest.hookimpl(tryfirst=True)
@@ -231,6 +298,17 @@ def pytest_configure(config): # noqa: D103
231298
)
232299
config.test_cases = TestCases.from_index_file(index_file)
233300

301+
if config.option.sim_limit:
302+
if config.option.dest_regex != ".*":
303+
pytest.exit(
304+
"Both the --sim.limit (via env var?) and the --regex flags are set. "
305+
"Please only set one of them."
306+
)
307+
config.option.dest_regex = config.option.sim_limit.pattern
308+
if config.option.sim_limit.collectonly:
309+
config.option.collectonly = True
310+
config.option.verbose = -1 # equivalent to -q; only print test ids
311+
234312
if config.option.collectonly:
235313
return
236314
if not config.getoption("disable_html") and config.getoption("htmlpath") is None:

src/pytest_plugins/consume/direct/conftest.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
from ethereum_test_fixtures import BaseFixture
2020
from ethereum_test_fixtures.consume import TestCaseIndexFile, TestCaseStream
2121
from ethereum_test_fixtures.file import Fixtures
22-
23-
from ..consume import FixturesSource
22+
from pytest_plugins.consume.consume import FixturesSource
2423

2524

2625
class CollectOnlyCLI(EthereumCLI):

src/pytest_plugins/consume/direct/test_via_direct.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77

88
from ethereum_test_fixtures import BaseFixture, FixtureConsumer, FixtureFormat
99
from ethereum_test_fixtures.consume import TestCaseIndexFile, TestCaseStream
10-
11-
from ..decorator import fixture_format
10+
from pytest_plugins.consume.decorator import fixture_format
1211

1312

1413
@fixture_format(*BaseFixture.formats.values())
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
"""Test the consume plugins with various cli arguments."""
2+
3+
import re
4+
import shutil
5+
from pathlib import Path
6+
from typing import List
7+
8+
import pytest
9+
from click.testing import CliRunner
10+
from pytest import Pytester, TempPathFactory
11+
12+
from cli.pytest_commands.fill import fill
13+
14+
15+
@pytest.fixture(scope="module")
16+
def test_paths() -> list[Path]:
17+
"""Specify the test paths to be filled."""
18+
return [
19+
Path("tests/istanbul/eip1344_chainid/test_chainid.py"),
20+
]
21+
22+
23+
@pytest.fixture(scope="module")
24+
def consume_test_case_ids() -> list[str]:
25+
"""Hard-coded expected output of `consume direct --collectonly -q`."""
26+
return [
27+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Cancun-blockchain_test_from_state_test]]",
28+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Paris-blockchain_test_from_state_test]]",
29+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-blockchain_test_from_state_test]]",
30+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Cancun-state_test]]",
31+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Paris-state_test]]",
32+
"src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-state_test]]",
33+
]
34+
35+
36+
@pytest.fixture(scope="module")
37+
def fill_fork_from() -> str:
38+
"""Specify the value for `fill`'s `--from` argument."""
39+
return "Paris"
40+
41+
42+
@pytest.fixture(scope="module")
43+
def fill_fork_until() -> str:
44+
"""Specify the value for `fill`'s `--until` argument."""
45+
return "Cancun"
46+
47+
48+
@pytest.fixture(scope="module")
49+
def fixtures_dir(tmp_path_factory: TempPathFactory) -> Path:
50+
"""Define the temporary test fixture directory for fill output."""
51+
return tmp_path_factory.mktemp("fixtures")
52+
53+
54+
@pytest.fixture(autouse=True, scope="module")
55+
def fill_tests(
56+
fixtures_dir: Path, fill_fork_from: str, fill_fork_until: str, test_paths: List[Path]
57+
) -> None:
58+
"""Run fill to generate test fixtures for use with testing consume."""
59+
fill_result = CliRunner().invoke(
60+
fill,
61+
[
62+
"-c",
63+
"pytest.ini",
64+
"--skip-evm-dump",
65+
"-m",
66+
"(not blockchain_test_engine) and (not eip_version_check)",
67+
f"--from={fill_fork_from}",
68+
f"--until={fill_fork_until}",
69+
f"--output={str(fixtures_dir)}",
70+
*[str(p) for p in test_paths],
71+
# if we fill many tests, it might help to add -n 8/auto
72+
],
73+
)
74+
assert fill_result.exit_code == 0, f"Fill command failed:\n{fill_result.output}"
75+
76+
77+
@pytest.fixture(autouse=True, scope="function")
78+
def test_fixtures(pytester: Pytester, fixtures_dir: Path, fill_tests: None) -> List[Path]:
79+
"""
80+
Copy test fixtures from the regular temp path to the pytester temporary dir.
81+
82+
We intentionally copy the `.meta/index.json` file to test its compatibility with consume.
83+
"""
84+
test_fixtures = []
85+
for json_file in fixtures_dir.rglob("*.json"):
86+
target_dir = Path(pytester.path) / json_file.parent
87+
if not target_dir.exists():
88+
target_dir.mkdir(parents=True)
89+
pytester.copy_example(name=json_file.as_posix())
90+
shutil.move(json_file.name, target_dir / json_file.name)
91+
if ".meta" not in str(json_file):
92+
test_fixtures.append(json_file)
93+
return test_fixtures
94+
95+
96+
@pytest.fixture(autouse=True)
97+
def copy_consume_test_paths(pytester: Pytester):
98+
"""Specify and copy the consume test paths to the testdir."""
99+
local_test_paths = [Path("src/pytest_plugins/consume/direct/test_via_direct.py")]
100+
for test_path in local_test_paths:
101+
target_dir = Path(pytester.path) / test_path.parent
102+
target_dir.mkdir(parents=True, exist_ok=True)
103+
pytester.copy_example(name=str(test_path))
104+
pytester.copy_example(name=str(test_path.parent / "conftest.py"))
105+
shutil.move(test_path.name, target_dir / test_path.name)
106+
shutil.move("conftest.py", target_dir / "conftest.py")
107+
108+
109+
single_test_id = "src/pytest_plugins/consume/direct/test_via_direct.py::test_fixture[CollectOnlyFixtureConsumer-tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-state_test]]" # noqa: E501
110+
111+
112+
@pytest.mark.parametrize(
113+
"extra_args, expected_filter_pattern",
114+
[
115+
pytest.param(
116+
["--collect-only", "-q"],
117+
re.compile(r".*"),
118+
id="no_extra_args",
119+
),
120+
pytest.param(
121+
["--collect-only", "-q", "--sim.limit", ".*fork_Cancun.*"],
122+
re.compile(".*Cancun.*"),
123+
id="sim_limit_regex",
124+
),
125+
pytest.param(
126+
["--sim.limit", "collectonly:.*fork_Cancun.*"],
127+
re.compile(".*Cancun.*"),
128+
id="sim_limit_collect_only_regex",
129+
),
130+
pytest.param(
131+
[
132+
"--collect-only",
133+
"-q",
134+
"--sim.limit",
135+
f"id:{single_test_id}",
136+
],
137+
re.compile(re.escape(f"{single_test_id}")),
138+
id="sim_limit_id",
139+
),
140+
pytest.param(
141+
[
142+
"--sim.limit",
143+
f"collectonly:id:{single_test_id}",
144+
],
145+
re.compile(
146+
re.compile(re.escape(f"{single_test_id}")),
147+
),
148+
id="sim_limit_collect_only_id",
149+
),
150+
],
151+
)
152+
def test_consume_simlimit_collectonly(
153+
pytester: Pytester,
154+
fixtures_dir: Path,
155+
consume_test_case_ids: List[str],
156+
extra_args: List[str],
157+
expected_filter_pattern: re.Pattern,
158+
) -> None:
159+
"""Test consume's --sim.limit argument in collect-only mode."""
160+
ini_file = "pytest-consume.ini"
161+
pytester.copy_example(name=ini_file)
162+
consume_test_path = "src/pytest_plugins/consume/direct/test_via_direct.py"
163+
args = [
164+
"-c",
165+
ini_file,
166+
"--input",
167+
str(fixtures_dir),
168+
consume_test_path,
169+
*extra_args,
170+
]
171+
result = pytester.runpytest(*args)
172+
assert result.ret == 0
173+
stdout_lines = str(result.stdout).splitlines()
174+
collected_test_ids = [
175+
line
176+
for line in stdout_lines
177+
if line.strip()
178+
and "tests collected in" not in line
179+
and "pytest-regex selected" not in line
180+
]
181+
expected_collected_test_ids = [
182+
line for line in consume_test_case_ids if expected_filter_pattern.search(line)
183+
]
184+
assert set(collected_test_ids) == set(expected_collected_test_ids)

src/pytest_plugins/filler/tests/test_filler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
"""
2-
Test the forks plugin.
2+
Test the filler plugin.
33
"""
44

55
import configparser

0 commit comments

Comments
 (0)