From 01a82d38f769a410e4f2d127c9f09e8f8b296554 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Aug 2020 12:44:59 +0930 Subject: [PATCH] pytest: add slow_test marker. And when it's set, and we're SLOW_MACHINE, simply disable valgrind. Since Travis (SLOW_MACHINE=1) only does VALGRIND=1 DEVELOPER=1 tests, and VALGRIND=0 DEVELOPER=0 tests, it was missing tests which needed DEVELOPER and !VALGRIND. Instead, this demotes them to non-valgrind tests for SLOW_MACHINEs. Signed-off-by: Rusty Russell --- contrib/pyln-testing/pyln/testing/fixtures.py | 1 + contrib/pyln-testing/pyln/testing/utils.py | 7 ++++-- tests/test_closing.py | 22 +++++++++---------- tests/test_connection.py | 15 ++++++++----- tests/test_pay.py | 14 +++++++----- tests/utils.py | 2 +- 6 files changed, 34 insertions(+), 27 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index 3f61bc930..3f32a6561 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -174,6 +174,7 @@ def teardown_checks(request): @pytest.fixture def node_factory(request, directory, test_name, bitcoind, executor, db_provider, teardown_checks, node_cls): nf = NodeFactory( + request, test_name, bitcoind, executor, diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index d0a6bcaf7..37b5900a6 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -959,7 +959,11 @@ class LightningNode(object): class NodeFactory(object): """A factory to setup and start `lightningd` daemons. """ - def __init__(self, testname, bitcoind, executor, directory, db_provider, node_cls): + def __init__(self, request, testname, bitcoind, executor, directory, db_provider, node_cls): + if request.node.get_closest_marker("slow_test") and SLOW_MACHINE: + self.valgrind = False + else: + self.valgrind = VALGRIND self.testname = testname self.next_id = 1 self.nodes = [] @@ -969,7 +973,6 @@ class NodeFactory(object): self.lock = threading.Lock() self.db_provider = db_provider self.node_cls = node_cls - self.valgrind = VALGRIND def split_options(self, opts): """Split node options from cli options diff --git a/tests/test_closing.py b/tests/test_closing.py index 193f2471f..503480fb5 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3,8 +3,8 @@ from flaky import flaky from pyln.client import RpcError from shutil import copyfile from utils import ( - only_one, sync_blockheight, wait_for, DEVELOPER, TIMEOUT, VALGRIND, - SLOW_MACHINE, account_balance, first_channel_id + only_one, sync_blockheight, wait_for, DEVELOPER, TIMEOUT, + account_balance, first_channel_id ) import os @@ -149,17 +149,15 @@ def test_closing_id(node_factory): wait_for(lambda: not only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['connected']) -@unittest.skipIf(VALGRIND, "Flaky under valgrind") +@pytest.mark.slow_test def test_closing_torture(node_factory, executor, bitcoind): # We set up a fully-connected mesh of N nodes, then try # closing them all at once. amount = 10**6 num_nodes = 10 # => 45 channels (36 seconds on my laptop) - if VALGRIND: + if node_factory.valgrind: num_nodes -= 4 # => 15 (135 seconds) - if SLOW_MACHINE: - num_nodes -= 1 # => 36/10 (37/95 seconds) nodes = node_factory.get_nodes(num_nodes) @@ -213,7 +211,7 @@ def test_closing_torture(node_factory, executor, bitcoind): wait_for(lambda: n.rpc.listpeers()['peers'] == []) -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_closing_different_fees(node_factory, bitcoind, executor): l1 = node_factory.get_node() @@ -690,8 +688,8 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db") +@pytest.mark.slow_test def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): """ Test that the penalizing node claims any published HTLC transactions @@ -823,8 +821,8 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db") +@pytest.mark.slow_test def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams): """ Test that the penalizing node claims any published HTLC transactions @@ -1866,7 +1864,7 @@ def setup_multihtlc_test(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for dev_ignore_htlcs") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_onchain_multihtlc_our_unilateral(node_factory, bitcoind): """Node pushes a channel onchain with multiple HTLCs with same payment_hash """ h, nodes = setup_multihtlc_test(node_factory, bitcoind) @@ -1958,7 +1956,7 @@ def test_onchain_multihtlc_our_unilateral(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for dev_ignore_htlcs") -@unittest.skipIf(SLOW_MACHINE and VALGRIND, "slow test") +@pytest.mark.slow_test def test_onchain_multihtlc_their_unilateral(node_factory, bitcoind): """Node pushes a channel onchain with multiple HTLCs with same payment_hash """ h, nodes = setup_multihtlc_test(node_factory, bitcoind) @@ -2257,7 +2255,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 VALGRIND: + if not node_factory.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 d099706fb..69957b9f6 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -5,8 +5,8 @@ from fixtures import TEST_NETWORK from flaky import flaky # noqa: F401 from pyln.client import RpcError, Millisatoshi from utils import ( - DEVELOPER, only_one, wait_for, sync_blockheight, VALGRIND, TIMEOUT, - SLOW_MACHINE, expected_peer_features, expected_node_features, + DEVELOPER, only_one, wait_for, sync_blockheight, TIMEOUT, + expected_peer_features, expected_node_features, expected_channel_features, check_coin_moves, first_channel_id, account_balance ) @@ -140,6 +140,7 @@ def test_bad_opening(node_factory): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") @unittest.skipIf(TEST_NETWORK != 'regtest', "Fee computation and limits are network specific") +@pytest.mark.slow_test def test_opening_tiny_channel(node_factory): # Test custom min-capacity-sat parameters # @@ -990,7 +991,7 @@ def test_funding_external_wallet_corners(node_factory, bitcoind): def test_funding_cancel_race(node_factory, bitcoind, executor): l1 = node_factory.get_node() - if VALGRIND or SLOW_MACHINE: + if node_factory.valgrind: num = 5 else: num = 100 @@ -1051,7 +1052,7 @@ def test_funding_cancel_race(node_factory, bitcoind, executor): assert num_cancel == len(nodes) # We should have raced at least once! - if not VALGRIND: + if not node_factory.valgrind: assert num_cancel > 0 assert num_complete > 0 @@ -1994,11 +1995,12 @@ def test_fulfill_incoming_first(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") +@pytest.mark.slow_test def test_restart_many_payments(node_factory, bitcoind): l1 = node_factory.get_node(may_reconnect=True) # On my laptop, these take 89 seconds and 12 seconds - if VALGRIND: + if node_factory.valgrind: num = 2 else: num = 5 @@ -2235,9 +2237,10 @@ def test_feerate_stress(node_factory, executor): @unittest.skipIf(not DEVELOPER, "need dev_disconnect") +@pytest.mark.slow_test def test_pay_disconnect_stress(node_factory, executor): """Expose race in htlc restoration in channeld: 50% chance of failure""" - if SLOW_MACHINE and VALGRIND: + if node_factory.valgrind: NUM_RUNS = 2 else: NUM_RUNS = 5 diff --git a/tests/test_pay.py b/tests/test_pay.py index 533fb8764..dc5d99364 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5,8 +5,8 @@ from flaky import flaky # noqa: F401 from pyln.client import RpcError, Millisatoshi from pyln.proto.onion import TlvPayload from utils import ( - DEVELOPER, wait_for, only_one, sync_blockheight, SLOW_MACHINE, TIMEOUT, - VALGRIND, EXPERIMENTAL_FEATURES + DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT, + EXPERIMENTAL_FEATURES ) import copy import os @@ -1303,7 +1303,8 @@ def test_forward_stats(node_factory, bitcoind): assert 'received_time' in stats['forwards'][2] and 'resolved_time' not in stats['forwards'][2] -@unittest.skipIf(not DEVELOPER or (VALGRIND and SLOW_MACHINE), "Gossip too slow without DEVELOPER, and too stressful if VALGRIND on slow machines") +@unittest.skipIf(not DEVELOPER, "too slow without --dev-fast-gossip") +@pytest.mark.slow_test def test_forward_local_failed_stats(node_factory, bitcoind, executor): """Check that we track forwarded payments correctly. @@ -1522,7 +1523,8 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor): assert 'received_time' in stats['forwards'][3] and 'resolved_time' not in stats['forwards'][4] -@unittest.skipIf(not DEVELOPER or SLOW_MACHINE, "needs DEVELOPER=1 for dev_ignore_htlcs, and temporarily disabled on Travis") +@unittest.skipIf(not DEVELOPER, "too slow without --dev-fast-gossip") +@pytest.mark.slow_test def test_htlcs_cltv_only_difference(node_factory, bitcoind): # l1 -> l2 -> l3 -> l4 # l4 ignores htlcs, so they stay. @@ -1599,7 +1601,7 @@ def test_pay_variants(node_factory): @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow") -@unittest.skipIf(VALGRIND and SLOW_MACHINE, "Travis times out under valgrind") +@pytest.mark.slow_test def test_pay_retry(node_factory, bitcoind, executor, chainparams): """Make sure pay command retries properly. """ @@ -1683,7 +1685,7 @@ def test_pay_retry(node_factory, bitcoind, executor, chainparams): @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 otherwise gossip takes 5 minutes!") -@unittest.skipIf(VALGRIND, "temporarily disabled due to timeouts") +@pytest.mark.slow_test def test_pay_routeboost(node_factory, bitcoind, compat): """Make sure we can use routeboost information. """ # l1->l2->l3--private-->l4 diff --git a/tests/utils.py b/tests/utils.py index 843664393..f7a337e16 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,4 +1,4 @@ -from pyln.testing.utils import TEST_NETWORK, SLOW_MACHINE, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS # noqa: F401 +from pyln.testing.utils import TEST_NETWORK, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS # noqa: F401 from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum # noqa: F401 import bitstring from pyln.client import Millisatoshi