-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix Shanghai tests parsing #63
Conversation
pub(crate) const TEST_GROUPS: [&str; 1] = ["GeneralStateTests"]; | ||
// The following subgroups contain subfolders unlike the other test folders. | ||
pub(crate) const SPECIAL_TEST_SUBGROUPS: [&str; 3] = ["Cancun", "Shanghai", "VMTests"]; | ||
pub(crate) const SPECIAL_TEST_SUBGROUPS: [&str; 2] = ["Shanghai", "VMTests"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are actually no _Shanghai
tests in the Cancun
folder so it can be simply removed.
@@ -1,4 +1,4 @@ | |||
pub const GENERATION_INPUTS_DEFAULT_OUTPUT_DIR: &str = "generation_inputs"; | |||
pub const MAIN_TEST_DIR: &str = "BlockchainTests"; | |||
pub const MAIN_TEST_DIR: &str = "Cancun/BlockchainTests"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name may be confusing, but it's implying all tests until Cancun included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a comment to explain what MAIN_TEST_DIR
corresponds to then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it might be worth a comment just explaining that a sub-group is now the main sub-group, but I'm good either way.
@BGluth Thought it'd be nice to have some coverage results at each new version (similarly to what Hermez does, though we have a different coverage base). I've kicked off a job for all (including heavy) tests, will open a PR to update the README once he finishes. |
Filter unprovable killer txn tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@@ -1,4 +1,4 @@ | |||
pub const GENERATION_INPUTS_DEFAULT_OUTPUT_DIR: &str = "generation_inputs"; | |||
pub const MAIN_TEST_DIR: &str = "BlockchainTests"; | |||
pub const MAIN_TEST_DIR: &str = "Cancun/BlockchainTests"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a comment to explain what MAIN_TEST_DIR
corresponds to then?
Co-authored-by: Linda Guiga <[email protected]>
I'm not sure what you mean by that @LindaGuiga, could you clarify? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah LGTM!
@@ -1,4 +1,4 @@ | |||
pub const GENERATION_INPUTS_DEFAULT_OUTPUT_DIR: &str = "generation_inputs"; | |||
pub const MAIN_TEST_DIR: &str = "BlockchainTests"; | |||
pub const MAIN_TEST_DIR: &str = "Cancun/BlockchainTests"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it might be worth a comment just explaining that a sub-group is now the main sub-group, but I'm good either way.
Since ethereum/tests#1380, all tests prior to Cancun have been migrated to the
LegacyTests
repo, which makes the current parser unable to process tests forShanghai
.