From 20a2bf9547e821eba0310479001043d150e66ed3 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 27 Aug 2019 10:36:38 +0930
Subject: [PATCH] pytest: make test_forward_event_notification more explicit.

We seem to be getting intermittant failures, but it's hard
to disgnose.  Simplify it by moving all the test logic into
the test itself, and making the plugin dumber.  This means we'll
see exactly what the differences are if it fails again.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 tests/plugins/forward_payment_status.py | 33 +++---------------------
 tests/test_plugin.py                    | 34 ++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/tests/plugins/forward_payment_status.py b/tests/plugins/forward_payment_status.py
index 040cb321a..1bb35a810 100755
--- a/tests/plugins/forward_payment_status.py
+++ b/tests/plugins/forward_payment_status.py
@@ -6,24 +6,6 @@ from lightning import Plugin
 plugin = Plugin()
 
 
-def check(forward, dbforward):
-    # After finding the corresponding notification record, this function will
-    # make some changes on mutative fields of this record to make this record
-    # same as the ideal format with given status.
-    record = forward
-    if record['status'] == 'offered':
-        if dbforward['status'] == 'local_failed':
-            record['failcode'] = dbforward['failcode']
-            record['failreason'] = dbforward['failreason']
-        elif dbforward['status'] != 'offered':
-            record['resolved_time'] = dbforward['resolved_time']
-    record['status'] = dbforward['status']
-    if record == dbforward:
-        return True
-    else:
-        return False
-
-
 @plugin.init()
 def init(configuration, options, plugin):
     plugin.forward_list = []
@@ -37,18 +19,9 @@ def notify_warning(plugin, forward_event):
     plugin.forward_list.append(forward_event)
 
 
-@plugin.method('recordcheck')
-def record_lookup(payment_hash, status, dbforward, plugin):
-    # Check if we received all notifications when forward changed.
-    # This check is based on the records of 'listforwards'
-    plugin.log("recordcheck: payment_hash: {}, status: {}".format(payment_hash, status))
-    for forward in plugin.forward_list:
-        if forward['payment_hash'] == payment_hash and forward['status'] == status:
-            plugin.log("record exists")
-            check_result = check(forward, dbforward)
-            return check_result
-    plugin.log("no record")
-    return False
+@plugin.method('listforwards_plugin')
+def record_lookup(plugin):
+    return {'forwards': plugin.forward_list}
 
 
 plugin.run()
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index e1ece1a52..4ab2fceb6 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -649,14 +649,34 @@ def test_forward_event_notification(node_factory, bitcoind, executor):
     bitcoind.generate_block(100)
     sync_blockheight(bitcoind, [l2])
 
-    stats = l2.rpc.listforwards()
+    stats = l2.rpc.listforwards()['forwards']
+    assert len(stats) == 3
+    plugin_stats = l2.rpc.call('listforwards_plugin')['forwards']
+    assert len(plugin_stats) == 6
 
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash13, 'status': 'offered', 'dbforward': stats['forwards'][0]})
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash13, 'status': 'settled', 'dbforward': stats['forwards'][0]})
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash14, 'status': 'offered', 'dbforward': stats['forwards'][1]})
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash14, 'status': 'failed', 'dbforward': stats['forwards'][1]})
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash15, 'status': 'offered', 'dbforward': stats['forwards'][2]})
-    assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash15, 'status': 'local_failed', 'dbforward': stats['forwards'][2]})
+    # use stats to build what we expect went to plugin.
+    expect = stats[0].copy()
+    # First event won't have conclusion.
+    del expect['resolved_time']
+    expect['status'] = 'offered'
+    assert plugin_stats[0] == expect
+    expect = stats[0].copy()
+    assert plugin_stats[1] == expect
+
+    expect = stats[1].copy()
+    del expect['resolved_time']
+    expect['status'] = 'offered'
+    assert plugin_stats[2] == expect
+    expect = stats[1].copy()
+    assert plugin_stats[3] == expect
+
+    expect = stats[2].copy()
+    del expect['failcode']
+    del expect['failreason']
+    expect['status'] = 'offered'
+    assert plugin_stats[4] == expect
+    expect = stats[2].copy()
+    assert plugin_stats[5] == expect
 
 
 def test_plugin_deprecated_relpath(node_factory):