Skip to content

chore(packages/tests): update bal mappers#1661

Merged
spencer-tb merged 2 commits intoethereum:forks/osakafrom
spencer-tb:packages-tests-bal-cli-mappers
Oct 28, 2025
Merged

chore(packages/tests): update bal mappers#1661
spencer-tb merged 2 commits intoethereum:forks/osakafrom
spencer-tb:packages-tests-bal-cli-mappers

Conversation

@spencer-tb
Copy link
Contributor

@spencer-tb spencer-tb commented Oct 23, 2025

🗒️ Description

Note: must be rebased once #1654 is merged.

This PR should make all ELs (BAL enabled) pass all the consume engine tests. I'm not sure we need to be super strict on the exception checking for BAL. I think we should only have a single BAL exception based on what the clients are returning. More granularity would be nice for debugging, but if clients are failing for the BAL hash mismatch at the highest level (i.e not stating why) I feel this is enough. This PR is also to prompt discussion.

🔗 Related Issues or PRs

N/A.

✅ 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 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).

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@spencer-tb spencer-tb added E-easy Experience: easy, good for newcomers P-medium F-amsterdam C-chore Category: chore A-test-client-clis Area: execution_testing.client_clis labels Oct 23, 2025
fselmo pushed a commit to fselmo/execution-specs that referenced this pull request Oct 23, 2025
…tests (ethereum#1661)

* fix(plugins/ported_from): Fix and add `--ported-from-output-file` parameter

* fix(github/coverage): Use `--ported-from-output-file`

* changelog
@spencer-tb spencer-tb requested a review from fselmo October 24, 2025 07:50
chetna-mittal pushed a commit to gnosischain/execution-specs that referenced this pull request Oct 24, 2025
…tests (ethereum#1661)

* fix(plugins/ported_from): Fix and add `--ported-from-output-file` parameter

* fix(github/coverage): Use `--ported-from-output-file`

* changelog
@SamWilsn SamWilsn added this to the amsterdam milestone Oct 27, 2025
@fselmo fselmo force-pushed the packages-tests-bal-cli-mappers branch from 5224778 to 05840dc Compare October 27, 2025 21:06
Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

@spencer-tb this lgtm if we want to simplify things for now 👍🏼. I rebased and cleaned up the linting errors. I was going to add the mapping in this PR as well so that we can get tests passing on hive in one PR. Didn't get time today but I will work on this tomorrow. If you feel like this is not needed in the same PR, don't let this be a blocker, I'm approving 👍🏼. I think it would be nice to go through the hive flow and add the mapping here though.

Also, I suppose this makes sense in forks/osaka... but this is messy having to recreate, rebase, etc, for forks/amsterdam with every change. Thought for another scope but we should figure out how to improve the devex here 😅.

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.07%. Comparing base (de8973e) to head (825950f).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##           forks/osaka    #1661   +/-   ##
============================================
  Coverage        86.07%   86.07%           
============================================
  Files              743      743           
  Lines            44078    44078           
  Branches          3894     3894           
============================================
  Hits             37938    37938           
  Misses            5659     5659           
  Partials           481      481           
Flag Coverage Δ
unittests 86.07% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@spencer-tb
Copy link
Contributor Author

Thanks for fixing! Will merge and can follow up in another PR.

I was going to add the mapping in this PR as well so that we can get tests passing on hive in one PR.

Do you mean the ruleset here: https://github.com/ethereum/execution-specs/blob/forks/osaka/packages/testing/src/execution_testing/cli/pytest_commands/plugins/consume/simulators/helpers/ruleset.py#L483-L504
I'm hoping once this is merged all of https://hive.ethpandaops.io/#/group/bal will pass!

Also, I suppose this makes sense in forks/osaka... but this is messy having to recreate, rebase, etc, for forks/amsterdam with every change. Thought for another scope but we should figure out how to improve the devex here 😅.

I commented a small potential improvement here for the STEEL call: ethsteel/pm#4 (comment)

@spencer-tb spencer-tb merged commit a191de9 into ethereum:forks/osaka Oct 28, 2025
11 checks passed
@fselmo

This comment was marked as resolved.

@rakita

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-client-clis Area: execution_testing.client_clis C-chore Category: chore E-easy Experience: easy, good for newcomers P-medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants