chore(fill): disable writing evm debug information to ./logs/ by default (#1874)

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

Author: richardgreg

.github/actions/build-fixtures/action.yaml

@@ -34,7 +34,7 @@ runs:
     - name: Generate fixtures using fill
       shell: bash
       run: |
-        uv run fill -n ${{ steps.evm-builder.outputs.x-dist }} --evm-bin=${{ steps.evm-builder.outputs.evm-bin }} --solc-version=${{ steps.properties.outputs.solc }} --skip-evm-dump ${{ steps.properties.outputs.fill-params }} --output=fixtures_${{ inputs.release_name }}.tar.gz --build-name ${{ inputs.release_name }}
+        uv run fill -n ${{ steps.evm-builder.outputs.x-dist }} --evm-bin=${{ steps.evm-builder.outputs.evm-bin }} --solc-version=${{ steps.properties.outputs.solc }} ${{ steps.properties.outputs.fill-params }} --output=fixtures_${{ inputs.release_name }}.tar.gz --build-name ${{ inputs.release_name }}
     - name: Wrap ethereum/tests fixtures with eofwrap tool
       shell: bash
       if: ${{ steps.properties.outputs.eofwrap }}

.github/scripts/fill_introduced_tests.sh

@@ -16,7 +16,7 @@ FILL_UNTIL="${4:-Cancun}"
 # As when we translate from yul/solidity some dup/push opcodes could become untouched
 files="$CHANGED_TEST_FILES tests/homestead/coverage/test_coverage.py"
 
-uv run fill $files --clean --until=$FILL_UNTIL --evm-bin evmone-t8n --skip-evm-dump --block-gas-limit $BLOCK_GAS_LIMIT -m "state_test or blockchain_test" --output $PATCH_TEST_PATH > >(tee -a filloutput.log) 2> >(tee -a filloutput.log >&2)
+uv run fill $files --clean --until=$FILL_UNTIL --evm-bin evmone-t8n --block-gas-limit $BLOCK_GAS_LIMIT -m "state_test or blockchain_test" --output $PATCH_TEST_PATH > >(tee -a filloutput.log) 2> >(tee -a filloutput.log >&2)
 
 if grep -q "FAILURES" filloutput.log; then
     echo "Error: failed to generate .py tests."

.github/scripts/fill_prepatched_tests.sh

@@ -27,7 +27,7 @@ echo "$MODIFIED_DELETED_FILES"
 rm -rf fixtures
 rm -f filloutput.log
 
-uv run fill $MODIFIED_DELETED_FILES --clean --until=$FILL_UNTIL --evm-bin evmone-t8n --skip-evm-dump --block-gas-limit $BLOCK_GAS_LIMIT -m "state_test or blockchain_test" --output $BASE_TEST_PATH > >(tee -a filloutput.log) 2> >(tee -a filloutput.log >&2)
+uv run fill $MODIFIED_DELETED_FILES --clean --until=$FILL_UNTIL --evm-bin evmone-t8n --block-gas-limit $BLOCK_GAS_LIMIT -m "state_test or blockchain_test" --output $BASE_TEST_PATH > >(tee -a filloutput.log) 2> >(tee -a filloutput.log >&2)
 
 if grep -q "FAILURES" filloutput.log; then
     echo "Error: failed to generate .py tests from before the PR."

docs/CHANGELOG.md

@@ -22,6 +22,8 @@ The output behavior of `fill` has changed ([#1608](https://github.com/ethereum/e
 - Before: `fill` wrote fixtures into the directory specified by the `--output` flag (default: `fixtures`). This could have many unintended consequences, including unexpected errors if old or invalid fixtures existed in the directory (for details see [#1030](https://github.com/ethereum/execution-spec-tests/issues/1030)).
 - Now: `fill` will exit without filling any tests if the specified directory exists and is not-empty. This may be overridden by adding the `--clean` flag, which will first remove the specified directory.
 
+Additionally, writing debugging information to the EVM "dump directory" by default has been disabled. To obtain debug output, the `--evm-dump-dir` flag must now be explicitly set. As a consequence, the now redundant `--skip-evm-dump` option was removed ([#1874](https://github.com/ethereum/execution-spec-tests/pull/1874)). This undoes functionality originally introduced in [#999](https://github.com/ethereum/execution-spec-tests/pull/999) and [#1150](https://github.com/ethereum/execution-spec-tests/pull/1150).
+
 #### Feature `zkevm` updated to `benchmark`
 
 Due to the crossover between `zkevm` and `benchmark` tests, all instances of the former have been replaced with the latter nomenclature. Repository PR labels and titles are additionally updated to reflect this change.
@@ -49,6 +51,7 @@ 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)).
 - 🔀 Changed INVALID_DEPOSIT_EVENT_LAYOUT to a BlockException instead of a TransactionException ([#1773](https://github.com/ethereum/execution-spec-tests/pull/1773)).
+- 🔀 Disabled writing debugging information to the EVM "dump directory" to improve performance. To obtain debug output, the `--evm-dump-dir` flag must now be explicitly set. As a consequence, the now redundant `--skip-evm-dump` option was removed ([#1874](https://github.com/ethereum/execution-spec-tests/pull/1874)).
 
 #### `consume`
 

docs/filling_tests/debugging_t8n_tools.md

@@ -2,13 +2,13 @@
 
 There are two flags that can help debugging `t8n` tools or the execution-spec-tests framework:
 
-1. `--evm-dump-dir` (Default: <repo>/logs/evm): Write debug information from `t8n` tool calls to the specified directory.
+1. `--evm-dump-dir`: Write debug information from `t8n` tool calls to the specified directory.
 2. `--traces`: Collect traces of the execution from the transition tool.
 3. `--verify-fixtures`: Run go-ethereum's `evm blocktest` command to verify the generated test fixtures.
 
 ## EVM Dump Directory
 
-The `--evm-dump-dir` flag tells the framework to write the inputs and outputs of every call made to the `t8n` command to the specified output directory. The aim is to help debugging or simply understand how a test is interacting with the EVM. The default location is `logs/evm` in the project root.
+The `--evm-dump-dir` flag tells the framework to write the inputs and outputs of every call made to the `t8n` command to the specified output directory. The aim is to help debugging or simply understand how a test is interacting with the EVM. Debug output is only generated when this flag is explicitly specified.
 
 Each test case receives its own sub-directory under the `--evm-dump-dir` that contains these files which can be easily accessed from the HTML test report generated by `fill` (located by default in the root of the `--output` directory).
 

src/config/app.py

@@ -25,8 +25,5 @@ def version(self) -> str:
     DEFAULT_LOGS_DIR: Path = Path(__file__).resolve().parent.parent.parent / "logs"
     """The default directory where log files are stored."""
 
-    DEFAULT_EVM_LOGS_DIR: Path = DEFAULT_LOGS_DIR / "evm"
-    """The default directory where EVM log files are stored."""
-
     ROOT_DIR: Path = Path(__file__).resolve().parents[2]
     """The root directory of the project."""

src/pytest_plugins/consume/tests/test_consume_args.py

@@ -61,7 +61,6 @@ def fill_tests(
         [
             "-c",
             "pytest.ini",
-            "--skip-evm-dump",
             "-m",
             "not blockchain_test_engine",
             f"--from={fill_fork_from}",

src/pytest_plugins/filler/filler.py

@@ -20,7 +20,6 @@
 from pytest_metadata.plugin import metadata_key  # type: ignore
 
 from cli.gen_index import generate_fixtures_index
-from config import AppConfig
 from ethereum_clis import TransitionTool
 from ethereum_clis.clis.geth import FixtureConsumerTool
 from ethereum_test_base_types import Account, Address, Alloc, ReferenceSpec
@@ -273,20 +272,12 @@ def pytest_addoption(parser: pytest.Parser):
         "--t8n-dump-dir",
         action="store",
         dest="base_dump_dir",
-        default=AppConfig().DEFAULT_EVM_LOGS_DIR,
+        default=None,
         help=(
             "Path to dump the transition tool debug output. "
-            f"(Default: {AppConfig().DEFAULT_EVM_LOGS_DIR})"
+            "Only creates debug output when explicitly specified."
         ),
     )
-    debug_group.addoption(
-        "--skip-evm-dump",
-        "--skip-t8n-dump",
-        action="store_true",
-        dest="skip_dump_dir",
-        default=False,
-        help=("Skip dumping the the transition tool debug output."),
-    )
 
 
 def pytest_sessionstart(session: pytest.Session):
@@ -663,8 +654,6 @@ def evm_fixture_verification(
 @pytest.fixture(scope="session")
 def base_dump_dir(request: pytest.FixtureRequest) -> Path | None:
     """Path to base directory to dump the evm debug output."""
-    if request.config.getoption("skip_dump_dir"):
-        return None
     base_dump_dir_str = request.config.getoption("base_dump_dir")
     if base_dump_dir_str:
         return Path(base_dump_dir_str)

src/pytest_plugins/filler/tests/test_output_directory.py

@@ -38,7 +38,6 @@ def _run_fill(output_dir: Path, clean: bool = False, expect_failure: bool = Fals
         args = [
             "-c",
             "pytest.ini",
-            "--skip-evm-dump",
             "-m",
             "(not blockchain_test_engine) and (not eip_version_check)",
             f"--from={fill_fork_from}",

tox.ini

@@ -83,18 +83,18 @@ description = Fill test cases in ./tests/ for deployed mainnet forks, except for
 setenv =
     # Use custom EELS_RESOLUTIONS_FILE if it is set via the environment (eg, in CI)
     EELS_RESOLUTIONS_FILE = {env:EELS_RESOLUTIONS_FILE:}
-commands = pytest -n auto -m "not slow and not benchmark" --skip-evm-dump --output=/tmp/fixtures-tox --clean
+commands = pytest -n auto -m "not slow and not benchmark" --output=/tmp/fixtures-tox --clean
 
 [testenv:tests-deployed-benchmark]
 description = Fill benchmarking test cases in ./tests/ for deployed mainnet forks, using evmone-t8n.
-commands = pytest -n auto -m "benchmark" --skip-evm-dump --block-gas-limit 36000000 --output=/tmp/fixtures-tox --clean --evm-bin=evmone-t8n
+commands = pytest -n auto -m "benchmark" --block-gas-limit 36000000 --output=/tmp/fixtures-tox --clean --evm-bin=evmone-t8n
 
 [testenv:tests-develop]
 description = Fill test cases in ./tests/ for deployed and development mainnet forks
 setenv =
     # Use custom EELS_RESOLUTIONS_FILE if it is set via the environment (eg, in CI)
     EELS_RESOLUTIONS_FILE = {env:EELS_RESOLUTIONS_FILE:}
-commands = pytest -n auto --until={[forks]develop} -k "not slow and not benchmark" --skip-evm-dump --output=/tmp/fixtures-tox --clean
+commands = pytest -n auto --until={[forks]develop} -k "not slow and not benchmark" --output=/tmp/fixtures-tox --clean
 
 # ----------------------------------------------------------------------------------------------
 # ALIAS ENVIRONMENTS
@@ -158,4 +158,4 @@ setenv =
 commands =
     {[testenv:spellcheck]commands}
     {[testenv:markdownlint]commands}
-    {[testenv:mkdocs]commands}
+    {[testenv:mkdocs]commands}