doc: test: suggest multi-line imports in functional test style guide #25811

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 #25792 (review)).

ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c

Concept ACK

ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c

ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c

Thanks for pointing this out, I hadn't considered merge conflicts in my comment but I'm honoured to be an inspiration. Reducing maintenance burden seems a good reason for these updated style guidelines.

We would still have merge conflicts for initially single name imports that then become multiple names, as showcased in the below script. Would it then be better to apply a uniform rule and always have imports be multiline? Either way, I think the rest of example_test.py should be updated to uniformly reflect whatever we end up with, since we now mix both.

from collections import defaultdict
...
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
    assert_equal,
)

<details> <summary>Example merge conflict for importing single -> multiple names</summary>

mkdir mergeconflict && cd mergeconflict && git init && git checkout -b master
echo “from typing import List” > main.py && git add main.py && git commit -m “init”
git checkout -b a
echo "from typing import (\n    List,\n    Any,\n)" > main.py && git commit main.py -m "import any"
git checkout master && git checkout -b b
echo "from typing import (\n    Tuple,\n    List,\n)" > main.py && git commit main.py -m "import tuple"
git checkout master && git merge a
git checkout b && git rebase master

</details

Concept ACK. AFAICT this is preferred and for this reason, so might as well document it.

ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c

We would still have merge conflicts for initially single name imports that then become multiple names, as showcased in the below script. Would it then be better to apply a uniform rule and always have imports be multiline? Either way, I think the rest of example_test.py should be updated to uniformly reflect whatever we end up with, since we now mix both.

Thanks for testing out the merge-conflict-scenario for single-/multi-line imports! I'd personally be fine with applying an uniform rule as it's more clear. Though one could counter-argue that there are some modules from where in almost all cases we only import one symbol, and it's probably not worth it to add two extra lines, e.g. the following, being present in every single functional test: from test_framework.test_framework import BitcoinTestFramework

Anyways, if you (or anyone else) plans a follow-up, feel free to tag me as reviewer.