Skip to content

Commit

Permalink
pytest: detect warnings, too.
Browse files Browse the repository at this point in the history
Since we turned many errors into warnings, we want our tests to fail
when they happen unexpectedly.  We make WARNING clear in the strings
we print, too, to help out.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Feb 1, 2021
1 parent c0f928f commit 9bf8e5d
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 14 deletions.
1 change: 1 addition & 0 deletions contrib/pyln-testing/pyln/testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def map_node_error(nodes, f, msg):
map_node_error(nf.nodes, printValgrindErrors, "reported valgrind errors")
map_node_error(nf.nodes, printCrashLog, "had crash.log files")
map_node_error(nf.nodes, lambda n: not n.allow_broken_log and n.daemon.is_in_log(r'\*\*BROKEN\*\*'), "had BROKEN messages")
map_node_error(nf.nodes, lambda n: not n.allow_warning and n.daemon.is_in_log(r' WARNING:'), "had warning messages")
map_node_error(nf.nodes, checkReconnect, "had unexpected reconnections")
map_node_error(nf.nodes, checkBadGossip, "had bad gossip messages")
map_node_error(nf.nodes, lambda n: n.daemon.is_in_log('Bad reestablish'), "had bad reestablish")
Expand Down
9 changes: 7 additions & 2 deletions contrib/pyln-testing/pyln/testing/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,15 +618,19 @@ def call(self, method, payload=None):

class LightningNode(object):
def __init__(self, node_id, lightning_dir, bitcoind, executor, valgrind, may_fail=False,
may_reconnect=False, allow_broken_log=False,
allow_bad_gossip=False, db=None, port=None, disconnect=None, random_hsm=None, options=None,
may_reconnect=False,
allow_broken_log=False,
allow_warning=False,
allow_bad_gossip=False,
db=None, port=None, disconnect=None, random_hsm=None, options=None,
**kwargs):
self.bitcoin = bitcoind
self.executor = executor
self.may_fail = may_fail
self.may_reconnect = may_reconnect
self.allow_broken_log = allow_broken_log
self.allow_bad_gossip = allow_bad_gossip
self.allow_warning = allow_warning
self.db = db

# Assume successful exit
Expand Down Expand Up @@ -1203,6 +1207,7 @@ def split_options(self, opts):
'disconnect',
'may_fail',
'allow_broken_log',
'allow_warning',
'may_reconnect',
'random_hsm',
'feerates',
Expand Down
2 changes: 1 addition & 1 deletion lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ void channel_errmsg(struct channel *channel,
* and we would close the channel on them. We now support warnings
* for this case. */
if (warning) {
channel_fail_reconnect_later(channel, "%s: (ignoring) %s",
channel_fail_reconnect_later(channel, "%s WARNING: %s",
channel->owner->name, desc);
return;
}
Expand Down
5 changes: 4 additions & 1 deletion tests/test_closing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2523,7 +2523,10 @@ def test_shutdown(node_factory):
@flaky
@unittest.skipIf(not DEVELOPER, "needs to set upfront_shutdown_script")
def test_option_upfront_shutdown_script(node_factory, bitcoind, executor):
l1 = node_factory.get_node(start=False)
# There's a workaround in channeld, that it treats incoming errors
# before both sides are locked in as warnings; this happens in
# this test, so l1 reports the error as a warning!
l1 = node_factory.get_node(start=False, allow_warning=True)
# Insist on upfront script we're not going to match.
l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = "76a91404b61f7dc1ea0dc99424464cc4064dc564d91e8988ac"
l1.start()
Expand Down
6 changes: 4 additions & 2 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1956,8 +1956,10 @@ def test_update_fee(node_factory, bitcoind):

@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
def test_fee_limits(node_factory, bitcoind):
l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'dev-max-fee-multiplier': 5, 'may_reconnect': True},
{'dev-max-fee-multiplier': 5, 'may_reconnect': True},
l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'dev-max-fee-multiplier': 5, 'may_reconnect': True,
'allow_warning': True},
{'dev-max-fee-multiplier': 5, 'may_reconnect': True,
'allow_warning': True},
{'ignore-fee-limits': True, 'may_reconnect': True},
{}])

Expand Down
2 changes: 1 addition & 1 deletion tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ def test_bitcoind_goes_backwards(node_factory, bitcoind):
@flaky
def test_reserve_enforcement(node_factory, executor):
"""Channeld should disallow you spending into your reserve"""
l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True})
l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True, 'allow_warning': True})

# Pay 1000 satoshi to l2.
l1.pay(l2, 1000000)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_pay.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ def test_pay0(node_factory):
def test_pay_disconnect(node_factory, bitcoind):
"""If the remote node has disconnected, we fail payment, but can try again when it reconnects"""
l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5,
'may_reconnect': True})
'may_reconnect': True,
'allow_warning': True})

# Dummy payment to kick off update_fee messages
l1.pay(l2, 1000)
Expand All @@ -261,7 +262,7 @@ def test_pay_disconnect(node_factory, bitcoind):
l1.set_feerates((10**6, 1000**6, 1000**6, 1000**6), False)

# Wait for l1 notice
l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld: .*: update_fee \d+ outside range 1875-75000')
l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld WARNING: .*: update_fee \d+ outside range 1875-75000')

# Make l2 fail hard.
l2.rpc.close(l1.info['id'], unilateraltimeout=1)
Expand Down
15 changes: 10 additions & 5 deletions tests/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,16 @@ def test_plugin_connected_hook_chaining(node_factory):
l1 is configured to accept connections from l2, but not from l3.
we check that logger_a is always called and logger_b only for l2.
"""
opts = [{'plugin': [
os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_a.py'),
os.path.join(os.getcwd(), 'tests/plugins/reject.py'),
os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_b.py'),
]}, {}, {}]
opts = [{'plugin':
[os.path.join(os.getcwd(),
'tests/plugins/peer_connected_logger_a.py'),
os.path.join(os.getcwd(),
'tests/plugins/reject.py'),
os.path.join(os.getcwd(),
'tests/plugins/peer_connected_logger_b.py')],
'allow_warning': True},
{},
{'allow_warning': True}]

l1, l2, l3 = node_factory.get_nodes(3, opts=opts)
l2id = l2.info['id']
Expand Down

0 comments on commit 9bf8e5d

Please sign in to comment.