Skip to content

Conversation

@fselmo
Copy link
Contributor

@fselmo fselmo commented Nov 3, 2025

🗒️ Description

When we have two Unscheduled forks, which will be more frequent moving forward, while Osaka is Unscheduled and Amsterdam is too, we cannot resolve the fork criteria for the e2e test for the fork tool (e.g. here).

  • We can check in builder.py if the fork is Unscheduled as well as if it's forks[-1] and raise the order_index to come after it as we already do.
  • We can always use the last fork dynamically in this e2e test so that we always resolve and don't hard-code Osaka into it.

I'm not tied to the last change. If we want to make hard-coding Osaka work, we can change some other things around as well but this change does get this test passing on Amsterdam as it uses Amsterdam as the last fork.

✅ 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: Set appropriate labels for the changes (only maintainers can apply labels) --- question below

Cute Animal Picture

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

@fselmo
Copy link
Contributor Author

fselmo commented Nov 3, 2025

Are we perhaps missing a label for spec-tests or something that signifies json_infra/ tests?

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.

Might be worth having a test_end_to_end_latest and keeping test_end_to_end as is. That way we can tell if the new fork tool breaks for old forks.

@fselmo fselmo force-pushed the fix/fork-criteria-e2e-test-fix branch from a870452 to e4bb577 Compare November 4, 2025 18:23
@fselmo fselmo added the A-spec-tests Area: tests for specifications e.g. json_infra label Nov 4, 2025
@fselmo
Copy link
Contributor Author

fselmo commented Nov 4, 2025

Are we perhaps missing a label for spec-tests or something that signifies json_infra/ tests?

I added A-spec-tests. Feel free to change the description or remove it but I felt a good label for this was missing. [Hmm... I think C-test then A-spec-tool works here?]

@fselmo fselmo marked this pull request as ready for review November 4, 2025 18:26
@fselmo fselmo requested a review from SamWilsn November 4, 2025 18:26
@fselmo
Copy link
Contributor Author

fselmo commented Nov 4, 2025

Might be worth having a test_end_to_end_latest and keeping test_end_to_end as is. That way we can tell if the new fork tool breaks for old forks.

@SamWilsn I split these to have both a hard-coded Osaka and the latest fork. I didn't see a strong reason to change the parameters for these but let me know if you think we should.

@fselmo
Copy link
Contributor Author

fselmo commented Nov 4, 2025

Just sanity checked this by cherry-picking these commits over in Amsterdam bals branch and they pass 👌🏼

@fselmo fselmo added C-chore Category: chore C-test Category: test A-spec-tool Area: Specification Tooling—Tools for the Ethereum specification (eg. `src/ethereum_spec_tools/*`) and removed C-chore Category: chore A-spec-tests Area: tests for specifications e.g. json_infra labels Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-spec-tool Area: Specification Tooling—Tools for the Ethereum specification (eg. `src/ethereum_spec_tools/*`) C-test Category: test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants