-
Notifications
You must be signed in to change notification settings - Fork 192
refactor(benchmark): rename zkevm tests to benchmark
#1804
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
Conversation
marioevz
left a comment
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.
Looks good to me, I think we should go ahead and do the rename.
The fixture issue has an easy fix IMO: #1778 (comment)
We should also modify https://github.com/ethereum/execution-spec-tests/blob/main/.github/configs/feature.yaml in this same PR and rename all zkevm references to benchmark.
zkevm tests to benchmark
zkevm tests to benchmarkzkevm tests to benchmark
danceratopz
left a comment
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.
Thanks @spencer-tb. Would you mind taking care of the release naming (zkevm -> benchmark in .github/configs/evm.yaml and .github/configs/evm.yaml if appropriate?
zkevm tests to benchmarkzkevm tests to benchmark
3c07fca to
4c7f819
Compare
|
I updated the docs in this commit to assume a I think after we merge this we finish the following:
Then we can spin up a |
4c7f819 to
36b20fd
Compare
zkevm tests to benchmarkzkevm tests to benchmark
marioevz
left a comment
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, just two small comments. Thanks!
|
I feel like there's no way the Evmone Coverage Report / evmone-coverage-diff (pull_request) is going to complete successfully due to the amount of cpu intensive tests that are being touched. The only way the coverage would complete is if it could intelligently determine that the fixtures are exactly the same and have just been renamed, and then skip those, but that's not yet a capability of the coverage script sadly. If it fails IMO we should still merge. |
We should consider adding the hasher logic to the coverage script for this! (holds urge to YOLO merge :P) |
* chore: rename zkevm tests to benchmark. * chore(docs): changelog. * refactor(tests,fw,tox,docs): rename zkevm marker to benchmark * chore(tests): remove zkevm specific nomenclature from docstrings * chore(ci|docs): rename zkevm releases to benchmark. * chore(docs): align changelog docs for future releases. * chore: final tweaks after rebase. * Apply suggestions from code review --------- Co-authored-by: danceratopz <[email protected]> Co-authored-by: Mario Vega <[email protected]>
ποΈ Description
Renames
tests/zkevmtotests/benchmark. Just to get the process started.Additionally renames the releases & markers to benchmark.
π Related Issues
This change will affect all open zkevm/benchmark test PRs.
β Checklist