re: #31866 (review)
I see your point about not wanting to use self.nodes[0].binaries, so an alternative might be to remove TestFramework::binary_paths and instead add an array of the same size as the versions array to store the result of a call to [Binaries(self.get_binary_paths(), bin_dir_from_version(v)) for v in versions].
I think I don’t fully understand the suggestion and the problem this is trying to solve. If the goal is to drop the TestFramework.binary_paths variable, that could be done pretty easily with:
0--- a/test/functional/test_framework/test_framework.py
1+++ b/test/functional/test_framework/test_framework.py
2@@ -263,7 +263,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
3 config = configparser.ConfigParser()
4 config.read_file(open(self.options.configfile))
5 self.config = config
6- self.binary_paths = self.get_binary_paths()
7 if self.options.v1transport:
8 self.options.v2transport=False
9
10@@ -307,7 +306,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
11 return paths
12
13 def get_binaries(self, bin_dir=None):
14- return Binaries(self.binary_paths, bin_dir)
15+ return Binaries(self.get_binary_paths(), bin_dir)
16
17 def setup(self):
18 """Call this method to start up the test framework object with options set."""
Or if the problem is that having a BitcoinTestFramework.get_binaries() method is bad because developers might call this method to access binaries in cases where it would be better to use the TestNode.binaries object instead, then maybe better documentation might be helpful:
0--- a/test/functional/test_framework/test_framework.py
1+++ b/test/functional/test_framework/test_framework.py
2@@ -307,6 +307,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
3 return paths
4
5 def get_binaries(self, bin_dir=None):
6+ """Return new Binaries object that can be used to call binaries.
7+
8+ Typically tests will not need to call this method themselves and can use
9+ the TestNode.binaries objects assigned to each test node."""
10 return Binaries(self.binary_paths, bin_dir)
11
12 def setup(self):
I’m sure other changes might be good too, you can let me know what you think might be useful, or if there is another problem with the current approach that I’m not seeing.
