mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-08-29 12:48:11 +02:00
Merge bitcoin/bitcoin#32509: qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)
bf950c4544
qa: Improve suppressed errors output (Hodlinator)075352ec8e
qa: assert_raises_message() - search in str(e) (Hodlinator)bd8ebbc4ab
qa: Make --timeout-factor=0 result in a smaller factor (Hodlinator)d8f05e7bf3
qa: Fix dormant bug caused by multiple --tmpdir (Hodlinator) Pull request description: * Handle multiple `--tmpdir` args properly. * Handle `--timeout-factor=0` properly (fixes #32506). * Improve readability of expected error message (`assert_raises_message()`). * Make suppressed error output less confusing (`wait_for_rpc_connection()`). ACKs for top commit: i-am-yuvi: re-ACKbf950c4544
ismaelsadeeq: Code review and tested ACKbf950c45
achow101: ACKbf950c4544
janb84: LGTM ACKbf950c4544
Tree-SHA512: 8993f53e962231adef9cad92594dc6a8b8e979b1571d1381cd0fffa1929833b2163c68c03b6a6e29995006a3c3121822ec25ac7725e71ccdab8876ac24aae86c
This commit is contained in:
@@ -36,15 +36,11 @@ class FeatureFrameworkStartupFailures(BitcoinTestFramework):
|
||||
# Launches a child test process that runs this same file, but instantiates
|
||||
# a child test. Verifies that it raises only the expected exception, once.
|
||||
def _verify_startup_failure(self, test, internal_args, expected_exception):
|
||||
# Inherit args from parent, only modifying tmpdir so children don't fail
|
||||
# as a cause of colliding with the parent dir.
|
||||
parent_args = sys.argv.copy()
|
||||
# Inherit sys.argv from parent, only overriding tmpdir to a subdirectory
|
||||
# so children don't fail due to colliding with the parent dir.
|
||||
assert self.options.tmpdir, "Framework should always set tmpdir."
|
||||
i, path = next(((i, m[1]) for i, arg in enumerate(parent_args) if (m := re.match(r'--tm(?:p(?:d(?:ir?)?)?)?=(.+)', arg))),
|
||||
(len(parent_args), self.options.tmpdir))
|
||||
subdir = md5(expected_exception.encode('utf-8')).hexdigest()[:8]
|
||||
parent_args[i:i + 1] = [f"--tmpdir={path}/{subdir}"]
|
||||
args = [sys.executable] + parent_args + [f"--internal_test={test.__name__}"] + internal_args
|
||||
args = [sys.executable] + sys.argv + [f"--tmpdir={self.options.tmpdir}/{subdir}", f"--internal_test={test.__name__}"] + internal_args
|
||||
|
||||
try:
|
||||
# The subprocess encoding argument gives different results for e.output
|
||||
@@ -73,10 +69,10 @@ class FeatureFrameworkStartupFailures(BitcoinTestFramework):
|
||||
self.log.info("Verifying _verify_startup_failure() functionality (self-check).")
|
||||
assert_raises_message(
|
||||
AssertionError,
|
||||
("Child test didn't contain (only) expected errors:\n" +
|
||||
linesep.join(["Found 0/1 tracebacks - expecting exactly one with no knock-on exceptions.",
|
||||
"Found 0/1 occurrences of the specific exception: NonExistentError",
|
||||
"Found 0/1 test failure output messages."])).encode("unicode_escape").decode("utf-8"),
|
||||
( "Child test didn't contain (only) expected errors:\n"
|
||||
f"Found 0/1 tracebacks - expecting exactly one with no knock-on exceptions.{linesep}"
|
||||
f"Found 0/1 occurrences of the specific exception: NonExistentError{linesep}"
|
||||
"Found 0/1 test failure output messages."),
|
||||
self._verify_startup_failure,
|
||||
TestSuccess, [],
|
||||
"NonExistentError",
|
||||
@@ -93,7 +89,7 @@ class FeatureFrameworkStartupFailures(BitcoinTestFramework):
|
||||
self.log.info("Verifying inability to connect to bitcoind's RPC interface due to wrong port results in one exception containing at least one OSError.")
|
||||
self._verify_startup_failure(
|
||||
TestWrongRpcPortStartupFailure, [f"--internal_node_start_duration={node_start_duration}"],
|
||||
r"AssertionError: \[node 0\] Unable to connect to bitcoind after \d+s \(ignored errors: {[^}]*'OSError \w+'?: \d+[^}]*}, latest error: \w+\([^)]+\)\)"
|
||||
r"AssertionError: \[node 0\] Unable to connect to bitcoind after \d+s \(ignored errors: {[^}]*'OSError \w+'?: \d+[^}]*}, latest: '[\w ]+'/\w+\([^)]+\)\)"
|
||||
)
|
||||
|
||||
self.log.info("Verifying startup failure due to invalid arg results in only one exception.")
|
||||
|
@@ -267,7 +267,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
|
||||
parser.add_argument("-f", "--fff", help="a dummy argument to fool ipython", default="1")
|
||||
self.options = parser.parse_args()
|
||||
if self.options.timeout_factor == 0:
|
||||
self.options.timeout_factor = 99999
|
||||
self.options.timeout_factor = 999
|
||||
self.options.timeout_factor = self.options.timeout_factor or (4 if self.options.valgrind else 1)
|
||||
self.options.previous_releases_path = previous_releases_path
|
||||
|
||||
|
@@ -262,8 +262,13 @@ class TestNode():
|
||||
"""Sets up an RPC connection to the bitcoind process. Returns False if unable to connect."""
|
||||
# Poll at a rate of four times per second
|
||||
poll_per_s = 4
|
||||
|
||||
suppressed_errors = collections.defaultdict(int)
|
||||
latest_error = ""
|
||||
latest_error = None
|
||||
def suppress_error(category: str, e: Exception):
|
||||
suppressed_errors[category] += 1
|
||||
return (category, repr(e))
|
||||
|
||||
for _ in range(poll_per_s * self.rpc_timeout):
|
||||
if self.process.poll() is not None:
|
||||
# Attach abrupt shutdown error/s to the exception message
|
||||
@@ -317,8 +322,7 @@ class TestNode():
|
||||
# -342 Service unavailable, could be starting up or shutting down
|
||||
if e.error['code'] not in [-28, -342]:
|
||||
raise # unknown JSON RPC exception
|
||||
suppressed_errors[f"JSONRPCException {e.error['code']}"] += 1
|
||||
latest_error = repr(e)
|
||||
latest_error = suppress_error(f"JSONRPCException {e.error['code']}", e)
|
||||
except OSError as e:
|
||||
error_num = e.errno
|
||||
# Work around issue where socket timeouts don't have errno set.
|
||||
@@ -334,16 +338,14 @@ class TestNode():
|
||||
errno.ECONNREFUSED # Port not yet open?
|
||||
]:
|
||||
raise # unknown OS error
|
||||
suppressed_errors[f"OSError {errno.errorcode[error_num]}"] += 1
|
||||
latest_error = repr(e)
|
||||
latest_error = suppress_error(f"OSError {errno.errorcode[error_num]}", e)
|
||||
except ValueError as e:
|
||||
# Suppress if cookie file isn't generated yet and no rpcuser or rpcpassword; bitcoind may be starting.
|
||||
if "No RPC credentials" not in str(e):
|
||||
raise
|
||||
suppressed_errors["missing_credentials"] += 1
|
||||
latest_error = repr(e)
|
||||
latest_error = suppress_error("missing_credentials", e)
|
||||
time.sleep(1.0 / poll_per_s)
|
||||
self._raise_assertion_error(f"Unable to connect to bitcoind after {self.rpc_timeout}s (ignored errors: {str(dict(suppressed_errors))}, latest error: {latest_error})")
|
||||
self._raise_assertion_error(f"Unable to connect to bitcoind after {self.rpc_timeout}s (ignored errors: {dict(suppressed_errors)!s}{'' if latest_error is None else f', latest: {latest_error[0]!r}/{latest_error[1]}'})")
|
||||
|
||||
def wait_for_cookie_credentials(self):
|
||||
"""Ensures auth cookie credentials can be read, e.g. for testing CLI with -rpcwait before RPC connection is up."""
|
||||
|
@@ -102,9 +102,9 @@ def assert_raises_message(exc, message, fun, *args, **kwds):
|
||||
except JSONRPCException:
|
||||
raise AssertionError("Use assert_raises_rpc_error() to test RPC failures")
|
||||
except exc as e:
|
||||
if message is not None and message not in repr(e):
|
||||
if message is not None and message not in str(e):
|
||||
raise AssertionError("Expected substring not found in exception:\n"
|
||||
f"substring: '{message}'\nexception: {repr(e)}.")
|
||||
f"substring: '{message}'\nexception: {e!r}.")
|
||||
except Exception as e:
|
||||
raise AssertionError("Unexpected exception raised: " + type(e).__name__)
|
||||
else:
|
||||
|
Reference in New Issue
Block a user