-
Notifications
You must be signed in to change notification settings - Fork 319
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
fix tests that were failing via RPC #437
Conversation
@@ -2,10 +2,11 @@ | |||
"createNameRegistratorPerTxsNotEnoughGas" : { | |||
"_info" : { | |||
"comment" : "", | |||
"filledwith" : "cpp-1.3.0+commit.619a6438.Linux.g++", | |||
"filledwith" : "cpp-1.3.0+commit.84983edf.Linux.g++", | |||
"retesteth" : "cpp-0.0.1+commit.b346d8e6.Linux.g++", |
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.
What?
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.
retesteth commit hash
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 see. But can you rename it to something more generic. E.g. "client_version" and "tool_version". And why retesteth is called "cpp"?
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.
tool_version
does not look more generic then retesteth
cpp
states for cpp implementation
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.
@winsvega can you make it, something like
"filling-rpc-host-version" : "cpp-1.3.0+commit.84983edf.Linux.g++",
"filling-runner-version" : "retesteth-0.0.1+commit.abdcda.Linux.g++"
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.
@winsvega let's prepare for alternative implementations of retesteth
and let's assume not all of them are called retesteth
.
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.
filling-rpc-client
filling-tool-version
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.
@winsvega For consistency, you either add -version
suffix or not.
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.
@winsvega cpp-ethereum
is the RPC server, not the client. retesteth
sends requests (client) and cpp-ethereum
answers (server).
@winsvega please file a PR to |
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.
waiting for a successful Travis run with testeth
somewhere.
How was it filled, exactly? What was used for input, and what API was used? |
the input is (which are not final and it's format will probably be changed) |
@winsvega should this be ready for merge? |
cpp-ethereum testeth would fail to pass this. |
@winsvega are you going to fix cpp-ethereum? |
the tests itself might be incorrect. now moving from test_addTransaction to eth_sendRawTransaction |
touched coinbase should exist in the post state
@pirapira a PR passing this branch has just been merged to cpp. |
Here is the first test filled with retesteth via RPC and custom cpp client version.
this test should be compatible with existing cpp-develop.