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

Migrate benchmarks to the JSON State Test format #513

Merged
merged 3 commits into from
Oct 23, 2022

Conversation

miles170
Copy link
Contributor

@miles170 miles170 commented Oct 6, 2022

Closes #511.

I haven't checked gas used in every benchmark loop iteration because I want to make sure current changes won't break anything.
After that, I'll add a check for gas used.

@axic axic requested a review from chfast October 20, 2022 11:33
@miles170 miles170 force-pushed the migrate-to-state-test branch 2 times, most recently from 0e16776 to f965a83 Compare October 21, 2022 02:16
test/bench/CMakeLists.txt Outdated Show resolved Hide resolved
test/statetest/statetest_loader.cpp Outdated Show resolved Hide resolved
test/state/state.hpp Outdated Show resolved Hide resolved
@miles170 miles170 force-pushed the migrate-to-state-test branch 2 times, most recently from 66fce18 to c68c2df Compare October 21, 2022 12:50
CMakeLists.txt Outdated Show resolved Hide resolved
test/bench/bench.cpp Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
test/bench/CMakeLists.txt Outdated Show resolved Hide resolved
test/statetest/statetest_loader.cpp Outdated Show resolved Hide resolved
@miles170 miles170 force-pushed the migrate-to-state-test branch 2 times, most recently from ab99342 to 439de19 Compare October 22, 2022 02:45
@codecov
Copy link

codecov bot commented Oct 22, 2022

Codecov Report

Merging #513 (ec9e5ae) into master (13a7873) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #513   +/-   ##
=======================================
  Coverage   99.06%   99.06%           
=======================================
  Files          54       54           
  Lines        5341     5344    +3     
=======================================
+ Hits         5291     5294    +3     
  Misses         50       50           
Flag Coverage Δ
blockchaintests 77.78% <ø> (ø)
statetests 7.55% <100.00%> (+0.05%) ⬆️
unittests 97.18% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/state/state.hpp 80.00% <ø> (ø)
test/statetest/statetest.hpp 100.00% <ø> (ø)
test/statetest/statetest_loader.cpp 81.35% <100.00%> (+0.48%) ⬆️

@miles170
Copy link
Contributor Author

miles170 commented Oct 22, 2022

Should I add more test cases to GeneralStateTests(https://github.com/ethereum/tests)?

@chfast
Copy link
Member

chfast commented Oct 22, 2022

Should I add more test cases to GeneralStateTests(https://github.com/ethereum/tests)?

If you are asking in context of code coverage, then this is not needed. We don't run the tests yet. But on the other hand it is always welcomed to add new tests if you have any ideas.

@chfast chfast merged commit f67b7da into ethereum:master Oct 23, 2022
@miles170 miles170 deleted the migrate-to-state-test branch October 23, 2022 10:52
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.

Migrate to benchmarks in JSON format
2 participants