feat(test-fill): enable phase-1-only pre-alloc generation#2720
Merged
marioevz merged 5 commits intoethereum:forks/amsterdamfrom Apr 22, 2026
Merged
feat(test-fill): enable phase-1-only pre-alloc generation#2720marioevz merged 5 commits intoethereum:forks/amsterdamfrom
marioevz merged 5 commits intoethereum:forks/amsterdamfrom
Conversation
Re-order `FillCommand.create_executions` so that the execution plan reflects the user's intent: - `--use-pre-alloc-groups` now takes priority. The flag means pre-alloc groups already exist on disk from a previous run, so even alongside `--generate-all-formats` the run is single-phase. - `--generate-pre-alloc-groups` without `--generate-all-formats` now runs phase 1 only; don't continue to phase 2. - `--generate-all-formats` continues to trigger the full two-phase run. Update `test_legacy_generate_pre_alloc_groups_still_works` to reflect the new phase-1-only behaviour and add a test covering the `--use-pre-alloc-groups` priority.
Add failing tests asserting that `FillCommand.create_executions` raises `click.UsageError` when `--use-pre-alloc-groups` is combined with: - `--generate-pre-alloc-groups`: the first asserts the groups already exist on disk, the second regenerates them: contradictory intents. - `--clean`: `--clean` wipes the output directory that holds the pre-alloc groups the user is asserting exist. Without a guard, `FillCommand.create_executions` silently returns a single-phase plan and `FillingSession._load_pre_alloc_groups_from_folder` then fails on a missing folder. Validation logic follows in the next commit.
Add `_validate_flag_combinations` to `FillCommand.create_executions` so contradictory flag pairs fail fast with `click.UsageError` instead of silently falling through to a broken single-phase execution. Rejected combinations: - `--use-pre-alloc-groups` + `--generate-pre-alloc-groups`: the first asserts the groups already exist, the second regenerates them. - `--use-pre-alloc-groups` + `--clean`: `--clean` wipes the output directory that holds the pre-alloc groups. `--use-pre-alloc-groups` + `--generate-all-formats` remains valid and single-phase, per the intent stated in 116e5093e83.
Swap `--generate-pre-alloc-groups` for `--generate-all-formats` in the `bench-gas` recipe. After 116e5093e83, `--generate-pre-alloc-groups` alone runs phase 1 only and produces no benchmark fixtures; the recipe needs the full two-phase run to generate the `BlockchainEngineXFixture` output it targets. Matches the flag used in `.github/configs/feature.yaml`.
Update documentation to reflect the phase-1-only behaviour of `--generate-pre-alloc-groups` introduced in 116e5093e83: - Remove the "Alternative approach" note in `filling_tests_command_line.md` that recommended `--generate-pre-alloc-groups` as a single-command replacement for `--generate-all-formats`: it now produces no fixtures. - Reframe the `blockchain_test_engine_x.md` usage notes to describe the two-invocation workflow (`--generate-pre-alloc-groups` then `--use-pre-alloc-groups`) as the explicit alternative to the single-command `--generate-all-formats` path.
danceratopz
added
C-feat
4 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2720 +/- ##
===================================================
+ Coverage 84.72% 86.26% +1.53%
===================================================
Files 524 599 +75
Lines 31117 37038 +5921
Branches 3036 3795 +759
===================================================
+ Hits 26365 31949 +5584
- Misses 4181 4525 +344
+ Partials 571 564 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🗒️ Description
Splits the overloaded
--generate-pre-alloc-groupsflag so it now runs phase 1 only: populate thepre_alloc/folder and stop. This enables splitting fixture generation across runners (phase 1 on one, phase 2 via--use-pre-alloc-groupson another) without requiring--generate-all-formatsas a wrapper.Changes:
fill.py:--generate-pre-alloc-groupsalone creates a phase-1-only execution;--generate-all-formatsremains the two-phase driver.fill.py: new_validate_flag_combinationsrejects--use-pre-alloc-groupscombined with--generate-pre-alloc-groupsor--cleanup-front with aclick.UsageError, instead of failing later with aFileNotFoundErrorfrom_load_pre_alloc_groups_from_folder.Justfile:bench-gasswitched to--generate-all-formatssince--generate-pre-alloc-groupsno longer two-phases.filling_tests_command_line.mdandblockchain_test_engine_x.mdto match the new semantics.🔗 Related Issues or PRs
N/A.
✅ Checklist
just statictype(scope):.Cute Animal Picture
🐢