From a9ff3cb039113b205a4d28c774834b728e8a0f9a Mon Sep 17 00:00:00 2001 From: daywalker90 <8257956+daywalker90@users.noreply.github.com> Date: Wed, 15 May 2024 18:53:27 +0200 Subject: [PATCH] pytests: use reserve_unused_port() everywhere Changelog-None --- contrib/pyln-testing/pyln/testing/db.py | 5 +++-- contrib/pyln-testing/pyln/testing/utils.py | 5 +++++ tests/test_cln_rs.py | 13 ++++++------ tests/test_clnrest.py | 23 +++++++++++----------- tests/test_connection.py | 11 +++++------ tests/test_gossip.py | 8 ++++---- tests/test_misc.py | 5 ++--- 7 files changed, 36 insertions(+), 34 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/db.py b/contrib/pyln-testing/pyln/testing/db.py index 96abc7a5c..7930e6a37 100644 --- a/contrib/pyln-testing/pyln/testing/db.py +++ b/contrib/pyln-testing/pyln/testing/db.py @@ -1,4 +1,4 @@ -from ephemeral_port_reserve import reserve # type: ignore +from .utils import reserve_unused_port, drop_unused_port import itertools import logging @@ -192,7 +192,7 @@ class PostgresDbProvider(object): with open(conffile, 'a') as f: f.write('max_connections = 1000\nshared_buffers = 240MB\n') - self.port = reserve() + self.port = reserve_unused_port() self.proc = subprocess.Popen([ postgres, '-k', '/tmp/', # So we don't use /var/lib/... @@ -240,3 +240,4 @@ class PostgresDbProvider(object): self.proc.send_signal(signal.SIGINT) self.proc.wait() shutil.rmtree(self.pgdir) + drop_unused_port(self.port) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 19d391f21..91f9d5294 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -1693,6 +1693,11 @@ class NodeFactory(object): self.join_nodes(nodes, fundchannel, fundamount, wait_for_announce, announce_channels) return nodes + def get_unused_port(self): + port = reserve_unused_port() + self.reserved_ports.append(port) + return port + def killall(self, expected_successes): """Returns true if every node we expected to succeed actually succeeded""" unexpected_fail = False diff --git a/tests/test_cln_rs.py b/tests/test_cln_rs.py index 83d91e8cd..51f89ecb1 100644 --- a/tests/test_cln_rs.py +++ b/tests/test_cln_rs.py @@ -1,4 +1,3 @@ -from ephemeral_port_reserve import reserve from fixtures import * # noqa: F401,F403 from pathlib import Path from pyln import grpc as clnpb @@ -94,7 +93,7 @@ def test_grpc_connect(node_factory): """Attempts to connect to the grpc interface and call getinfo""" # These only exist if we have rust! - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1 = node_factory.get_node(options={"grpc-port": str(grpc_port)}) p = Path(l1.daemon.lightning_dir) / TEST_NETWORK @@ -150,7 +149,7 @@ def test_grpc_generate_certificate(node_factory): - If we have certs, we they should just get loaded - If we delete one cert or its key it should get regenerated. """ - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1 = node_factory.get_node(options={ "grpc-port": str(grpc_port), }, start=False) @@ -210,7 +209,7 @@ def test_grpc_wrong_auth(node_factory): """ # These only exist if we have rust! - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1, l2 = node_factory.get_nodes(2, opts={ "start": False, "grpc-port": str(grpc_port), @@ -297,7 +296,7 @@ def test_grpc_keysend_routehint(bitcoind, node_factory): recipient. """ - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1, l2, l3 = node_factory.line_graph( 3, opts=[ @@ -341,7 +340,7 @@ def test_grpc_keysend_routehint(bitcoind, node_factory): def test_grpc_listpeerchannels(bitcoind, node_factory): """ Check that conversions of this rather complex type work. """ - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1, l2 = node_factory.line_graph( 2, opts=[ @@ -371,7 +370,7 @@ def test_grpc_listpeerchannels(bitcoind, node_factory): def test_grpc_decode(node_factory): - grpc_port = reserve() + grpc_port = node_factory.get_unused_port() l1 = node_factory.get_node(options={'grpc-port': str(grpc_port)}) inv = l1.grpc.Invoice(clnpb.InvoiceRequest( amount_msat=clnpb.AmountOrAny(any=True), diff --git a/tests/test_clnrest.py b/tests/test_clnrest.py index 4a2a240f6..6a1c6a73f 100644 --- a/tests/test_clnrest.py +++ b/tests/test_clnrest.py @@ -1,4 +1,3 @@ -from ephemeral_port_reserve import reserve from fixtures import * # noqa: F401,F403 from pyln.testing.utils import env, TEST_NETWORK from pyln.client import Millisatoshi @@ -35,7 +34,7 @@ def test_clnrest_no_auto_start(node_factory): def test_clnrest_self_signed_certificates(node_factory): """Test that self-signed certificates have `clnrest-host` IP in Subject Alternative Name.""" - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_host = '127.0.0.1' base_url = f'https://{rest_host}:{rest_port}' l1 = node_factory.get_node(options={'disable-plugin': 'cln-grpc', @@ -58,8 +57,8 @@ def test_clnrest_uses_grpc_plugin_certificates(node_factory): - clnrest-protocol: https """ rest_host = 'localhost' - grpc_port = str(reserve()) - rest_port = str(reserve()) + grpc_port = str(node_factory.get_unused_port()) + rest_port = str(node_factory.get_unused_port()) l1 = node_factory.get_node(options={'grpc-port': grpc_port, 'clnrest-host': rest_host, 'clnrest-port': rest_port}) base_url = f'https://{rest_host}:{rest_port}' # This might happen really early! @@ -75,7 +74,7 @@ def test_clnrest_uses_grpc_plugin_certificates(node_factory): def test_clnrest_generate_certificate(node_factory): """Test whether we correctly generate the certificates.""" # when `clnrest-protocol` is `http`, certs are not generated at `clnrest-certs` path - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_protocol = 'http' rest_certs = node_factory.directory + '/clnrest-certs' l1 = node_factory.get_node(options={'clnrest-port': rest_port, @@ -85,7 +84,7 @@ def test_clnrest_generate_certificate(node_factory): assert not Path(rest_certs).exists() # node l1 not started - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_certs = node_factory.directory + '/clnrest-certs' l1 = node_factory.get_node(options={'clnrest-port': rest_port, 'clnrest-certs': rest_certs}, start=False) @@ -130,7 +129,7 @@ def start_node_with_clnrest(node_factory): - the node, - the base url and - the certificate authority path used for the self-signed certificates.""" - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_certs = node_factory.directory + '/clnrest-certs' l1 = node_factory.get_node(options={'clnrest-port': rest_port, 'clnrest-certs': rest_certs}) base_url = 'https://127.0.0.1:' + rest_port @@ -378,7 +377,7 @@ def test_clnrest_websocket_rune_no_listnotifications(node_factory): def test_clnrest_numeric_msat_notification(node_factory): """Test that msat fields are integers in notifications also.""" # start a node with clnrest - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) base_url = 'http://127.0.0.1:' + rest_port l1, l2 = node_factory.get_nodes(2, opts=[{}, {'clnrest-port': rest_port, 'clnrest-protocol': 'http'}]) node_factory.join_nodes([l1, l2], wait_for_announce=True) @@ -405,14 +404,14 @@ def test_clnrest_options(node_factory): assert l1.daemon.is_in_log(f'plugin-clnrest: Killing plugin: disabled itself at init: `clnrest-port` {rest_port}, should be a valid available port between 1024 and 65535.') # with invalid protocol - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_protocol = 'htttps' l1 = node_factory.get_node(options={'clnrest-port': rest_port, 'clnrest-protocol': rest_protocol}) assert l1.daemon.is_in_log(r'plugin-clnrest: Killing plugin: disabled itself at init: `clnrest-protocol` can either be http or https.') # with invalid host - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_host = '127.0.0.12.15' l1 = node_factory.get_node(options={'clnrest-port': rest_port, 'clnrest-host': rest_host}) @@ -434,7 +433,7 @@ def test_clnrest_http_headers(node_factory): l1.daemon.wait_for_log(f'plugin-clnrest: REST server running at {base_url}') # Custom values for `clnrest-csp` and `clnrest-cors-origins` options - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_certs = node_factory.directory + '/clnrest-certs' l2 = node_factory.get_node(options={ 'clnrest-port': rest_port, @@ -461,7 +460,7 @@ def test_clnrest_http_headers(node_factory): def test_clnrest_old_params(node_factory): """Test that we handle the v23.08-style parameters""" - rest_port = str(reserve()) + rest_port = str(node_factory.get_unused_port()) rest_host = '127.0.0.1' base_url = f'https://{rest_host}:{rest_port}' l1 = node_factory.get_node(options={'rest-port': rest_port, diff --git a/tests/test_connection.py b/tests/test_connection.py index 42313cca7..3f8d2a37e 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1,7 +1,6 @@ from fixtures import * # noqa: F401,F403 from fixtures import TEST_NETWORK from decimal import Decimal -from ephemeral_port_reserve import reserve # type: ignore from pyln.client import RpcError, Millisatoshi import pyln.proto.wire as wire from utils import ( @@ -4099,8 +4098,8 @@ def test_old_feerate(node_factory): def test_websocket(node_factory): - ws_port = reserve() - port = reserve() + ws_port = node_factory.get_unused_port() + port = node_factory.get_unused_port() l1, l2 = node_factory.line_graph(2, opts=[{'addr': ':' + str(port), 'bind-addr': 'ws:127.0.0.1: ' + str(ws_port), @@ -4524,9 +4523,9 @@ def test_last_stable_connection(node_factory): def test_wss_proxy(node_factory): - wss_port = reserve() - ws_port = reserve() - port = reserve() + wss_port = node_factory.get_unused_port() + ws_port = node_factory.get_unused_port() + port = node_factory.get_unused_port() wss_proxy_certs = node_factory.directory + '/wss-proxy-certs' l1 = node_factory.get_node(options={'addr': ':' + str(port), 'bind-addr': 'ws:127.0.0.1:' + str(ws_port), diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 1db881e73..42924fd32 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1,5 +1,4 @@ from collections import Counter -from ephemeral_port_reserve import reserve from fixtures import * # noqa: F401,F403 from fixtures import TEST_NETWORK from pyln.client import RpcError, Millisatoshi @@ -1731,7 +1730,6 @@ def test_static_tor_onions(node_factory): torips = '127.0.0.1:9051' torport = 9050 torserviceport = 9051 - portA, portB = reserve(), reserve() if not check_socket(format(torip), torserviceport): return @@ -1739,10 +1737,12 @@ def test_static_tor_onions(node_factory): if not check_socket(format(torip), torport): return + portA = node_factory.get_unused_port() l1 = node_factory.get_node(may_fail=True, options={ 'bind-addr': '127.0.0.1:{}'.format(portA), 'addr': ['statictor:{}'.format(torips)] }) + portB = node_factory.get_unused_port() l2 = node_factory.get_node(may_fail=True, options={ 'bind-addr': '127.0.0.1:{}'.format(portB), 'addr': ['statictor:{}/torblob=11234567890123456789012345678901/torport={}'.format(torips, 9736)] @@ -1772,9 +1772,9 @@ def test_tor_port_onions(node_factory): if not check_socket(torip, torport): return - portA, portB = reserve(), reserve() - + portA = node_factory.get_unused_port() l1 = node_factory.get_node(may_fail=True, options={'bind-addr': '127.0.0.1:{}'.format(portA), 'addr': ['statictor:{}/torport=45321'.format(torips)]}) + portB = node_factory.get_unused_port() l2 = node_factory.get_node(may_fail=True, options={'bind-addr': '127.0.0.1:{}'.format(portB), 'addr': ['statictor:{}/torport=45321/torblob=11234567890123456789012345678901'.format(torips)]}) assert l1.daemon.is_in_log('45321,127.0.0.1:{}'.format(l1.port)) diff --git a/tests/test_misc.py b/tests/test_misc.py index 328c2a82f..8287d9dd7 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -11,7 +11,6 @@ from pyln.testing.utils import ( from utils import ( account_balance, scriptpubkey_addr, check_coin_moves, first_scid ) -from ephemeral_port_reserve import reserve import json import os @@ -1680,7 +1679,7 @@ def test_reserve_enforcement(node_factory, executor): def test_ipv4_and_ipv6(node_factory): """Test we can bind to both IPv4 and IPv6 addresses (if supported)""" - port = reserve() + port = node_factory.get_unused_port() l1 = node_factory.get_node(options={'addr': ':{}'.format(port)}) bind = l1.rpc.getinfo()['binding'] @@ -2673,7 +2672,7 @@ def test_unix_socket_path_length(node_factory, bitcoind, directory, executor, db db = db_provider.get_db(lightning_dir, "test_unix_socket_path_length", 1) db.provider = db_provider - l1 = LightningNode(1, lightning_dir, bitcoind, executor, VALGRIND, db=db, port=reserve()) + l1 = LightningNode(1, lightning_dir, bitcoind, executor, VALGRIND, db=db, port=node_factory.get_unused_port()) # `LightningNode.start()` internally calls `LightningRpc.getinfo()` which # exercises the socket logic, and raises an issue if it fails.