From 98a19df413bce415446fb1679b064bbaa1ae6dbb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 7 Jan 2026 09:43:11 +1030 Subject: [PATCH] pytest: don't run tests marked slow_test at all if VALGRIND and SLOW_MACHINE. We used to just run these without valgrind, but we already run them in CI (which sets SLOW_MACHINE) without valgrind, so this just doubles up. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/utils.py | 14 +++++--------- tests/conftest.py | 4 +++- tests/test_closing.py | 4 ++-- tests/test_connection.py | 16 ++++++++-------- tests/test_plugin.py | 2 +- tests/test_reckless.py | 4 ++-- 6 files changed, 21 insertions(+), 23 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 8607cd9bd..732bd7b49 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -841,7 +841,7 @@ class PrettyPrintingLightningRpc(LightningRpc): class LightningNode(object): - def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fail=False, + def __init__(self, node_id, lightning_dir, bitcoind, executor, may_fail=False, may_reconnect=False, broken_log=None, allow_warning=False, @@ -900,7 +900,7 @@ class LightningNode(object): self.daemon.opts["dev-debugger"] = dbgvar if os.getenv("DEBUG_LIGHTNINGD"): self.daemon.opts["dev-debug-self"] = None - if valgrind: + if VALGRIND: self.daemon.env["LIGHTNINGD_DEV_NO_BACKTRACE"] = "1" self.daemon.opts["dev-no-plugin-checksum"] = None else: @@ -926,7 +926,7 @@ class LightningNode(object): dsn = db.get_dsn() if dsn is not None: self.daemon.opts['wallet'] = dsn - if valgrind: + if VALGRIND: trace_skip_pattern = '*python*,*bitcoin-cli*,*elements-cli*,*cln-grpc*,*clnrest*,*wss-proxy*,*cln-bip353*,*reckless' if not valgrind_plugins: trace_skip_pattern += ',*plugins*' @@ -1653,10 +1653,6 @@ class NodeFactory(object): """ def __init__(self, request, testname, bitcoind, executor, directory, db_provider, node_cls, jsonschemas): - if request.node.get_closest_marker("slow_test") and SLOW_MACHINE: - self.valgrind = False - else: - self.valgrind = VALGRIND self.testname = testname # Set test name in environment for coverage file organization @@ -1755,7 +1751,7 @@ class NodeFactory(object): db = self.db_provider.get_db(os.path.join(lightning_dir, TEST_NETWORK), self.testname, node_id) db.provider = self.db_provider node = self.node_cls( - node_id, lightning_dir, self.bitcoind, self.executor, self.valgrind, db=db, + node_id, lightning_dir, self.bitcoind, self.executor, db=db, port=port, grpc_port=grpc_port, options=options, may_fail=may_fail or expect_fail, jsonschemas=self.jsonschemas, **kwargs @@ -1872,7 +1868,7 @@ class NodeFactory(object): # leak detection upsets VALGRIND by reading uninitialized mem, # and valgrind adds extra fds. # If it's dead, we'll catch it below. - if not self.valgrind: + if not VALGRIND: try: # This also puts leaks in log. leaks = self.nodes[i].rpc.dev_memleak()['leaks'] diff --git a/tests/conftest.py b/tests/conftest.py index 029050742..dcbd43cab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,6 @@ import pytest -from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND +from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, VALGRIND, SLOW_MACHINE # This function is based upon the example of how to @@ -37,3 +37,5 @@ def pytest_runtest_setup(item): else: # If there's no openchannel marker, skip if EXP_DF if EXPERIMENTAL_DUAL_FUND: pytest.skip('v1-only test, EXPERIMENTAL_DUAL_FUND=1') + if "slow_test" in item.keywords and VALGRIND and SLOW_MACHINE: + pytest.skip("Skipping slow tests under VALGRIND") diff --git a/tests/test_closing.py b/tests/test_closing.py index 3a7c31642..f3c7dbee0 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1,7 +1,7 @@ from fixtures import * # noqa: F401,F403 from pyln.client import RpcError, Millisatoshi from shutil import copyfile -from pyln.testing.utils import SLOW_MACHINE +from pyln.testing.utils import SLOW_MACHINE, VALGRIND from utils import ( only_one, sync_blockheight, wait_for, TIMEOUT, account_balance, first_channel_id, closing_fee, TEST_NETWORK, @@ -3232,7 +3232,7 @@ def test_permfail(node_factory, bitcoind): def test_shutdown(node_factory): # Fail, in that it will exit before cleanup. l1 = node_factory.get_node(may_fail=True) - if not node_factory.valgrind: + if not VALGRIND: leaks = l1.rpc.dev_memleak()['leaks'] if len(leaks): raise Exception("Node {} has memory leaks: {}" diff --git a/tests/test_connection.py b/tests/test_connection.py index 05724c517..78261f711 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -13,7 +13,7 @@ from utils import ( mine_funding_to_announce, first_scid, CHANNEL_SIZE ) -from pyln.testing.utils import SLOW_MACHINE, VALGRIND, EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT, RUST +from pyln.testing.utils import VALGRIND, EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT, RUST import os import pytest @@ -1463,7 +1463,7 @@ def test_funding_v2_corners(node_factory, bitcoind): assert l1.rpc.openchannel_update(start['channel_id'], start['psbt'])['commitments_secured'] -@unittest.skipIf(SLOW_MACHINE and not VALGRIND, "Way too taxing on CI machines") +@pytest.mark.slow_test @pytest.mark.openchannel('v1') def test_funding_cancel_race(node_factory, bitcoind, executor): l1 = node_factory.get_node() @@ -1474,7 +1474,7 @@ def test_funding_cancel_race(node_factory, bitcoind, executor): bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0) - if node_factory.valgrind: + if VALGRIND: num = 5 else: num = 100 @@ -1536,7 +1536,7 @@ def test_funding_cancel_race(node_factory, bitcoind, executor): assert num_cancel == len(nodes) # We should have raced at least once! - if not node_factory.valgrind: + if not VALGRIND: assert num_cancel > 0 assert num_complete > 0 @@ -1544,7 +1544,7 @@ def test_funding_cancel_race(node_factory, bitcoind, executor): executor.map(lambda n: n.stop(), node_factory.nodes) -@unittest.skipIf(SLOW_MACHINE and not VALGRIND, "Way too taxing on CI machines") +@pytest.mark.slow_test @pytest.mark.openchannel('v2') def test_funding_v2_cancel_race(node_factory, bitcoind, executor): l1 = node_factory.get_node() @@ -1555,7 +1555,7 @@ def test_funding_v2_cancel_race(node_factory, bitcoind, executor): bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0) - if node_factory.valgrind: + if VALGRIND: num = 5 else: num = 100 @@ -1610,7 +1610,7 @@ def test_funding_v2_cancel_race(node_factory, bitcoind, executor): print("Cancelled {} complete {}".format(num_cancel, num_complete)) # We should have raced at least once! - if not node_factory.valgrind: + if not VALGRIND: assert num_cancel > 0 assert num_complete > 0 @@ -3452,7 +3452,7 @@ def test_feerate_stress(node_factory, executor): @pytest.mark.slow_test def test_pay_disconnect_stress(node_factory, executor): """Expose race in htlc restoration in channeld: 50% chance of failure""" - if node_factory.valgrind: + if VALGRIND: NUM_RUNS = 2 else: NUM_RUNS = 5 diff --git a/tests/test_plugin.py b/tests/test_plugin.py index f02bd8070..1b5499269 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2983,7 +2983,7 @@ def test_autoclean(node_factory): # Under valgrind in CI, it can 50 seconds between creating invoice # and restarting. - if node_factory.valgrind: + if VALGRIND: short_timeout = 10 longer_timeout = 60 else: diff --git a/tests/test_reckless.py b/tests/test_reckless.py index 7fb04d742..275960d57 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -2,7 +2,7 @@ from fixtures import * # noqa: F401,F403 import subprocess from pathlib import PosixPath, Path import socket -from pyln.testing.utils import VALGRIND, SLOW_MACHINE +from pyln.testing.utils import VALGRIND import pytest import os import re @@ -352,7 +352,7 @@ def test_tag_install(node_factory): @pytest.mark.flaky(reruns=5) -@unittest.skipIf(VALGRIND and SLOW_MACHINE, "node too slow for starting plugin under valgrind") +@pytest.mark.slow_test def test_reckless_uv_install(node_factory): node = get_reckless_node(node_factory) node.start()