Skip to content

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Sep 4, 2025

πŸ—’οΈ Description

  • Restrict extra args being passed to any BaseTest implementation
  • Fix benign issues with restricting extra args - conveniently caught by mypy without needing to run all the tests πŸ‘ŒπŸΌ
  • Fix valid BAL tests to pass expected BAL on each Block, not on the BlockchainTest class

πŸ”— Related Issues or PRs

#2067

βœ… Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • 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.

@fselmo fselmo added type:bug Something isn't working scope:tests Scope: Changes EL client test cases in `./tests` scope:fw Scope: Framework (evm|tools|forks|pytest) fork: amsterdam Amsterdam hard fork labels Sep 4, 2025
- refactor(tests): Remove benign extra args to blockchain tests

  After restricting extra args, some tests were failing because
  they passed extra args that do not exist on the `BlockchainTest` class.
  This does not affect the test itself but now will cause failure.
  Thankfully, mypy caught all of these so it was easier to identify.
- BAL tests were wrongly passing the expected BAL to the ``blockchain_test``
  itself. This was updated to live on the ``Block`` itself, but the tests
  were not updated. With the recent changes to restrict extra fields on
  the ``BlockchainTest`` (``BaseTest`` classess), this now correctly
  raises an error and allows us to have a sanity check.
@fselmo fselmo force-pushed the fix/bal-tests-and-test-constraints branch from accfedd to b2fb124 Compare September 4, 2025 15:04
@fselmo fselmo requested a review from marioevz September 4, 2025 15:05
@fselmo fselmo self-assigned this Sep 4, 2025
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM! I don't know how we survived this long without this, but better late than never, tysm!

@marioevz marioevz merged commit 14ed88d into ethereum:main Sep 4, 2025
16 checks passed
@fselmo fselmo deleted the fix/bal-tests-and-test-constraints branch September 4, 2025 16:13
LouisTsai-Csie pushed a commit to LouisTsai-Csie/execution-spec-tests that referenced this pull request Sep 5, 2025
… BAL tests (ethereum#2102)

* - fix(fw): Restrict extra fields for ``BaseTest`` types
- refactor(tests): Remove benign extra args to blockchain tests

  After restricting extra args, some tests were failing because
  they passed extra args that do not exist on the `BlockchainTest` class.
  This does not affect the test itself but now will cause failure.
  Thankfully, mypy caught all of these so it was easier to identify.

* fix(tests,bal): Fix unreleased BAL tests from old design

- BAL tests were wrongly passing the expected BAL to the ``blockchain_test``
  itself. This was updated to live on the ``Block`` itself, but the tests
  were not updated. With the recent changes to restrict extra fields on
  the ``BlockchainTest`` (``BaseTest`` classess), this now correctly
  raises an error and allows us to have a sanity check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fork: amsterdam Amsterdam hard fork scope:fw Scope: Framework (evm|tools|forks|pytest) scope:tests Scope: Changes EL client test cases in `./tests` type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants