-
Notifications
You must be signed in to change notification settings - Fork 37.7k
[tests] Fix RPC failure testing (again) #10853
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
0bb80b7
to
9ebed01
Compare
Rebased and scripted-diff Travis check fixed. |
Not sure if it is worth to go through the rebases due to renaming of the method. Mind to drop the last commit for now? |
I wanted the name change for a couple of reasons:
It's a scripted diff changing one function name, so rebasing shouldn't be too difficult. |
6f4b7aa
to
befcccc
Compare
be79245
to
cc5c3e4
Compare
cc5c3e4
to
9a7512c
Compare
rebased |
9a7512c
to
13836c7
Compare
rebased |
Needs rebase |
9827bee
to
7093b9a
Compare
rebased |
Needs silent merge conflicts solved. |
-BEGIN VERIFY SCRIPT- sed -i 's/assert_raises_jsonrpc/assert_raises_rpc_error/g' test/functional/*py test/functional/test_framework/*py -END VERIFY SCRIPT-
7093b9a
to
47ba8cf
Compare
Thanks. scripted-diff rerun and repushed. |
utACK 47ba8cf |
47ba8cf scripted-diff: rename assert_raises_jsonrpc to assert_raises_rpc error (John Newbery) 677d893 [tests] do not allow assert_raises_message to be called with JSONRPCException (John Newbery) 5864e9c [tests] remove direct testing on JSONRPCException from individual test cases (John Newbery) Pull request description: I did this a few months ago (here: #9707), but a few new examples have crept back in. When testing RPC failures, the test case should always assert the error value and message, to ensure that the failure was for the correct reason. Not doing that can hide bugs in the test code and mean that the test is not testing the correct behaviour. RPC failure testing should use the utility function `assert_raises_jsonrpc()` (renamed in the final commit of this PR to `assert_raises_rpc_error()`. This PR does the following: - changes all remaining instances of tests directly testing on `JSONRPCException` to calls to `assert_raises_jsonrpc()` - prevents `assert_raises_message()` from being called with `JSONRPCException` - scripted-diff changes `assert_raises_jsonrpc()` to `assert_raises_rpc_error()` Tree-SHA512: 2cc5e320704ec623a6e5a27d3c2c81cea86b502e29896f03bb5bf92cc36725132c1144410aecdf49e90d4577d512ee467d50d8184e9d5c5d0870bfc931316a5a
…t cases Github-Pull: bitcoin#10853 Rebased-From: 5864e9c
…xception Github-Pull: bitcoin#10853 Rebased-From: 677d893
-BEGIN VERIFY SCRIPT- sed -i 's/assert_raises_jsonrpc/assert_raises_rpc_error/g' test/functional/*py test/functional/test_framework/*py -END VERIFY SCRIPT- Github-Pull: bitcoin#10853 Rebased-From: 47ba8cf
I did this a few months ago (here: #9707), but a few new examples have crept back in.
When testing RPC failures, the test case should always assert the error value and message, to ensure that the failure was for the correct reason. Not doing that can hide bugs in the test code and mean that the test is not testing the correct behaviour.
RPC failure testing should use the utility function
assert_raises_jsonrpc()
(renamed in the final commit of this PR toassert_raises_rpc_error()
.This PR does the following:
JSONRPCException
to calls toassert_raises_jsonrpc()
assert_raises_message()
from being called withJSONRPCException
assert_raises_jsonrpc()
toassert_raises_rpc_error()