feat(pytest): use node id as fixture name; remove "=" in ids (ethereu…

…m#342)

Co-authored-by: spencer <[email protected]>

docs/CHANGELOG.md

@@ -11,13 +11,28 @@ Test fixtures for use by clients are available for each release on the [Github r
 ### 🛠️ Framework
 
 - ✨ Add a `--single-fixture-per-file` flag to generate one fixture JSON file per test case ([#331](https://github.com/ethereum/execution-spec-tests/pull/331)).
+- 🔀 Rename test fixtures names to match the corresponding pytest node ID as generated using `fill` ([#342](https://github.com/ethereum/execution-spec-tests/pull/342)).
+- 💥 Replace "=" with "_" in pytest node ids and test fixture names ([#342](https://github.com/ethereum/execution-spec-tests/pull/342)).
 
 ### 🔧 EVM Tools
 
 ### 📋 Misc
 
 - 🔀 Docs: Update `t8n` tool branch to fill tests for development features in the [readme](https://github.com/ethereum/execution-spec-test) ([#338](https://github.com/ethereum/execution-spec-tests/pull/338)).
 
+## Breaking Changes
+
+1. In this release the pytest node ID is now used for fixture names (previously only the test parameters were used), this should not be breaking. However, "=" in both node IDs and therefore fixture names, have been replaced with "_", which may break tooling that depends on the "=" character.
+
+   Pytest node ID example:
+
+   - Previous node ID: `tests/frontier/opcodes/test_dup.py::test_dup[fork=Frontier]`
+   - New node ID: `tests/frontier/opcodes/test_dup.py::test_dup[fork_Frontier]`
+
+   Fixture name example:
+   - Previous fixture name: `fork=Frontier`
+   - New fixture name: `fork_Frontier`
+
 ## [v1.0.6](https://github.com/ethereum/execution-spec-tests/releases/tag/v1.0.6) - 2023-10-19: 🐍🏖️ Cancun Devnet 10
 
 ### 🧪 Test Cases

docs/getting_started/executing_tests_command_line.md

@@ -71,7 +71,7 @@ fill tests/shanghai/eip3651_warm_coinbase/test_warm_coinbase.py::test_warm_coinb
 or, for a test function and specific parameter combination:
 
 ```console
-fill tests/shanghai/eip3651_warm_coinbase/test_warm_coinbase.py::test_warm_coinbase_gas_usage[fork=Merge-DELEGATECALL]
+fill tests/shanghai/eip3651_warm_coinbase/test_warm_coinbase.py::test_warm_coinbase_gas_usage[fork_Merge-DELEGATECALL]
 ```
 
 ## Execution for Development Forks

src/pytest_plugins/forks/tests/test_forks.py

@@ -34,7 +34,7 @@ def test_all_forks(state_test):
     all_forks = get_deployed_forks()
     forks_under_test = forks_from_until(all_forks[0], all_forks[-1])
     for fork in forks_under_test:
-        assert f":test_all_forks[fork={fork}]" in "\n".join(result.stdout.lines)
+        assert f":test_all_forks[fork_{fork}]" in "\n".join(result.stdout.lines)
     result.assert_outcomes(
         passed=len(forks_under_test),
         failed=0,
@@ -64,7 +64,7 @@ def test_all_forks(state_test):
     all_forks = get_deployed_forks()
     forks_under_test = forks_from_until(fork_map[fork], all_forks[-1])
     for fork_under_test in forks_under_test:
-        assert f":test_all_forks[fork={fork_under_test}]" in "\n".join(result.stdout.lines)
+        assert f":test_all_forks[fork_{fork_under_test}]" in "\n".join(result.stdout.lines)
     result.assert_outcomes(
         passed=len(forks_under_test),
         failed=0,
@@ -94,7 +94,7 @@ def test_all_forks(state_test):
     if ArrowGlacier in forks_under_test:
         forks_under_test.remove(ArrowGlacier)
     for fork_under_test in forks_under_test:
-        assert f":test_all_forks[fork={fork_under_test}]" in "\n".join(result.stdout.lines)
+        assert f":test_all_forks[fork_{fork_under_test}]" in "\n".join(result.stdout.lines)
     result.assert_outcomes(
         passed=len(forks_under_test),
         failed=0,

src/pytest_plugins/test_filler/test_filler.py

@@ -300,10 +300,10 @@ def convert_test_id_to_test_name_and_parameters(name: str) -> Tuple[str, str]:
     Converts a test name to a tuple containing the test name and test parameters.
 
     Example:
-    test_push0_key_sstore[fork=Shanghai] -> test_push0_key_sstore, fork_Shanghai
+    test_push0_key_sstore[fork_Shanghai] -> test_push0_key_sstore, fork_Shanghai
     """
     test_name, parameters = name.split("[")
-    return test_name, re.sub(r"[\[=\-]", "_", parameters).replace("]", "")
+    return test_name, re.sub(r"[\[\-]", "_", parameters).replace("]", "")
 
 
 def get_module_relative_output_dir(test_module: Path, filler_path: Path) -> Path:
@@ -380,7 +380,7 @@ class FixtureCollector:
     Collects all fixtures generated by the test cases.
     """
 
-    all_fixtures: Dict[Path, List[Tuple[str, Any]]]
+    all_fixtures: Dict[Path, Dict[str, Any]]
     output_dir: str
     flat_output: bool
     json_path_to_fixture_type: Dict[Path, FixtureFormats]
@@ -436,34 +436,22 @@ def get_fixture_basename_for_nested_output(self, item):
 
         fixture_path = self.output_dir / fixture_basename.with_suffix(".json")
         if fixture_path not in self.all_fixtures:  # relevant when we group by test function
-            self.all_fixtures[fixture_path] = []
+            self.all_fixtures[fixture_path] = {}
             self.json_path_to_fixture_type[fixture_path] = fixture_format
             self.json_path_to_test_item[fixture_path] = item
 
-        m = re.match(r".*?\[(.*)\]", item.name)
-        if not m:
-            raise Exception("Could not parse test name: " + item.name)
-        name = m.group(1)
-        if fixture.name:
-            name += "-" + fixture.name
-        jsonFixture = fixture.to_json()
-        self.all_fixtures[fixture_path].append((name, jsonFixture))
+        self.all_fixtures[fixture_path][item.nodeid] = fixture.to_json()
 
     def dump_fixtures(self) -> None:
         """
         Dumps all collected fixtures to their respective files.
         """
         os.makedirs(self.output_dir, exist_ok=True)
         for fixture_path, fixtures in self.all_fixtures.items():
-            output_json = {}
-            for index, fixture_props in enumerate(fixtures):
-                name, fixture = fixture_props
-                name = str(index).zfill(3) + "-" + name
-                output_json[name] = fixture
             if not self.flat_output:
                 os.makedirs(fixture_path.parent, exist_ok=True)
             with open(fixture_path, "w") as f:
-                json.dump(output_json, f, indent=4)
+                json.dump(fixtures, f, indent=4)
 
     def verify_fixture_files(self, evm_fixture_verification: TransitionTool) -> None:
         """
@@ -699,7 +687,7 @@ def pytest_make_parametrize_id(config, val, argname):
     Pytest hook called when generating test ids. We use this to generate
     more readable test ids for the generated tests.
     """
-    return f"{argname}={val}"
+    return f"{argname}_{val}"
 
 
 def pytest_runtest_call(item):

src/pytest_plugins/test_filler/tests/test_test_filler.py

@@ -217,18 +217,18 @@ def test_fixture_output_based_on_command_line_args(
     - each fixture file contains the expected number of fixtures.
 
     The modules above generate the following test cases:
-        tests/merge/test_module_merge.py::test_merge_one[fork=Merge] PASSED
-        tests/merge/test_module_merge.py::test_merge_one[fork=Shanghai] PASSED
-        tests/merge/test_module_merge.py::test_merge_two[fork=Merge] PASSED
-        tests/merge/test_module_merge.py::test_merge_two[fork=Shanghai] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_one[fork=Merge] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_one[fork=Shanghai] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Merge-x=1] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Merge-x=2] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Merge-x=3] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Shanghai-x=1] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Shanghai-x=2] PASSED
-        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork=Shanghai-x=3] PASSED
+        tests/merge/test_module_merge.py::test_merge_one[fork_Merge] PASSED
+        tests/merge/test_module_merge.py::test_merge_one[fork_Shanghai] PASSED
+        tests/merge/test_module_merge.py::test_merge_two[fork_Merge] PASSED
+        tests/merge/test_module_merge.py::test_merge_two[fork_Shanghai] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_one[fork_Merge] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_one[fork_Shanghai] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Merge-x=1] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Merge-x=2] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Merge-x=3] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Shanghai-x=1] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Shanghai-x=2] PASSED
+        tests/shanghai/test_module_shanghai.py::test_shanghai_two[fork_Shanghai-x=3] PASSED
     """
     tests_dir = testdir.mkdir("tests")