From efdf2b0a7b021e1fe3db2cc44fe093cdb7b4e9c0 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 1 Mar 2026 14:44:53 +0100 Subject: [PATCH 1/4] pyln-testing: add SystemPostgresDbProvider for external postgres Add a new database provider that connects to an existing PostgreSQL instance instead of spawning a new cluster for each test session. Changes: - Add replace_dsn_database() helper to parse and modify postgres DSNs - Extend PostgresDb to accept a base_dsn parameter for DSN-based connections - Add SystemPostgresDbProvider class that reads DSN from TEST_DB_PROVIDER_DSN - Register 'postgres-system' in the providers mapping Usage: export TEST_DB_PROVIDER=postgres-system export TEST_DB_PROVIDER_DSN="postgres://postgres:password@localhost:5432/postgres" pytest tests/ Changelog-Added: pyln-testing: new `postgres-system` db provider connects to existing PostgreSQL instance via TEST_DB_PROVIDER_DSN --- contrib/pyln-testing/pyln/testing/db.py | 76 +++++++++++++++++-- contrib/pyln-testing/pyln/testing/fixtures.py | 3 +- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/db.py b/contrib/pyln-testing/pyln/testing/db.py index 373df50f3742..99790334d244 100644 --- a/contrib/pyln-testing/pyln/testing/db.py +++ b/contrib/pyln-testing/pyln/testing/db.py @@ -1,4 +1,5 @@ from .utils import reserve_unused_port, drop_unused_port +from urllib.parse import urlparse, urlunparse import itertools import logging @@ -14,6 +15,18 @@ from typing import Dict, List, Optional, Union +def replace_dsn_database(dsn: str, dbname: str) -> str: + """Replace the database name in a PostgreSQL DSN. + + Takes a DSN like 'postgres://user:pass@host:port/olddb' and returns + 'postgres://user:pass@host:port/newdb'. + """ + parsed = urlparse(dsn) + # Replace path (database name) with the new one + new_parsed = parsed._replace(path=f"/{dbname}") + return urlunparse(new_parsed) + + class BaseDb(object): def wipe_db(self): raise NotImplementedError("wipe_db method must be implemented by the subclass") @@ -72,19 +85,26 @@ def wipe_db(self): class PostgresDb(BaseDb): - def __init__(self, dbname, port): + def __init__(self, dbname, port, base_dsn=None): self.dbname = dbname self.port = port + self.base_dsn = base_dsn self.provider = None - self.conn = psycopg2.connect("dbname={dbname} user=postgres host=localhost port={port}".format( - dbname=dbname, port=port - )) + if base_dsn: + # Connect using base DSN but with our specific database + self.conn = psycopg2.connect(replace_dsn_database(base_dsn, dbname)) + else: + self.conn = psycopg2.connect("dbname={dbname} user=postgres host=localhost port={port}".format( + dbname=dbname, port=port + )) cur = self.conn.cursor() cur.execute('SELECT 1') cur.close() def get_dsn(self): + if self.base_dsn: + return replace_dsn_database(self.base_dsn, self.dbname) return "postgres://postgres:password@localhost:{port}/{dbname}".format( port=self.port, dbname=self.dbname ) @@ -118,10 +138,15 @@ def stop(self): """Clean up the database. """ self.conn.close() - conn = psycopg2.connect("dbname=postgres user=postgres host=localhost port={self.port}") + if self.base_dsn: + conn = psycopg2.connect(self.base_dsn) + else: + conn = psycopg2.connect(f"dbname=postgres user=postgres host=localhost port={self.port}") + conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) cur = conn.cursor() cur.execute("DROP DATABASE {};".format(self.dbname)) cur.close() + conn.close() def wipe_db(self): cur = self.conn.cursor() @@ -251,3 +276,44 @@ def stop(self): self.proc.wait() shutil.rmtree(self.pgdir) drop_unused_port(self.port) + + +class SystemPostgresDbProvider(object): + """Use an existing system-wide PostgreSQL instance instead of spawning one. + + This provider connects to an existing PostgreSQL server using a DSN from + the TEST_DB_PROVIDER_DSN environment variable. The DSN should point to a + database where the user has CREATE DATABASE privileges (typically the + 'postgres' database with a superuser). + + Example DSN: postgres://postgres:password@localhost:5432/postgres + """ + + def __init__(self, directory): + self.directory = directory + self.dsn = os.environ.get('TEST_DB_PROVIDER_DSN') + if not self.dsn: + raise ValueError( + "SystemPostgresDbProvider requires TEST_DB_PROVIDER_DSN environment variable" + ) + self.conn = None + + def start(self): + self.conn = psycopg2.connect(self.dsn) + self.conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) + logging.info(f"Connected to system PostgreSQL via {self.dsn}") + + def get_db(self, node_directory, testname, node_id): + # Random suffix to avoid collisions on repeated tests + nonce = ''.join(random.choice(string.ascii_lowercase + string.digits) for _ in range(8)) + dbname = "{}_{}_{}".format(testname, node_id, nonce) + + cur = self.conn.cursor() + cur.execute("CREATE DATABASE {};".format(dbname)) + cur.close() + db = PostgresDb(dbname, port=None, base_dsn=self.dsn) + return db + + def stop(self): + if self.conn: + self.conn.close() diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index e04d6d8e7929..23bdef52b8fc 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -1,5 +1,5 @@ from concurrent import futures -from pyln.testing.db import SqliteDbProvider, PostgresDbProvider +from pyln.testing.db import SqliteDbProvider, PostgresDbProvider, SystemPostgresDbProvider from pyln.testing.utils import NodeFactory, BitcoinD, ElementsD, env, LightningNode, TEST_DEBUG, TEST_NETWORK, SLOW_MACHINE, VALGRIND from pyln.client import Millisatoshi from typing import Dict @@ -718,6 +718,7 @@ def checkMemleak(node): providers = { 'sqlite3': SqliteDbProvider, 'postgres': PostgresDbProvider, + 'postgres-system': SystemPostgresDbProvider, } From 31ac100d4ad600034a4e51861ef1177f1bf87642 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 1 Mar 2026 14:44:57 +0100 Subject: [PATCH 2/4] pyproject.toml: add flaky test dependency --- pyproject.toml | 1 + uv.lock | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 76335c2e5d71..052144f13997 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ "pyln-testing", "pyln-proto", "mnemonic>=0.21", + "flaky>=3.8.1", ] package-mode = false [dependency-groups] diff --git a/uv.lock b/uv.lock index 0c75e49f7605..35fa5e23d670 100644 --- a/uv.lock +++ b/uv.lock @@ -457,6 +457,7 @@ version = "0.1.0" source = { virtual = "." } dependencies = [ { name = "cryptography" }, + { name = "flaky" }, { name = "grpcio" }, { name = "grpcio-tools" }, { name = "mako" }, @@ -499,6 +500,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "cryptography", specifier = ">=46" }, + { name = "flaky", specifier = ">=3.8.1" }, { name = "grpcio", specifier = "==1.75.1" }, { name = "grpcio-tools", specifier = "==1.75.1" }, { name = "mako", specifier = ">=1.1.6" }, @@ -820,6 +822,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9f/56/13ab06b4f93ca7cac71078fbe37fcea175d3216f31f85c3168a6bbd0bb9a/flake8-7.3.0-py2.py3-none-any.whl", hash = "sha256:b9696257b9ce8beb888cdbe31cf885c90d31928fe202be0889a7cdafad32f01e", size = 57922, upload-time = "2025-06-20T19:31:34.425Z" }, ] +[[package]] +name = "flaky" +version = "3.8.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/5b/c5/ef69119a01427204ff2db5fc8f98001087bcce719bbb94749dcd7b191365/flaky-3.8.1.tar.gz", hash = "sha256:47204a81ec905f3d5acfbd61daeabcada8f9d4031616d9bcb0618461729699f5", size = 25248, upload-time = "2024-03-12T22:17:59.265Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/7f/b8/b830fc43663246c3f3dd1ae7dca4847b96ed992537e85311e27fa41ac40e/flaky-3.8.1-py2.py3-none-any.whl", hash = "sha256:194ccf4f0d3a22b2de7130f4b62e45e977ac1b5ccad74d4d48f3005dcc38815e", size = 19139, upload-time = "2024-03-12T22:17:51.59Z" }, +] + [[package]] name = "flask" version = "3.1.2" From 7758281169e9683a8f4a4a22d11674d39539d91f Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 1 Mar 2026 14:49:50 +0100 Subject: [PATCH 3/4] ci: use shared postgres service for database tests Replace spawning individual postgres clusters with a shared postgres service container. This avoids the overhead of initdb and cluster startup for each test session. Changes: - Add postgres:16 service to integration and check-downgrade jobs - Switch TEST_DB_PROVIDER from 'postgres' to 'postgres-system' - Add TEST_DB_PROVIDER_DSN environment variable for the service --- .github/workflows/ci.yaml | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9e610d9161cf..8bbcccda8240 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -356,6 +356,20 @@ jobs: runs-on: ubuntu-24.04 needs: - check-compiled-source + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 strategy: fail-fast: false matrix: @@ -365,7 +379,7 @@ jobs: TEST_NETWORK: regtest VALGRIND: 1 - CFG: compile-gcc - TEST_DB_PROVIDER: postgres + TEST_DB_PROVIDER: postgres-system TEST_NETWORK: regtest - CFG: compile-gcc TEST_DB_PROVIDER: sqlite3 @@ -425,6 +439,7 @@ jobs: SLOW_MACHINE: 1 TEST_DEBUG: 1 TEST_DB_PROVIDER: ${{ matrix.TEST_DB_PROVIDER }} + TEST_DB_PROVIDER_DSN: postgres://postgres:postgres@localhost:5432/postgres TEST_NETWORK: ${{ matrix.TEST_NETWORK }} LIGHTNINGD_POSTGRES_NO_VACUUM: 1 VALGRIND: ${{ matrix.VALGRIND }} @@ -528,6 +543,20 @@ jobs: RUST_PROFILE: small # Has to match the one in the compile step needs: - first-integration + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 strategy: fail-fast: false matrix: @@ -537,7 +566,7 @@ jobs: - NAME: postgres CFG: compile-gcc-O3 COMPILER: gcc - TEST_DB_PROVIDER: postgres + TEST_DB_PROVIDER: postgres-system TEST_NETWORK: regtest # And don't forget about elements (like cdecker did when # reworking the CI...) @@ -611,6 +640,7 @@ jobs: SLOW_MACHINE: 1 TEST_DEBUG: 1 TEST_DB_PROVIDER: ${{ matrix.TEST_DB_PROVIDER }} + TEST_DB_PROVIDER_DSN: postgres://postgres:postgres@localhost:5432/postgres TEST_NETWORK: ${{ matrix.TEST_NETWORK }} LIGHTNINGD_POSTGRES_NO_VACUUM: 1 PYTEST_OPTS: ${{ env.PYTEST_OPTS_BASE }} From cdd9a2661ca1fc3d988e4005f8eb5d2a27f01854 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Sun, 1 Mar 2026 14:59:46 +0100 Subject: [PATCH 4/4] pyln-testing: register finalizer to drop databases at test end Register a pytest finalizer in SystemPostgresDbProvider.get_db() to drop each database when the test completes. This ensures cleanup happens for all databases created during a test, even if tests fail or don't go through the normal node stop path. The db_provider fixture now passes the pytest request object to the provider, allowing it to register finalizers via request.addfinalizer(). --- contrib/pyln-testing/pyln/testing/db.py | 15 +++++++++++++++ contrib/pyln-testing/pyln/testing/fixtures.py | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/contrib/pyln-testing/pyln/testing/db.py b/contrib/pyln-testing/pyln/testing/db.py index 99790334d244..ececbe8c54f2 100644 --- a/contrib/pyln-testing/pyln/testing/db.py +++ b/contrib/pyln-testing/pyln/testing/db.py @@ -297,12 +297,22 @@ def __init__(self, directory): "SystemPostgresDbProvider requires TEST_DB_PROVIDER_DSN environment variable" ) self.conn = None + self.request = None # Set by fixture to allow registering finalizers def start(self): self.conn = psycopg2.connect(self.dsn) self.conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) logging.info(f"Connected to system PostgreSQL via {self.dsn}") + def _drop_database(self, dbname): + """Drop a database, used as finalizer.""" + try: + cur = self.conn.cursor() + cur.execute("DROP DATABASE IF EXISTS {};".format(dbname)) + cur.close() + except Exception as e: + logging.warning(f"Failed to drop database {dbname}: {e}") + def get_db(self, node_directory, testname, node_id): # Random suffix to avoid collisions on repeated tests nonce = ''.join(random.choice(string.ascii_lowercase + string.digits) for _ in range(8)) @@ -311,6 +321,11 @@ def get_db(self, node_directory, testname, node_id): cur = self.conn.cursor() cur.execute("CREATE DATABASE {};".format(dbname)) cur.close() + + # Register finalizer to drop this database at end of test + if self.request is not None: + self.request.addfinalizer(lambda db=dbname: self._drop_database(db)) + db = PostgresDb(dbname, port=None, base_dsn=self.dsn) return db diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index 23bdef52b8fc..b77cbaec26bf 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -723,8 +723,9 @@ def checkMemleak(node): @pytest.fixture -def db_provider(test_base_dir): +def db_provider(test_base_dir, request): provider = providers[os.getenv('TEST_DB_PROVIDER', 'sqlite3')](test_base_dir) + provider.request = request # Allow provider to register finalizers provider.start() yield provider provider.stop()