From afc1a9f4acc77ced89b6be87ca4765edbfb70c9b Mon Sep 17 00:00:00 2001 From: MacroFake Date: Wed, 10 Aug 2022 19:22:05 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#25811: doc: test: suggest multi-line imports in functional test style guide 4edc6893825fd8c45c53c81c73a6a7801e1b458c doc: test: suggest multi-line imports in functional test style guide (Sebastian Falbesoner) Pull request description: As long as I remember contributing to functional tests (~2-3 years), it was always kind of an unwritten rule that multi-line imports are preferred over single-line imports in order to reduce the possibility of potential merge conflicts -- at least if more than one symbol from a module is imported. This PR adds this rule to the style guide and adapts the example test accordingly. (Inspired by https://github.com/bitcoin/bitcoin/pull/25792#discussion_r941180819). ACKs for top commit: kouloumos: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c fanquake: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c Tree-SHA512: c7b6ff62f601f4e57cc8334d291662987d6737ebca8d81c063280409f4412302172f1404ec16afc9a13007bcdba55bdab66b9b80363287e287888929cb386584 --- test/functional/README.md | 4 +++- test/functional/example_test.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/test/functional/README.md b/test/functional/README.md index e114298a33..ca3e92a54d 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -28,7 +28,9 @@ don't have test cases for. could lead to bugs and issues in the test code. - Use [type hints](https://docs.python.org/3/library/typing.html) in your code to improve code readability and to detect possible bugs earlier. -- Avoid wildcard imports +- Avoid wildcard imports. +- If more than one name from a module is needed, use lexicographically sorted multi-line imports + in order to reduce the possibility of potential merge conflicts. - Use a module-level docstring to describe what the test is testing, and how it is testing it. - When subclassing the BitcoinTestFramework, place overrides for the diff --git a/test/functional/example_test.py b/test/functional/example_test.py index 76a0428242..4c576f72b9 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -14,8 +14,15 @@ is testing and *how* it's being tested from collections import defaultdict # Avoid wildcard * imports -from test_framework.blocktools import (create_block, create_coinbase) -from test_framework.messages import CInv, MSG_BLOCK +# Use lexicographically sorted multi-line imports +from test_framework.blocktools import ( + create_block, + create_coinbase, +) +from test_framework.messages import ( + CInv, + MSG_BLOCK, +) from test_framework.p2p import ( P2PInterface, msg_block,