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

Exclude certain Ethereum tests in CI #15

Open
PaulRBerg opened this issue Sep 27, 2023 · 5 comments
Open

Exclude certain Ethereum tests in CI #15

PaulRBerg opened this issue Sep 27, 2023 · 5 comments
Assignees
Labels
effort: medium Default level of effort. priority: 2 We will do our best to deal with this. type: ci Changes to our CI configuration files and scripts. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.

Comments

@PaulRBerg
Copy link
Member

PaulRBerg commented Sep 27, 2023

We need to exclude certain tests, e.g.,

  • Those that check the chain ID
  • Those that compare the state roots

https://github.com/sablier-labs/sabvm/actions/runs/6315064104/job/17146750681

@PaulRBerg PaulRBerg added effort: medium Default level of effort. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise. type: ci Changes to our CI configuration files and scripts. priority: 2 We will do our best to deal with this. and removed testing labels Jan 1, 2024
@PaulRBerg PaulRBerg changed the title Exclude Ethereum tests in CI Exclude certain Ethereum tests in CI May 30, 2024
@PaulRBerg
Copy link
Member Author

@IaroslavMazur could you please handle this task to make the Ethereum tests on #155 pass?

@IaroslavMazur
Copy link
Member

The root problem behind this Issue (SabVM failing the Ethereum tests) has been addressed. But with a twist: in the meantime, several Ethereum Tests have been "broken" (causing SabVM to now fail the respective tests for no fault of ours).

There's already a PR with a solution to the above breakage: fix: wrong json format issue #1384. I've applied the solution from the PR locally - and successfully ran all of the tests afterwards.

@PaulRBerg, having said this, should I close this issue as Not Planned, given that the root problem has been fixed differently from what the discription of this Issue suggested?

@IaroslavMazur
Copy link
Member

The aforementioned PR has been merged into the Ethereum Tests repo, and now, SabVM passes all of the Ethereum State Tests 💪

@PaulRBerg
Copy link
Member Author

Great news!

should I close this issue as Not Planned

Why?

Could you explain how do the tests that check for the chain ID pass in SabVM?

@IaroslavMazur
Copy link
Member

Could you explain how do the tests that check for the chain ID pass in SabVM?

While our 706 chain id is, indeed, the default value for the chain_id field inside the Env's CfgEnv field, it is being overwritten when the Ethereum tests are being executed by the runner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Default level of effort. priority: 2 We will do our best to deal with this. type: ci Changes to our CI configuration files and scripts. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.
Projects
None yet
Development

No branches or pull requests

2 participants