test: consolidate to f-strings (part 1)#22229
Conversation
|
Concept ACK |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
|
Concept ACK |
|
Concept ACK and good idea. Are you using a tool (flynt?) to do this? I guess it requires a good amount of review since there are some cases where there are discrepancies between string formats. I’ll trying running flynt and diffing it against this PR to see that in the next day or so, and give this some more review. I may also trying diffing test logs before/after these changes. Will get back after that. Any thoughts on squashing commits? |
|
Concept ACK good GOHIO (get our house in order) PR |
|
Concept ACK |
mjdietzx
left a comment
mjdietzx
left a comment
There was a problem hiding this comment.
Code review ACK, went through line by line, wasn't as much/bad as I expected
Also ran tests and scrolled through (skimming them) and didn't see anything weird that would indicate a malformed string.
|
Concept ACK |
fanquake
force-pushed
the
consolidate_to_f_strings
branch
from
705d503 to
68faa87
Compare
|
Rebased, addressed comment, and squashed these down a bit. |
|
reACK 68faa87 |
|
|
||
| # Check chainTip response | ||
| json_obj = self.test_rest_request("/getutxos/{}-{}".format(*spending)) | ||
| json_obj = self.test_rest_request(f"/getutxos/{spending[0]}-{spending[1]}") |
There was a problem hiding this comment.
While f-strings are preferred, I think here it is subjective which version is better. Since our dev notes don't require a specific style, I think it is fine to not use f-strings everywhere.
|
Also, I'd slightly prefer if the conflicts (#22229 (comment)) are first dealt with before part 2 (etc) are opened. |
9 participants
Rather than using 3 different ways to build/format strings (sometimes all in the same test, i.e
feature_config_args.py), consolidate to using f-strings (3.6+), which are generally more concise / readable, as well as more performant than existing methods.This deals with the
feature_*.py,interface_*.pyandmining_*.pytests.See also: PEP 498