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

EipStateTests #1110

Merged
merged 1 commit into from
Dec 8, 2022
Merged

EipStateTests #1110

merged 1 commit into from
Dec 8, 2022

Conversation

winsvega
Copy link
Collaborator

@winsvega winsvega commented Dec 2, 2022

No description provided.

@gumb0
Copy link
Member

gumb0 commented Dec 5, 2022

To me it would make more sense to still have it in GeneralsStateTests, e.g.
src/EIPStateTests => src/GeneralStateTests/EIPTests

@winsvega
Copy link
Collaborator Author

winsvega commented Dec 5, 2022

The reason it is out of state tests since it represent not a mandatory tests. But yes, can consider that too.

@gumb0
Copy link
Member

gumb0 commented Dec 5, 2022

Other top-level folders corrsepond to kinds of tests - State, Blockchain, Transaction... "EIPs" is not a separate kind of tests, it's state tests, too.
(having non-mandatory forks like Merge+3540 inside makes it possible to "skip" them if not implemented, but yeah you still would run these files if you run all state tests)

@holgerd77
Copy link
Contributor

To me it would make more sense to still have it in GeneralsStateTests, e.g. src/EIPStateTests => src/GeneralStateTests/EIPTests

Yes, think I would prefer this too (writing this after my comment on our Discord where I expressed general agreement in a first-round).

@winsvega
Copy link
Collaborator Author

winsvega commented Dec 7, 2022

ok moved there.
and in /src/BlockchainTestsFiller/EIPTests for many tx per block tests too.

@winsvega winsvega merged commit c896d1e into develop Dec 8, 2022
@winsvega winsvega deleted the EIPStateTests branch December 8, 2022 12:57
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