tests: do not leak file descriptors

Changelog-None

Author: whitslack

contrib/pyln-testing/pyln/testing/utils.py

@@ -940,7 +940,7 @@ def grpc(self):
 
         import grpc
         p = Path(self.daemon.lightning_dir) / TEST_NETWORK
-        cert, key, ca = [f.open('rb').read() for f in [
+        cert, key, ca = [f.read_bytes() for f in [
             p / 'client.pem',
             p / 'client-key.pem',
             p / "ca.pem"]]
@@ -1691,16 +1691,16 @@ def get_node(self, node_id=None, options=None, dbfile=None,
         self.nodes.append(node)
         self.reserved_ports.append(port)
         if dbfile:
-            out = open(os.path.join(node.daemon.lightning_dir, TEST_NETWORK,
-                                    'lightningd.sqlite3'), 'xb')
-            with lzma.open(os.path.join('tests/data', dbfile), 'rb') as f:
-                out.write(f.read())
+            with open(os.path.join(node.daemon.lightning_dir, TEST_NETWORK,
+                                   'lightningd.sqlite3'), 'xb') as out:
+                with lzma.open(os.path.join('tests/data', dbfile), 'rb') as f:
+                    out.write(f.read())
 
         if bkpr_dbfile:
-            out = open(os.path.join(node.daemon.lightning_dir, TEST_NETWORK,
-                                    'accounts.sqlite3'), 'xb')
-            with lzma.open(os.path.join('tests/data', bkpr_dbfile), 'rb') as f:
-                out.write(f.read())
+            with open(os.path.join(node.daemon.lightning_dir, TEST_NETWORK,
+                                   'accounts.sqlite3'), 'xb') as out:
+                with lzma.open(os.path.join('tests/data', bkpr_dbfile), 'rb') as f:
+                    out.write(f.read())
 
         if gossip_store_file:
             shutil.copy(gossip_store_file, os.path.join(node.daemon.lightning_dir, TEST_NETWORK,

tests/fixtures.py

@@ -58,7 +58,8 @@ class CompatLevel(object):
     """
     def __init__(self):
         makefile = os.path.join(os.path.dirname(__file__), "..", "Makefile")
-        lines = [l for l in open(makefile, 'r') if l.startswith('COMPAT_CFLAGS')]
+        with open(makefile, 'r') as f:
+            lines = [l for l in f if l.startswith('COMPAT_CFLAGS')]
         assert(len(lines) == 1)
         line = lines[0]
         flags = re.findall(r'COMPAT_V([0-9]+)=1', line)

tests/test_bookkeeper.py

@@ -690,15 +690,15 @@ def test_bookkeeping_descriptions(node_factory, bitcoind, chainparams):
     l1.rpc.bkpr_dumpincomecsv('koinly', 'koinly.csv')
     l2.rpc.bkpr_dumpincomecsv('koinly', 'koinly.csv')
     koinly_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'koinly.csv')
-    l1_koinly_csv = open(koinly_path, 'rb').read()
+    l1_koinly_csv = Path(koinly_path).read_bytes()
     bolt11_exp = bytes('invoice,"test \'bolt11\' description, 🥰🪢",', 'utf-8')
     bolt12_exp = bytes('invoice,"test \'bolt12\' description, 🥰🪢",', 'utf-8')
 
     assert l1_koinly_csv.find(bolt11_exp) >= 0
     assert l1_koinly_csv.find(bolt12_exp) >= 0
 
     koinly_path = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, 'koinly.csv')
-    l2_koinly_csv = open(koinly_path, 'rb').read()
+    l2_koinly_csv = Path(koinly_path).read_bytes()
     assert l2_koinly_csv.find(bolt11_exp) >= 0
     assert l2_koinly_csv.find(bolt12_exp) >= 0
 

tests/test_cln_rs.py

@@ -129,9 +129,9 @@ def test_grpc_connect(node_factory):
     key_path = p / "client-key.pem"
     ca_cert_path = p / "ca.pem"
     creds = grpc.ssl_channel_credentials(
-        root_certificates=ca_cert_path.open('rb').read(),
-        private_key=key_path.open('rb').read(),
-        certificate_chain=cert_path.open('rb').read()
+        root_certificates=ca_cert_path.read_bytes(),
+        private_key=key_path.read_bytes(),
+        certificate_chain=cert_path.read_bytes()
     )
 
     wait_for_grpc_start(l1)
@@ -196,15 +196,15 @@ def test_grpc_generate_certificate(node_factory):
     assert [f.exists() for f in files] == [True] * len(files)
 
     # The files exist, restarting should not change them
-    contents = [f.open().read() for f in files]
+    contents = [f.read_bytes() for f in files]
     l1.restart()
-    assert contents == [f.open().read() for f in files]
+    assert contents == [f.read_bytes() for f in files]
 
     # Now we delete the last file, we should regenerate it as well as its key
     files[-1].unlink()
     l1.restart()
-    assert contents[-2] != files[-2].open().read()
-    assert contents[-1] != files[-1].open().read()
+    assert contents[-2] != files[-2].read_bytes()
+    assert contents[-1] != files[-1].read_bytes()
 
     keys = [f for f in files if f.name.endswith('-key.pem')]
     modes = [f.stat().st_mode for f in keys]
@@ -241,7 +241,7 @@ def test_grpc_wrong_auth(node_factory):
 
     def connect(node):
         p = Path(node.daemon.lightning_dir) / TEST_NETWORK
-        cert, key, ca = [f.open('rb').read() for f in [
+        cert, key, ca = [f.read_bytes() for f in [
             p / 'client.pem',
             p / 'client-key.pem',
             p / "ca.pem"]]

tests/test_clnrest.py

@@ -105,22 +105,22 @@ def test_generate_certificate(node_factory):
     wait_for(lambda: [f.exists() for f in files] == [True] * len(files))
 
     # the files exist, restarting should not change them
-    contents = [f.open().read() for f in files]
+    contents = [f.read_bytes() for f in files]
     l1.restart()
-    assert contents == [f.open().read() for f in files]
+    assert contents == [f.read_bytes() for f in files]
 
     # remove client.pem file, so all certs are regenerated at restart
     files[2].unlink()
     l1.restart()
     wait_for(lambda: [f.exists() for f in files] == [True] * len(files))
-    contents_1 = [f.open().read() for f in files]
+    contents_1 = [f.read_bytes() for f in files]
     assert [c[0] != c[1] for c in zip(contents, contents_1)] == [True] * len(files)
 
     # remove client-key.pem file, so all certs are regenerated at restart
     files[3].unlink()
     l1.restart()
     wait_for(lambda: [f.exists() for f in files] == [True] * len(files))
-    contents_2 = [f.open().read() for f in files]
+    contents_2 = [f.read_bytes() for f in files]
     assert [c[0] != c[1] for c in zip(contents, contents_2)] == [True] * len(files)
 
 

tests/test_connection.py

@@ -1,6 +1,7 @@
 from fixtures import *  # noqa: F401,F403
 from fixtures import TEST_NETWORK
 from decimal import Decimal
+from pathlib import Path
 from pyln.client import RpcError, Millisatoshi
 import pyln.proto.wire as wire
 from utils import (
@@ -3019,7 +3020,7 @@ def test_dataloss_protection(node_factory, bitcoind):
 
     # Save copy of the db.
     dbpath = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "lightningd.sqlite3")
-    orig_db = open(dbpath, "rb").read()
+    orig_db = Path(dbpath).read_bytes()
     l2.start()
 
     # l1 should have sent WIRE_CHANNEL_REESTABLISH with extra fields.
@@ -3063,7 +3064,7 @@ def test_dataloss_protection(node_factory, bitcoind):
     # Now, move l2 back in time.
     l2.stop()
     # Overwrite with OLD db.
-    open(dbpath, "wb").write(orig_db)
+    Path(dbpath).write_bytes(orig_db)
     l2.start()
 
     # l2 should freak out!
@@ -3118,7 +3119,7 @@ def test_dataloss_protection_no_broadcast(node_factory, bitcoind):
 
     # Save copy of the db.
     dbpath = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "lightningd.sqlite3")
-    orig_db = open(dbpath, "rb").read()
+    orig_db = Path(dbpath).read_bytes()
     l2.start()
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
@@ -3133,9 +3134,9 @@ def test_dataloss_protection_no_broadcast(node_factory, bitcoind):
     # Now, move l2 back in time.
     l2.stop()
     # Save new db
-    new_db = open(dbpath, "rb").read()
+    new_db = Path(dbpath).read_bytes()
     # Overwrite with OLD db.
-    open(dbpath, "wb").write(orig_db)
+    Path(dbpath).write_bytes(orig_db)
     l2.start()
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
@@ -3149,7 +3150,7 @@ def test_dataloss_protection_no_broadcast(node_factory, bitcoind):
 
     # fix up l2.
     l2.stop()
-    open(dbpath, "wb").write(new_db)
+    Path(dbpath).write_bytes(new_db)
     l2.start()
 
     # All is forgiven

tests/test_misc.py

@@ -3,6 +3,7 @@
 from decimal import Decimal
 from fixtures import *  # noqa: F401,F403
 from fixtures import LightningNode, TEST_NETWORK
+from pathlib import Path
 from pyln.client import RpcError, Millisatoshi
 from threading import Event
 from pyln.testing.utils import (
@@ -1899,11 +1900,13 @@ def test_logging(node_factory):
     wait_for(lambda: os.path.exists(logpath_moved))
     wait_for(lambda: os.path.exists(logpath))
 
-    log1 = open(logpath_moved).readlines()
+    with open(logpath_moved) as f:
+        log1 = f.readlines()
     assert log1[-1].endswith("Ending log due to SIGHUP\n")
 
     def check_new_log():
-        log2 = open(logpath).readlines()
+        with open(logpath) as f:
+            log2 = f.readlines()
         return len(log2) > 0 and log2[0].endswith("Started log due to SIGHUP\n")
     wait_for(check_new_log)
 
@@ -3106,7 +3109,7 @@ def test_recover_plugin(node_factory, bitcoind):
 
     # Save copy of the db.
     dbpath = os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "lightningd.sqlite3")
-    orig_db = open(dbpath, "rb").read()
+    orig_db = Path(dbpath).read_bytes()
 
     l2.start()
 
@@ -3121,7 +3124,7 @@ def test_recover_plugin(node_factory, bitcoind):
     l2.stop()
 
     # Overwrite with OLD db.
-    open(dbpath, "wb").write(orig_db)
+    Path(dbpath).write_bytes(orig_db)
 
     l2.start()
 
@@ -4917,45 +4920,46 @@ def test_tracing(node_factory):
     traces = set()
     suspended = set()
     for fname in glob.glob(f"{trace_fnamebase}.*"):
-        for linenum, l in enumerate(open(fname, "rt").readlines(), 1):
-            # In case an assertion fails
-            print(f"Parsing {fname}:{linenum}")
-            parts = l.split(maxsplit=2)
-            cmd = parts[0]
-            spanid = parts[1]
-            if cmd == 'span_emit':
-                assert spanid in traces
-                assert spanid not in suspended
-                # Should be valid JSON
-                res = json.loads(parts[2])
-
-                # This is an array for some reason
-                assert len(res) == 1
-                res = res[0]
-                assert res['id'] == spanid
-                assert res['localEndpoint'] == {"serviceName": "lightningd"}
-                expected_keys = ['id', 'name', 'timestamp', 'duration', 'tags', 'traceId', 'localEndpoint']
-                if 'parentId' in res:
-                    assert res['parentId'] in traces
-                    expected_keys.append('parentId')
-                assert set(res.keys()) == set(expected_keys)
-                traces.remove(spanid)
-            elif cmd == 'span_end':
-                assert spanid in traces
-            elif cmd == 'span_start':
-                assert spanid not in traces
-                traces.add(spanid)
-            elif cmd == 'span_suspend':
-                assert spanid in traces
-                assert spanid not in suspended
-                suspended.add(spanid)
-            elif cmd == 'span_resume':
-                assert spanid in traces
-                suspended.remove(spanid)
-            elif cmd == 'destroying':
-                pass
-            else:
-                assert False, "Unknown trace line"
+        with open(fname, "rt") as f:
+            for linenum, l in enumerate(f.readlines(), 1):
+                # In case an assertion fails
+                print(f"Parsing {fname}:{linenum}")
+                parts = l.split(maxsplit=2)
+                cmd = parts[0]
+                spanid = parts[1]
+                if cmd == 'span_emit':
+                    assert spanid in traces
+                    assert spanid not in suspended
+                    # Should be valid JSON
+                    res = json.loads(parts[2])
+
+                    # This is an array for some reason
+                    assert len(res) == 1
+                    res = res[0]
+                    assert res['id'] == spanid
+                    assert res['localEndpoint'] == {"serviceName": "lightningd"}
+                    expected_keys = ['id', 'name', 'timestamp', 'duration', 'tags', 'traceId', 'localEndpoint']
+                    if 'parentId' in res:
+                        assert res['parentId'] in traces
+                        expected_keys.append('parentId')
+                    assert set(res.keys()) == set(expected_keys)
+                    traces.remove(spanid)
+                elif cmd == 'span_end':
+                    assert spanid in traces
+                elif cmd == 'span_start':
+                    assert spanid not in traces
+                    traces.add(spanid)
+                elif cmd == 'span_suspend':
+                    assert spanid in traces
+                    assert spanid not in suspended
+                    suspended.add(spanid)
+                elif cmd == 'span_resume':
+                    assert spanid in traces
+                    suspended.remove(spanid)
+                elif cmd == 'destroying':
+                    pass
+                else:
+                    assert False, "Unknown trace line"
 
         assert suspended == set()
         assert traces == set()
@@ -4969,20 +4973,21 @@ def test_tracing(node_factory):
 
     # The parent should set all the trace ids and span ids
     for fname in glob.glob(f"{trace_fnamebase}.*"):
-        for linenum, l in enumerate(open(fname, "rt").readlines(), 1):
-            # In case an assertion fails
-            print(f"Parsing {fname}:{linenum}")
-            parts = l.split(maxsplit=2)
-            cmd = parts[0]
-            spanid = parts[1]
-            # This span doesn't actually appear anywhere
-            assert spanid != '0123456789abcdef'
-            if cmd == 'span_emit':
-                # Should be valid JSON
-                res = json.loads(parts[2])
-                assert res[0]['traceId'] == '00112233445566778899aabbccddeeff'
-                # Everyone has a parent!
-                assert 'parentId' in res[0]
+        with open(fname, "rt") as f:
+            for linenum, l in enumerate(f.readlines(), 1):
+                # In case an assertion fails
+                print(f"Parsing {fname}:{linenum}")
+                parts = l.split(maxsplit=2)
+                cmd = parts[0]
+                spanid = parts[1]
+                # This span doesn't actually appear anywhere
+                assert spanid != '0123456789abcdef'
+                if cmd == 'span_emit':
+                    # Should be valid JSON
+                    res = json.loads(parts[2])
+                    assert res[0]['traceId'] == '00112233445566778899aabbccddeeff'
+                    # Everyone has a parent!
+                    assert 'parentId' in res[0]
 
 
 def test_zero_locktime_blocks(node_factory, bitcoind):

tests/test_pay.py

@@ -1241,7 +1241,7 @@ def test_forward(node_factory, bitcoind):
     l1.rpc.bkpr_dumpincomecsv('koinly', 'koinly.csv')
 
     koinly_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'koinly.csv')
-    koinly_csv = open(koinly_path, 'rb').read()
+    koinly_csv = Path(koinly_path).read_bytes()
     expected_line = r'0.00100000000,.*,,,0.00000001001,.*,invoice'
     assert only_one(re.findall(expected_line, str(koinly_csv)))