Forward net_* rpc commands to the upstream #377
Conversation
| nodeConfig, err := MakeTestNodeConfig(params.RinkebyNetworkID) | ||
| require.NoError(err) | ||
| for _, upstreamEnabled := range []bool{false, true} { | ||
| s.T().Logf("TestCallRPC with upstream: %t", upstreamEnabled) |
There was a problem hiding this comment.
Let's not add logging to tests. I actually want to clean up other tests that printing logs in similar way. Let's tests be tests, no the debug session runners.
There was a problem hiding this comment.
If run without -v and successful it shouldn't print, right?
There was a problem hiding this comment.
I hope so. But in Ci we still using -v, that means more lines to scrolling, and distract attention while looking for the real FAIL messages. Logs made with Logf visually similar to test failure messages, so they adds more confusion without any benefit. Anyway, let's deal with it in another PR.
| go func(r rpcCall) { | ||
| defer wg.Done() | ||
| resultJSON := rpcClient.CallRaw(r.inputJSON) | ||
| r.validator(resultJSON) |
There was a problem hiding this comment.
I know that's code existed before PR, but I'm not sure r.validator will work as expected when launched from goroutine. Seems like testify doesn't handle such cases correctly, but I might be wrong. Anyway, it's not to be fixed here, just to keep in mind that it might the an issue with this test.
There was a problem hiding this comment.
I am not really sure why you think it won't work as expected. testify can handle running assertions in goroutines just fine.
There was a problem hiding this comment.
@adambabik because I spent yesterday an hour fighting the same problem in other test that needed to do assertions/require in goroutines :) It just doesn't print anything in case of error and it's not easy to spot.
There was a problem hiding this comment.
I usually run tests so that they fail first and then pass (so as they should be :D). It printed everything nicely and reported failure. Maybe the conditions were a little bit different? Hard to tell but here everything was ok.
net_*commands are supported by infura and should be routed there.Otherwise, as we don't run
lesservice anymore when using the upstream node, callingnet_listeningwill result in an error and Dapps will think that our node is offline.