test: Avoid F541 (f-string without any placeholders) #31417

An extra f string-prefix is mostly harmless, but could be confusing or hint to a mistake where a format argument was forgotten.

Try to avoid the confusion and mistakes by applying the F541 linter rule.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31417.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #28676 ([WIP] Cluster mempool implementation by sdaftuar)

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.

Hi @maflcko, Tested ACK 531621d5

Tested ACK bd8312f

Thank you for addressing this. I understand the rationale behind the change: preventing potential confusion caused by f-strings without placeholders and mitigating mistakes where format arguments may have been inadvertently omitted.

While this is indeed a minor and harmless modification, I tested the changes by rebuilding the source and running the modified test suite. Everything functioned as expected.

<details> <summary>For reference, here is my testing environment and test results:</summary>

lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.5 LTS
Release:	22.04
Codename:	jammy

python3 --version
Python 3.10.12

Below is a screenshot of the test output for verification:

</details> To further justify this change and provide reference material for future contributors, I'd like to cite the following:

1. Ruff rule F541 documentation:

   An f-string without placeholders can be confusing for readers, who may expect such a placeholder to be present. An f-string without any placeholders could also indicate that the author forgot to add a placeholder expression.

2. PEP 498:

   The parts of the f-string outside of braces are literal strings.

This change aligns well with best practices and coding standards, ensuring improved readability and reducing potential errors in the future.

Thanks again for your contribution!

Tested ACK 531621d5

I don't think this commit hash is right (531621d5 doesn't exist). It would be bd8312fc31, but I just force pushed to add a new hunk to the diff.

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 At least one of the CI tasks failed. _{Debug: https://github.com/bitcoin/bitcoin/runs/33958434869}

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

Tested ACK fae7639

Hi @maflcko,

Apologies for the confusion in my earlier comment. I mistakenly referenced only the last eight characters of the commit hash instead of the first seven, which may have caused some misunderstanding.

To clarify, I have re-run all the tests, thoroughly verified the correct commit hash, and can confirm that everything works as expected:

<details> <summary>Test Results</summary>

</details>

Thank you for your understanding, and I apologize for any inconvenience caused.

ACK fae76393bdbf867176e65447799d6ee3d3567b18

Code review ACK fae76393bdbf867176e65447799d6ee3d3567b18

The style guidelines prefer f-string over .format(), but it's a preference and the minimal change in the PR seems reasonable when compared to something decomposed like the following:

diff --git a/contrib/tracing/log_raw_p2p_msgs.py b/contrib/tracing/log_raw_p2p_msgs.py
index 9cda7fd08d4..864847983a9 100755
--- a/contrib/tracing/log_raw_p2p_msgs.py
+++ b/contrib/tracing/log_raw_p2p_msgs.py
@@ -117,19 +117,13 @@ int trace_outbound_message(struct pt_regs *ctx) {
 
 
 def print_message(event, inbound):
-    print("{} {} msg '{}' from peer {} ({}, {}) with {} bytes: {}".format(
-
-              "Warning: incomplete message (only {} out of {} bytes)!".format(
-                  len(event.msg), event.msg_size) if len(event.msg) < event.msg_size else "",
-              "inbound" if inbound else "outbound",
-              event.msg_type.decode("utf-8"),
-              event.peer_id,
-              event.peer_conn_type.decode("utf-8"),
-              event.peer_addr.decode("utf-8"),
-              event.msg_size,
-              bytes(event.msg[:event.msg_size]).hex(),
-          )
-          )
+    warning = "Warning: incomplete message (only {len(event.msg)} out of {event.msg_size} bytes)!" if len(event.msg) < event.msg_size else ""
+    direction = "inbound" if inbound else "outbound"
+    msg_type = event.msg_type.decode("utf-8")
+    conn_type = event.peer_conn_type.decode("utf-8")
+    peer_addr = event.peer_addr.decode("utf-8")
+    num_bytes = bytes(event.msg[:event.msg_size]).hex()
+    print(f"{warning} {direction} msg '{msg_type}' from peer {event.peer_id} ({conn_type}, {peer_addr}) with {event.msg_size} bytes: {num_bytes}")
 
 
 def main(pid):

Use f'{x}' for string formatting in preference to '{}'.format(x) or '%s' % x. https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines