-
Notifications
You must be signed in to change notification settings - Fork 333
tests: discover state/blockchaintests by individual test #1331
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1331 +/- ##
==========================================
+ Coverage 87.07% 87.09% +0.02%
==========================================
Files 169 169
Lines 24820 24864 +44
Branches 4076 4081 +5
==========================================
+ Hits 21611 21656 +45
+ Misses 548 546 -2
- Partials 2661 2662 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
It seems that discovering tests individually has more problems than I thought initially, if you apply it always. Problems which arise:
Would it work for us if discover was done as it used to be (per file) when provided with a dir path (e.g. like we do in our CI) and discover per test when provided with a single EDIT: We seem to have decided it would work, this PR takes this approach |
97e3872 to
b8468ed
Compare
Co-authored-by: Paweł Bylica <[email protected]>
Tackles the point (3.) from github.com/ethereum/execution-spec-tests/pull/2243 (
consume directusingevmonein EEST).Here we switch statetest and blockchaintest runners to discover gtests by parsing the JSON files and recognizing each test.
The downside is that the invocation we use in CI (just
...evmone-statetest ... fixtures/state_tests) became very chatty, so--gtest_brief=1was added. There is an option to only go inside the JSONs when a single.jsonfile path is given (asconsume directdoes), but this would complicate things quite a bit, requiring us to have more code, but not terribly much more.Makes
consume directfor evmone version in PR mentioned 👇 work correctly