[tests] Fix RPC failure testing (again)#10853
Merged
maflcko merged 3 commits intobitcoin:masterfrom Oct 9, 2017
Merged
Conversation
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
2 times, most recently
from
0bb80b7 to
9ebed01
Compare
Contributor
Author
|
Rebased and scripted-diff Travis check fixed. |
Member
|
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? |
Contributor
Author
|
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. |
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
2 times, most recently
from
6f4b7aa to
befcccc
Compare
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
2 times, most recently
from
be79245 to
cc5c3e4
Compare
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
from
cc5c3e4 to
9a7512c
Compare
Contributor
Author
|
rebased |
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
from
9a7512c to
13836c7
Compare
Contributor
Author
|
rebased |
Member
|
Needs rebase |
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
2 times, most recently
from
9827bee to
7093b9a
Compare
Contributor
Author
|
rebased |
Member
|
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-
jnewbery
force-pushed
the
cleanup_jsonrpc_asserts
branch
from
7093b9a to
47ba8cf
Compare
Contributor
Author
|
Thanks. scripted-diff rerun and repushed. |
Member
|
utACK 47ba8cf |
maflcko
pushed a commit
that referenced
this pull request
Oct 9, 2017
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
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Nov 1, 2017
…t cases Github-Pull: bitcoin#10853 Rebased-From: 5864e9c
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Nov 1, 2017
…xception Github-Pull: bitcoin#10853 Rebased-From: 677d893
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Nov 1, 2017
-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
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
JSONRPCExceptionto calls toassert_raises_jsonrpc()assert_raises_message()from being called withJSONRPCExceptionassert_raises_jsonrpc()toassert_raises_rpc_error()