-
Notifications
You must be signed in to change notification settings - Fork 285
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
test: Ensure exported state tests have all fields #993
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #993 +/- ##
=======================================
Coverage 93.84% 93.84%
=======================================
Files 146 146
Lines 15456 15460 +4
=======================================
+ Hits 14505 14509 +4
Misses 951 951
Flags with carried forward coverage won't be shown. Click here to find out more.
|
3e18395
to
52e8d22
Compare
dd97a46
to
2d4d96f
Compare
@chfast Hey I think I figured out that it is ok to add accessLists and currentRandom like that (see the recent commit). Both were reported missing by EELS team, PTAL |
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. Please update the description because it will become the git commit message.
b6c02a9
to
2acef8d
Compare
Co-authored-by: Paweł Bylica <[email protected]>
Co-authored-by: Paweł Bylica <[email protected]>
2acef8d
to
68da00b
Compare
I was wrong when I said that the format inconsistency had a test which we can make pass by amending the exported format - there were no exclusions which my fix would fix. So to have some testing mechanism of correctness, we can maybe add an assertion in
statetest_loader
like this? I don't want to overdo it and this seems safe to do.I guess we can also leave it without the assertion.
I don't know yet about the other format inconsistencies found, so I might add more commits here