refactor(qa): Avoid unnecessary string operations
What the developer wrote
Authored by Hodlinator
refactor(qa): Avoid unnecessary string operations
print_log was recalculated every 0.05s in assert_debug_log(), even during successful circumstances - changed to only be computed upon failure.
Simplified terminology from "(does not) partially match(es)" to "(not) found in" since it seems to reference the first function having used regular expression matching, while it always escaped the search strings (see parent commit). (Simplified grammar also avoids issues with singular/plural "was/were not found").
What changed, and why it matters
This is a minor internal cleanup in Bitcoin Core's automated testing code. It changes how debug log messages are formatted only when a test fails, and tweaks the wording of error messages. There is no security issue.
No security action needed. Treat as ordinary code-quality/test-maintenance refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors assert_debug_log() and busy_wait_for_debug_log() in test/functional/test_framework/test_node.py. It moves the print_log string construction into a local helper join_log() so it is computed only on assertion failure rather than every 0.05 s polling loop. It also converts .format() strings to f-strings and rephrases ‘(does not) partially match(es)’ to ‘(not) found in’. No functional behavior of the test framework changes; only performance of the failure-path formatting and wording of error messages.
Changed components
test/functional/test_framework/test_node.pyInspect captured patch +9 / −6
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 3a13a923..cf92f926 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -550,6 +550,9 @@ class TestNode():
time_end = time.time() + timeout * self.timeout_factor
prev_size = self.debug_log_size(encoding="utf-8") # Must use same encoding that is used to read() below
+ def join_log(log):
+ return " - " + "\n - ".join(log.splitlines())
+
yield
while True:
@@ -557,10 +560,10 @@ class TestNode():
with open(self.debug_log_path, encoding="utf-8", errors="replace") as dl:
dl.seek(prev_size)
log = dl.read()
- print_log = " - " + "\n - ".join(log.splitlines())
for unexpected_msg in unexpected_msgs:
if unexpected_msg in log:
- self._raise_assertion_error('Unexpected message "{}" partially matches log:\n\n{}\n\n'.format(unexpected_msg, print_log))
+ self._raise_assertion_error(f'Unexpected message "{unexpected_msg}" '
+ f'found in log:\n\n{join_log(log)}\n\n')
for expected_msg in expected_msgs:
if expected_msg not in log:
found = False
@@ -569,7 +572,8 @@ class TestNode():
if time.time() >= time_end:
break
time.sleep(0.05)
- self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
+ self._raise_assertion_error(f'Expected message(s) {expected_msgs!s} '
+ f'not found in log:\n\n{join_log(log)}\n\n')
@contextlib.contextmanager
def busy_wait_for_debug_log(self, expected_msgs, timeout=60):
@@ -601,9 +605,8 @@ class TestNode():
# No sleep here because we want to detect the message fragment as fast as
# possible.
- self._raise_assertion_error(
- 'Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(
- str(expected_msgs), print_log))
+ self._raise_assertion_error(f'Expected message(s) {expected_msgs!s} '
+ f'not found in log:\n\n{print_log}\n\n')
@contextlib.contextmanager
def wait_for_new_peer(self, timeout=5):
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.