Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new(tests): EOF - EIP-6206: JUMPF Tests #540

Merged
merged 17 commits into from
May 23, 2024
Merged

Conversation

shemnon
Copy link
Collaborator

@shemnon shemnon commented May 2, 2024

🗒️ Description

Tests the assertsions in EIP-6206. Both container validation and
runtime execution are validated.

🔗 Related Issues

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@shemnon shemnon changed the title 1EOF JUMPF Tests EOF JUMPF Tests May 2, 2024
@shemnon shemnon marked this pull request as ready for review May 2, 2024 03:34
Tests the assertsions in EIP-6206.  Both container validation and
runtime execution are validated.

Signed-off-by: Danno Ferrin <[email protected]>
- move to parameterized calls
- combine eof and state tests in one go
- change file groupings

Signed-off-by: Danno Ferrin <[email protected]>
@shemnon shemnon requested a review from winsvega May 3, 2024 16:32
Copy link
Collaborator

@winsvega winsvega left a comment

Choose a reason for hiding this comment

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

now it looks much better. just add a little more text comments describing what is happening. and ask @marioevz about state_test invoke paralel to eof test.

Add comments to stack calculation variables.

Signed-off-by: Danno Ferrin <[email protected]>
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.

Looks good, one open question is whether we want the kind of logic from execute_tests to be embedded directly into the eof_test logic. This would enable generating state or blockchain tests for all EOF tests automatically, but my main concern is that it would be a lot of them.

tests/prague/eip6206_jumpf/helpers.py Outdated Show resolved Hide resolved
ids=lambda x: "to-%s" % ("N" if x == NON_RETURNING_SECTION else x),
)
@pytest.mark.parametrize(
"current_outputs",
Copy link
Member

Choose a reason for hiding this comment

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

Thinking that changing the word "current" to "source" (so we would have source section and target section) might make it easier to understand. Wdyt @shemnon ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sounds good. It also matches the EIP better.

@marioevz marioevz changed the title EOF JUMPF Tests new(tests): EOF - EIP-6206: JUMPF Tests May 23, 2024
@marioevz marioevz merged commit 647a55a into ethereum:main May 23, 2024
5 checks passed
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.

3 participants