From 17b9bd5ca366dbf9bbb1e10f083945f98fcae8f1 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 26 Jul 2022 14:22:53 +0930
Subject: [PATCH] pytest: fix test_commando_rune flake.

We reset counters every minute, so ratelimit tests can flake since we
might hit that boundary.

Instead, wait for the reset then test explicitly, assuming that takes
less than 60 seconds.

```
        for rune, cmd, params in failures:
            print("{} {}".format(cmd, params))
            with pytest.raises(RpcError, match='Not authorized:') as exc_info:
                l2.rpc.call(method='commando',
                            payload={'peer_id': l1.info['id'],
                                     'rune': rune['rune'],
                                     'method': cmd,
>                                    'params': params})
E               Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>
...
DEBUG:root:Calling commando with payload {'peer_id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'rune': 'O8Zr-ULTBKO3_pKYz0QKE9xYl1vQ4Xx9PtlHuist9Rk9NCZwbnVtPTAmcmF0ZT0zJnJhdGU9MQ==', 'method': 'getinfo', 'params': {}}
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 tests/test_plugin.py | 59 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index fb0790d61..67b242bc1 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -2739,9 +2739,7 @@ def test_commando_rune(node_factory):
     # Replace rune3 with a more useful timestamp!
     expiry = int(time.time()) + 15
     rune3 = l1.rpc.commando_rune(restrictions="time<{}".format(expiry))
-    ratelimit_successes = ((rune9, "getinfo", {}),
-                           (rune8, "getinfo", {}),
-                           (rune8, "getinfo", {}))
+
     successes = ((rune1, "listpeers", {}),
                  (rune2, "listpeers", {}),
                  (rune2, "getinfo", {}),
@@ -2753,7 +2751,11 @@ def test_commando_rune(node_factory):
                  (rune6, "listpeers", [l2.info['id'], 'broken']),
                  (rune6, "listpeers", [l2.info['id']]),
                  (rune7, "listpeers", []),
-                 (rune7, "getinfo", {})) + ratelimit_successes
+                 (rune7, "getinfo", {}),
+                 (rune9, "getinfo", {}),
+                 (rune8, "getinfo", {}),
+                 (rune8, "getinfo", {}))
+
     failures = ((rune2, "withdraw", {}),
                 (rune2, "plugin", {'subcommand': 'list'}),
                 (rune3, "getinfo", {}),
@@ -2761,9 +2763,7 @@ def test_commando_rune(node_factory):
                 (rune5, "listpeers", {'id': l2.info['id'], 'level': 'io'}),
                 (rune6, "listpeers", [l2.info['id'], 'io']),
                 (rune7, "listpeers", [l2.info['id']]),
-                (rune7, "listpeers", {'id': l2.info['id']}),
-                (rune9, "getinfo", {}),
-                (rune8, "getinfo", {}))
+                (rune7, "listpeers", {'id': l2.info['id']}))
 
     for rune, cmd, params in successes:
         l2.rpc.call(method='commando',
@@ -2785,6 +2785,47 @@ def test_commando_rune(node_factory):
                                  'params': params})
         assert exc_info.value.error['code'] == 0x4c51
 
+    # Now, this can flake if we cross a minute boundary!  So wait until
+    # It succeeds again.
+    while True:
+        try:
+            l2.rpc.call(method='commando',
+                        payload={'peer_id': l1.info['id'],
+                                 'rune': rune8['rune'],
+                                 'method': 'getinfo',
+                                 'params': {}})
+            break
+        except RpcError as e:
+            assert e.error['code'] == 0x4c51
+        time.sleep(1)
+
+    # This fails immediately, since we've done one.
+    with pytest.raises(RpcError, match='Not authorized:') as exc_info:
+        l2.rpc.call(method='commando',
+                    payload={'peer_id': l1.info['id'],
+                             'rune': rune9['rune'],
+                             'method': 'getinfo',
+                             'params': {}})
+    assert exc_info.value.error['code'] == 0x4c51
+
+    # Two more succeed for rune8.
+    for _ in range(2):
+        l2.rpc.call(method='commando',
+                    payload={'peer_id': l1.info['id'],
+                             'rune': rune8['rune'],
+                             'method': 'getinfo',
+                             'params': {}})
+    assert exc_info.value.error['code'] == 0x4c51
+
+    # Now we've had 3 in one minute, this will fail.
+    with pytest.raises(RpcError, match='Not authorized:') as exc_info:
+        l2.rpc.call(method='commando',
+                    payload={'peer_id': l1.info['id'],
+                             'rune': rune8['rune'],
+                             'method': 'getinfo',
+                             'params': {}})
+    assert exc_info.value.error['code'] == 0x4c51
+
     # rune5 can only be used by l2:
     l3 = node_factory.get_node()
     l3.connect(l1)
@@ -2799,7 +2840,9 @@ def test_commando_rune(node_factory):
     # Now wait for ratelimit expiry, ratelimits should reset.
     time.sleep(61)
 
-    for rune, cmd, params in ratelimit_successes:
+    for rune, cmd, params in ((rune9, "getinfo", {}),
+                              (rune8, "getinfo", {}),
+                              (rune8, "getinfo", {})):
         l2.rpc.call(method='commando',
                     payload={'peer_id': l1.info['id'],
                              'rune': rune['rune'],