Skip to content

Conversation

@Anuoluwapo25
Copy link
Contributor

@Anuoluwapo25 Anuoluwapo25 commented Oct 28, 2025

🗒️ Description

fix(new_fork): remove docstring from new fork init.py via codemod

Why codemod was used

The new_fork tool copies an existing fork (e.g., prague).
The generated __init__.py inherits the full docstring from the source — which is misleading for a new fork.

We use a LibCST codemod (RemoveDocstringCommand) to:

  • Automatically remove the docstring
  • Run as part of ForkBuilder
  • Enforce clean templates
  • Trigger ruff D100 to prompt real docstrings

String rename: string.pystring_replace.py

Renamed string.py to string_replace.py to avoid shadowing the standard library string module.

  • Importing string would previously import the local file instead of string from stdlib
  • This caused circular imports and runtime errors
  • string_replace.py is more descriptive and safe

🔗 Related Issues or PRs

Fixes #1423

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx tox -e static
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
  • Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
  • Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

Cute kitten playing

Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also like to see a check in test_tools_new_fork.py that verifies that the docstring is indeed removed from the correct file, and that it isn't removed from one other file (pick one).

RenameFork(),
SetForkCriteria(),
ReplaceForkName(),
ReplaceForkName(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ClearDocstring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should be ClearDocstring, I'm sorry for the mixup

@SamWilsn
Copy link
Contributor

Please run tox -e static and fix any errors it reports.

@Anuoluwapo25
Copy link
Contributor Author

I'd also like to see a check in test_tools_new_fork.py that verifies that the docstring is indeed removed from the correct file, and that it isn't removed from one other file (pick one).

Alright, I will work on it.

@Anuoluwapo25
Copy link
Contributor Author

Please run tox -e static and fix any errors it reports.

Alright

felix314159 pushed a commit to felix314159/execution-specs that referenced this pull request Oct 29, 2025
…gineReorgFixture` (ethereum#1706)

* feat(fill): enable interface for two-phase shared alloc filling

* feat(fixtures,types): add shared pre-allocation data models

- Add `SharedPreStateGroup` and `SharedPreState` models.
- Support grouping tests by (fork, environment) hash.
- Enable serialization for phase 1/2 coordination.

* feat(fixtures): add `BlockchainEngineReorgFixture` and common base class

- Add `BlockchainEngineFixtureCommon` base class for shared functionality.
- Add `BlockchainEngineReorgFixture` with `post_state_diff` field.
- Remove unnecessary backwards compatibility validator from common class.
- Update `test_collect_only` framework test for new fixture.

* feat(fixtures): export new fixture classes and shared models

- Export `BlockchainEngineReorgFixture` and `BlockchainEngineFixtureCommon`.
- Export `SharedPreState` and `SharedPreStateGroup` models.
- Enable imports for shared pre-allocation functionality.

* feat(specs): add shared pre-allocation support to `BaseTest`

- Add `update_shared_pre_state()` and `compute_shared_pre_alloc_hash()`.
- Add `get_genesis_environment()` for polymorphic environment access.
- Support both `StateTest` and `BlockchainTest` formats.

* feat(fill): implement shared pre-allocation (`fill` pytest hooks)

- Add `pytest_sessionstart` and `pytest_sessionfinish` for phase coordination.
- Add `calculate_post_state_diff()` for memory-efficient storage.
- Add custom terminal summary with group statistics.
- Load/save shared pre-allocation state.

* feat(fixtures): enable global address allocation and shared pre-alloc path

- Add `shared_prealloc_path` property to `FixtureOutput`.
- Extend global address iterators to all test formats.
- Make the fixture scope of address iterators dynamic: session-scoped for shared pre-alloc generation, function-scoped for regular pre-alloc.
- Update and improve `pre_alloc` tests.

* feat(specs): add `BlockchainEngineReorgFixture` generation support

- Support generating reorg fixtures from blockchain tests.
- Enable shared pre-allocation integration with blockchain specs.

* feat(fixtures): add `pre_hash` field to test case index

- Support pre-allocation hash tracking in index files.
- Enable fixture consumption with shared pre-allocation metadata.

* chore(fill): only generate engine_reorg fixtures w/shared-alloc flags

* feat(fill): add a new pytest mark for custom prealloc group control

* chore(tests): apply `prealloc_group` to system contract & requests tests

* fix(tests): prevent iterator state persistence in beacon root tests during two-phase execution

The beacon root tests were failing when run with two-phase shared pre-allocation because they used `itertools.count()` objects directly in pytest parametrization. These iterator objects maintain internal state that persists across test phases when run in the same Python process.

Changes:
- Replace direct `count()` objects with factory functions that return fresh iterators.
- Update `beacon_roots` fixture to create new iterator instances on each invocation.
- Modify test parametrization to use `timestamps_factory` instead of `timestamps`.

This bug only manifested in two-phase execution because:
1. In regular single-phase filling, each test gets its own fresh Python process.
2. In two-phase execution within one process, the same parametrized iterator.
   objects are reused, causing timestamps to continue from where phase 1 left off.
3. The second phase would see timestamps like 21000 instead of starting at 1000.

The fix ensures each test invocation gets fresh iterators with reset state, preventing cross-phase contamination while maintaining the same test behavior.

* chore(cli): add `groupstats` script to analyze shared pre-allocation groups

Add a new CLI tool that provides comprehensive analysis of shared pre-allocation groups generated by the test framework. The tool helps identify optimization opportunities for client teams by analyzing group distributions, test coverage, and identifying problematic test functions that create multiple size-1 groups.

Key features:
- Display overall statistics (groups, tests, accounts)
- Show distribution by fork with average tests per group
- Analyze group size frequency distribution
- Calculate test coverage impact by group size
- List test modules by execution complexity (group count)
- Identify split test functions with Groups/Fork ratio analysis
- Progressive disclosure with -v and -vv verbosity levels

The Groups/Fork ratio metric distinguishes between necessary multi-fork coverage and problematic parameter-heavy tests, enabling targeted optimization strategies for CI workflows.

* docs(consume): add `BlockchainEngineReorgFixture` docs

* refactor(all): standardize shared pre-allocation naming conventions

- CLI flags: `--generate-shared-alloc` → `--generate-shared-pre`
- CLI flags: `--use-shared-alloc` → `--use-shared-pre`
- Property: `shared_prealloc_path` → `shared_pre_alloc_path`
- Pytest marker: `prealloc_group` → `pre_alloc_group`
- Entry point: `show_pre_alloc_groups` → `groupstats`

* docs: update changelog

* fix(fixtures/filler): Pydantic fixes

* feat(filler): pre_alloc.json is now a folder

* fix(cli): groupstats

* fix: tox

* feat: Use deterministic addresses in pre-alloc

* fix: xdist

* feat: genesis header in pre_alloc files

* fix: tox

* fix: move pre

---------

Co-authored-by: Mario Vega <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New fork tool should clear the docstring in src/ethereum/forks/*/__init__.py

2 participants