Skip to content

Commit

Permalink
pytest: Remove valgrind opt-out via tests marker
Browse files Browse the repository at this point in the history
Reviewing ElementsProject#4391 I noticed that we have a sneaky way of opting out of
valgrind using the `slow_test` marker. I consider this dangerous as it
causes a false sense of security ("this test is being run under
valgrind" when really it isn't), as exemplified by some of the tests
even branching on whether we run under valgrind or not.

This is my attempt of making tests explicitly opt out of valgrind
testing via the `@unittest.skipIf` decorator, which is more honest, as
it doesn't run the test in a tweaked configuration, and reports a
skipped test. The tests are all already being run on CI without
valgrind, so we don't lose any coverage by this, but are more
respectful of the resources we have on CI.
  • Loading branch information
cdecker committed Feb 26, 2021
1 parent 644fae1 commit 016aea5
Showing 1 changed file with 5 additions and 14 deletions.
19 changes: 5 additions & 14 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ 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
#
Expand Down Expand Up @@ -2586,16 +2585,10 @@ def test_fulfill_incoming_first(node_factory, bitcoind):


@unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow")
@pytest.mark.slow_test
@unittest.skipIf(VALGRIND, "Too memory hungry under valgrind")
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 node_factory.valgrind:
num = 2
else:
num = 5

num = 5
nodes = node_factory.get_nodes(num * 2, opts={'may_reconnect': True})
innodes = nodes[:num]
outnodes = nodes[num:]
Expand Down Expand Up @@ -2883,13 +2876,11 @@ def test_feerate_stress(node_factory, executor):


@unittest.skipIf(not DEVELOPER, "need dev_disconnect")
@pytest.mark.slow_test
@unittest.skipIf(VALGRIND, "Reconnection slow due to process spawning")
def test_pay_disconnect_stress(node_factory, executor):
"""Expose race in htlc restoration in channeld: 50% chance of failure"""
if node_factory.valgrind:
NUM_RUNS = 2
else:
NUM_RUNS = 5
NUM_RUNS = 5

for i in range(NUM_RUNS):
l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True},
{'may_reconnect': True,
Expand Down

0 comments on commit 016aea5

Please sign in to comment.