From 4a5115298c4764a61a604d4fa49ac9a966c0b631 Mon Sep 17 00:00:00 2001 From: Richard Gregory Date: Sun, 8 Jun 2025 11:51:42 +0100 Subject: [PATCH] refactor(pytest): integrate EIP report generation into spec_version_checker plugin Move EIP version report generation from external script into the spec_version_checker pytest plugin to eliminate brittle regex parsing of console output and improve maintainability. - Add EIPVersionFailure dataclass to capture failure data directly - Implement generate_eip_report_markdown() function within plugin - Use pytest_sessionfinish hook to generate reports automatically --- .github/scripts/generate_eip_report.py | 147 ------------------ .github/workflows/check_eip_versions.yaml | 5 - docs/CHANGELOG.md | 4 + .../spec_version_checker.py | 134 +++++++++++++++- 4 files changed, 136 insertions(+), 154 deletions(-) delete mode 100644 .github/scripts/generate_eip_report.py diff --git a/.github/scripts/generate_eip_report.py b/.github/scripts/generate_eip_report.py deleted file mode 100644 index 66ff8534c53..00000000000 --- a/.github/scripts/generate_eip_report.py +++ /dev/null @@ -1,147 +0,0 @@ -"""Generate a markdown report of outdated EIP references from the EIP version checker output.""" - -import os -import re -import sys -import textwrap -from string import Template - -# Report template using textwrap.dedent for clean multiline strings -REPORT_TEMPLATE = Template( - textwrap.dedent("""\ - # EIP Version Check Report - - This automated check has detected that some EIP references in test files are outdated. This means that the EIPs have been updated in the [ethereum/EIPs](https://github.com/ethereum/EIPs) repository since our tests were last updated. - - ## Outdated EIP References - - ### Summary Table - - | File | EIP Link | Referenced Version | Latest Version | - | ---- | -------- | ------------------ | -------------- | - $summary_table - - ### Verbatim Failures - - ``` - $fail_messages - ``` - - ### Verbatim Errors - - ``` - $error_messages - ``` - - ## Action Required - - 1. Please verify whether the affected tests need updating based on changes in the EIP spec. - 2. Update the `REFERENCE_SPEC_VERSION` in each file with the latest version shown above. - 3. For detailed instructions, see the [reference specification documentation](https://eest.ethereum.org/main/writing_tests/reference_specification/). - - ## Workflow Information - - For more details, see the [workflow run](https://github.com/ethereum/execution-spec-tests/actions/runs/$run_id). -""") # noqa: E501 -) - - -def extract_failures(output): - """Extract failure information from the output using regex.""" - failures = [] - - for line in output.split("\n"): - if not line.startswith("FAILED"): - continue - - # Extract test file path - file_match = re.search(r"FAILED (tests/[^:]+\.py)", line) - if not file_match: - continue - file_path = file_match.group(1) - - # Extract EIP number - eip_match = re.search(r"eip(\d+)", file_path, re.IGNORECASE) - eip_num = f"EIP-{eip_match.group(1)}" if eip_match else "Unknown" - - # Extract full path - full_path_match = re.search(r"from '([^']+)'", line) - full_path = full_path_match.group(1) if full_path_match else "Unknown" - - # Extract EIP link - eip_link_match = re.search(r"Spec: (https://[^ ]+)\.", line) - eip_link = eip_link_match.group(1) if eip_link_match else "" - eip_link = eip_link.replace("blob/", "commits/") if eip_link else "" - - # Extract versions - ref_version_match = re.search(r"Referenced version: ([a-f0-9]+)", line) - ref_version = ref_version_match.group(1) if ref_version_match else "Unknown" - - latest_version_match = re.search(r"Latest version: ([a-f0-9]+)", line) - latest_version = latest_version_match.group(1) if latest_version_match else "Unknown" - - failures.append((file_path, eip_num, full_path, eip_link, ref_version, latest_version)) - - return failures - - -def generate_summary_table(failures): - """Generate a markdown summary table from the failures.""" - rows = [] - for file_path, eip_num, _, eip_link, ref_version, latest_version in failures: - rows.append( - f"| `{file_path}` | [{eip_num}]({eip_link}) | `{ref_version}` | `{latest_version}` |" - ) - return "\n".join(rows) - - -def main(): - """Generate the report.""" - if len(sys.argv) < 2: - print("Usage: uv run python generate_eip_report.py [output_file]") - sys.exit(1) - - input_file = sys.argv[1] - output_file = sys.argv[2] if len(sys.argv) > 2 else "./reports/outdated_eips.md" - - try: - with open(input_file, "r") as f: - output = f.read() - except Exception as e: - print(f"Error reading input file: {e}") - sys.exit(1) - - failures = extract_failures(output) - - fail_messages = "\n".join(line for line in output.split("\n") if line.startswith("FAILED")) - if not fail_messages: - fail_messages = ( - "No test failures were found in the pytest output: No lines start with 'FAILED'." - ) - - error_messages = "\n".join(line for line in output.split("\n") if line.startswith("ERROR")) - if not error_messages: - error_messages = ( - "No test errors were found in the pytest output: No lines start with 'ERROR'." - ) - - report_content = REPORT_TEMPLATE.substitute( - summary_table=generate_summary_table(failures), - fail_messages=fail_messages, - error_messages=error_messages, - run_id=os.environ.get("GITHUB_RUN_ID", ""), - ) - - try: - with open(output_file, "w") as report: - report.write(report_content) - except Exception as e: - print(f"Error writing output file: {e}") - sys.exit(1) - - print(f"Report generated successfully: {output_file}") - print(f"Found {len(failures)} outdated EIP references") - - -if __name__ == "__main__": - main() diff --git a/.github/workflows/check_eip_versions.yaml b/.github/workflows/check_eip_versions.yaml index 2f3a0758e9d..f7a52b3b299 100644 --- a/.github/workflows/check_eip_versions.yaml +++ b/.github/workflows/check_eip_versions.yaml @@ -39,11 +39,6 @@ jobs: # Always return success to GitHub Actions exit 0 - - name: Generate report file - if: steps.check-eip.outputs.exit_code != 0 - run: | - uv run python .github/scripts/generate_eip_report.py ./reports/eip_check_output.txt ./reports/outdated_eips.md - - name: Create Issue From File if: steps.check-eip.outputs.exit_code != 0 uses: peter-evans/create-issue-from-file@e8ef132d6df98ed982188e460ebb3b5d4ef3a9cd diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 2ac03ed9f22..f6f3cb733a4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -38,6 +38,10 @@ Users can select any of the artifacts depending on their testing needs for their - 🐞 zkEVM marked tests have been removed from `tests-deployed` tox environment into its own separate workflow `tests-deployed-zkevm` and are filled by `evmone-t8n` ([#1617](https://github.com/ethereum/execution-spec-tests/pull/1617)). - ✨ Field `postStateHash` is now added to all `blockchain_test` and `blockchain_test_engine` tests that use `exclude_full_post_state_in_output` in place of `postState`. Fixes `evmone-blockchaintest` test consumption and indirectly fixes coverage runs for these tests ([#1667](https://github.com/ethereum/execution-spec-tests/pull/1667)). +#### `check_eip_versions` + +- 🔀 Refactor: Move EIP version report generation from external script into the `spec_version_checker` pytest plugin to eliminate brittle regex parsing of console output and improve maintainability. + #### `consume` #### `execute` diff --git a/src/pytest_plugins/spec_version_checker/spec_version_checker.py b/src/pytest_plugins/spec_version_checker/spec_version_checker.py index c5344e0882b..1393b09f6b8 100644 --- a/src/pytest_plugins/spec_version_checker/spec_version_checker.py +++ b/src/pytest_plugins/spec_version_checker/spec_version_checker.py @@ -6,6 +6,8 @@ import os import re import textwrap +from dataclasses import dataclass +from pathlib import Path from types import ModuleType from typing import Any, List, Optional, Set @@ -22,6 +24,17 @@ ) +@dataclass +class EIPVersionFailure: + """Represents a failure in EIP version check.""" + + file_path: str + eip_spec_name: str + eip_spec_url: str + referenced_version: str + latest_version: str + + def pytest_addoption(parser): """Add Github token option to pytest command line options.""" group = parser.getgroup( @@ -52,6 +65,8 @@ def pytest_configure(config): "eip_version_check: a test that tests the reference spec defined in an EIP test module.", ) + config._eip_version_failures = [] + github_token = config.getoption("github_token") or os.environ.get("GITHUB_TOKEN") if not github_token: @@ -113,7 +128,11 @@ def is_test_for_an_eip(input_string: str) -> bool: return False -def test_eip_spec_version(module: ModuleType, github_token: Optional[str] = None): +def test_eip_spec_version( + module: ModuleType, + github_token: Optional[str] = None, + config: Optional[pytest.Config] = None, +): """ Test that the ReferenceSpec object as defined in the test module is not outdated when compared to the remote hash from @@ -122,8 +141,18 @@ def test_eip_spec_version(module: ModuleType, github_token: Optional[str] = None Args: module: Module to test github_token: Optional GitHub token for API authentication + config: Pytest config object, which must be provided. """ + # Ensure config is available + if config is None: + # This case should ideally not happen if called via EIPSpecTestItem.runtest + # or if pytest passes config correctly in other scenarios. + # Fallback or error if essential. For now, assume it's provided. + pytest.fail( + "pytest.Config not available in test_eip_spec_version. " + "This is an issue with the plugin itself." + ) ref_spec = get_ref_spec_from_module(module, github_token=github_token) assert ref_spec, "No reference spec object defined" @@ -143,6 +172,24 @@ def test_eip_spec_version(module: ModuleType, github_token: Optional[str] = None f"Reference spec URL: {ref_spec.api_url()}." ) from e + if not is_up_to_date: + failure_data = EIPVersionFailure( + file_path=str(module.__file__), + eip_spec_name=ref_spec.name(), + eip_spec_url=ref_spec.api_url(), + referenced_version=ref_spec.known_version(), + latest_version=ref_spec.latest_version(), + ) + # Ensure _eip_version_failures list exists, though it should by pytest_configure + if hasattr(config, "_eip_version_failures"): + config._eip_version_failures.append(failure_data) + else: + # This case should not happen if pytest_configure runs as expected. + # Log an error or handle as appropriate. + print( + "Warning: config._eip_version_failures not found. Failure data cannot be recorded." + ) + assert is_up_to_date, message @@ -189,7 +236,9 @@ def from_parent(cls, parent: Node, **kw: Any) -> "EIPSpecTestItem": def runtest(self) -> None: """Define the test to execute for this item.""" - test_eip_spec_version(self.module, github_token=self.github_token) + test_eip_spec_version( + self.module, github_token=self.github_token, config=self.session.config + ) def reportinfo(self) -> tuple[str, int, str]: """ @@ -217,3 +266,84 @@ def pytest_collection_modifyitems( for item in new_test_eip_spec_version_items: item.add_marker("eip_version_check", append=True) items.extend(new_test_eip_spec_version_items) + + +def generate_eip_report_markdown(failures: List[EIPVersionFailure], run_id: str) -> str: + """Generate a markdown report for EIP version check failures.""" + summary_table_header = ( + "| File Path | EIP | Referenced Version | Latest Version |\n" + "|-----------|-----|--------------------|----------------|\n" + ) + summary_table_rows = [] + for failure in failures: + eip_match = re.search(r"eip-?(\d+)", failure.eip_spec_name, re.IGNORECASE) + if not eip_match: + eip_match = re.search(r"eip-?(\d+)", failure.file_path, re.IGNORECASE) + eip_number_str = f"EIP-{eip_match.group(1)}" if eip_match else "Unknown" + + # Ensure the URL points to the commit history for better context + eip_link_url = failure.eip_spec_url.replace("blob/", "commits/") + + row = ( + f"| {failure.file_path} | [{eip_number_str}]({eip_link_url}) | " + f"`{failure.referenced_version}` | `{failure.latest_version}` |" + ) + summary_table_rows.append(row) + + fail_details_parts = [] + for failure in failures: + detail = ( + f"File: `{failure.file_path}`\n" + f"Spec Name: `{failure.eip_spec_name}`\n" + f"Spec URL: {failure.eip_spec_url}\n" + f"Referenced Version: `{failure.referenced_version}`\n" + f"Latest Version: `{failure.latest_version}`\n" + "---\n" + ) + fail_details_parts.append(detail) + + report_template = textwrap.dedent( + """\ + # EIP Version Check Report (Run ID: {run_id}) + + ## Summary of Outdated EIP References + + {summary_table_header}{summary_table_rows_str} + + ## Outdated EIP References Details + + {fail_details_str} + """ + ) + + return report_template.format( + run_id=run_id, + summary_table_header=summary_table_header, + summary_table_rows_str="\n".join(summary_table_rows), + fail_details_str="\n".join(fail_details_parts), + ) + + +def pytest_sessionfinish(session): + """ + Call after the whole test session finishes. + Generate a report if there are any EIP version failures. + """ + failures = session.config._eip_version_failures + if failures: + run_id = os.environ.get("GITHUB_RUN_ID", "local-run") + report_content = generate_eip_report_markdown(failures, run_id) + report_file_path_str = "./reports/outdated_eips.md" + + report_file_path = Path(report_file_path_str) + report_file_path.parent.mkdir(parents=True, exist_ok=True) + + with open(report_file_path, "w") as f: + f.write(report_content) + + print( + f"\nEIP Version Check: {len(failures)} outdated EIP references found. " + f"Report generated at {report_file_path_str}" + ) + else: + print("\nEIP Version Check: No outdated EIP references found.")