This adds explicit tests for the returned HTTP status codes to interface_rpc.py (for error cases) and the HTTP JSON-RPC client in general for success.
#15381 brought up discussion about the HTTP status codes in general, and the general opinion was that the current choice may not be ideal but should not be changed to preserve compatibility with existing JSON-RPC clients. Thus it makes sense to actually test the current status to ensure this desired compatibility is not broken accidentally.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
This adds explicit tests for the returned HTTP status codes to
interface_rpc.py (for error cases) and the HTTP JSON-RPC client in
general for success.
PR 15381 brought up discussion about the HTTP status codes in general,
and the general opinion was that the current choice may not be ideal
but should not be changed to preserve compatibility with existing
JSON-RPC clients. Thus it makes sense to actually test the current
status to ensure this desired compatibility is not broken accidentally.
141 elif 'result' not in response:
142 raise JSONRPCException({
143- 'code': -343, 'message': 'missing JSON-RPC result'})
144+ 'code': -343, 'message': 'missing JSON-RPC result'}, status)
145+ elif status != HTTPStatus.OK:
146+ raise JSONRPCExceptoin({
JSONRPCExceptoin does not exist :-)
So we are testing that this unexpected situation does not occur by means of Python throwing an error otherwise. :)
Good catch, fixed!
7
8+from test_framework.authproxy import JSONRPCException
9 from test_framework.test_framework import BitcoinTestFramework
10 from test_framework.util import assert_equal, assert_greater_than_or_equal
11
12+def expect_http_status(expected_http_status, expected_rpc_code,
test/functional/test_framework/util.py or even try to extend assert_raises_rpc_error?
I’m not sure if that makes sense - checking the HTTP status code is a rather specific thing, which I don’t expect us to do outside of this one test. Thus I think it makes sense to keep the code here.
But if you disagree and see us checking status codes in other tests as well, I’m happy to move this to a more general place.
assert_raises_rpc_error to optionally check the HTTP status.)
