This commit removes the dependency on the global function p2p_port when connecting nodes to each other. This is a first
step in removing PortSeed.n. We accomplish this by attaching the relevant information to the TestNode object and passing that in directly in to connect_nodes and disconnect_nodes.
Details
./test/functional/test_runner.py
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- deglobalize-port
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 4189 Build 6444: Bitcoin ABC Buildbot (legacy) Build 6443: arc lint + arc unit
Event Timeline
test/functional/test_framework/test_framework.py | ||
---|---|---|
295 | Move these to where they can get the seed. | |
test/functional/test_framework/test_node.py | ||
43 | Rename as it's not just for rpc | |
46 | Store state here in case other functions need it (they will). | |
test/functional/test_framework/util.py | ||
273 | The point of this diff is to remove this guy. | |
299 | This was simplified so there isn't a need to construct a string to specify the port. The port now needs to be passed in because rpc_port is attached to BitcoinTestFramework | |
310 | Require the ports to be passed in as we don't have access to a BitcoinTestFramework instance, nor do I think it makes sense to pass one in. | |
364 | This change could probably be separated out. I made it because I wasn't sure if something had gotten messed up elsewhere with string formatting. | |
376 | Switch from node_num to node. We store the port there so we can disconnect/connect nodes. An Alternative would be to take a BitcoinTestFramework, or make this a method there. I think this is the best option, but requires a lot of tests to change. |
test/functional/test_framework/util.py | ||
---|---|---|
364 | Yes, please separate it out. This diff is big enough as it is. |
test/functional/test_framework/test_node.py | ||
---|---|---|
48 ↗ | (On Diff #6224) | It's very unclear to me why there refactoring about the RPC port happens in the middle of a giant refactoring of connect_node, disconnect_node and connect_bi. It seems like this whole thing should be 4 diffs. |
test/functional/test_framework/util.py | ||
406 ↗ | (On Diff #6224) | It does look like this could be de-globalized easily without changing half the tests. The new API is arguably better, but changing that API does nothing for the current diff but make it bigger. |
test/functional/test_framework/util.py | ||
---|---|---|
406 ↗ | (On Diff #6224) | If you look carefully you will see that connect_nodes does not provide the nodes object, or the node object for the 2nd paramater. Therefor there's no place to fetch the p2p port from. I don't see how this can be fixed without changing the API. If you don't agree, please give an example of what I could do, because I don't understand how. |