-
Notifications
You must be signed in to change notification settings - Fork 70
Test: Fix parsing of contracts in ethereum-tests #908
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
b74b47d to
5d2b8b0
Compare
msooseth
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.
My only concern is the cache JSON being removed. Now we don't check the JSON cache? Maybe it'd need a separate PR? What do you think? Otherwise, this is great, thanks!
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.
Shouldn't there be a cache JSON so it can be loaded for the test?
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.
I think @msooseth is correct, this file only needs a small modification and it should work.
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 file is not being removed. Only external fields are removed.
I think GitHub's UI is giving you a false impression. |
Due to the changes in RPC cache, parsing of contracts in blockchain tests was failing. The solution here is to use a dedicated type, `RPCContract` for serializing and deserializing contracts. To fix the blockchain tests, we bring back the JSON parsing code that was used there before. As a next step, we could refactor also how contracts are handled in these tests.
5d2b8b0 to
1fa1411
Compare
msooseth
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.
OK, tested and it works as expected! Thanks!
Description
Due to the changes in RPC cache, parsing of contracts in blockchain
tests was failing.
The solution here is to use a dedicated type,
RPCContractforserializing and deserializing contracts.
To fix the blockchain tests, we bring back the JSON parsing code that
was used there before.
As a next step, we could refactor also how contracts are handled in
these tests.
Fixes #907.
Checklist