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

eof: Disable gasTests for EOF #15653

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Conversation

rodiazet
Copy link
Contributor

@rodiazet rodiazet commented Dec 18, 2024

gasTests are supposed to be run not via IR. As long as they are run this way they cannot be compiled to EOF.

  • Make GasTest supports bytecodeFormat flag and mark them all to run only for current EVM version
  • Enable excluded earlier gasTests in CI testing for EOF.
  • Add bytecodeFormat: legacy flag to gasTests

This comment was marked as resolved.

Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

For now disabling those on EOF is ok. But the PR needs a few adjustments because the way it is now, it seems to me that the EVM and EOF versions from the test settings will get ignored. I'm actually not sure how these tests even pass without that.

Eventually we should either make them work via-IR with and without EOF or just get rid of them if they're not worth converting. But that's something for later.

.circleci/soltest.sh Show resolved Hide resolved
test/libsolidity/GasTest.h Show resolved Hide resolved
@rodiazet rodiazet force-pushed the eof-update-gas-tests branch from e03a76c to 586d314 Compare December 18, 2024 20:32
@cameel cameel merged commit 2fe4769 into ethereum:develop Dec 18, 2024
71 of 72 checks passed
shahab136586

This comment was marked as spam.

@rodiazet rodiazet deleted the eof-update-gas-tests branch December 19, 2024 08:51
@cameel cameel mentioned this pull request Dec 21, 2024
61 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants