autoreload: Add tests and warning when manifest changes

Rene pointed out that we were silently accepting an updated manifest despite
not actually being able to register it with `lightningd`. This meant that if
options, RPC methods, hooks or notifications get added or removed they will
not be wired in correctly.

The warning is really obnoxious and multi-line to draw the attention.

Suggested-by: Rene Pickhardt <@renepickhardt>

autoreload/autoreload.py

@@ -37,8 +37,11 @@ class ChildPlugin(object):
                 last = now
                 try:
                     self.restart()
-                except:
-                    self.plugin.log("Failed to start plugin, will wait for next change and try again.", level='warn')
+                except Exception as e:
+                    self.plugin.log(
+                        "Failed to start plugin, will wait for next change and try again:",
+                        level='error'
+                    )
 
     def handle_init(self, request):
         """Lightningd has sent us its first init message, clean and forward.
@@ -93,7 +96,7 @@ class ChildPlugin(object):
         try:
             self.proc = subprocess.Popen([self.path], stdout=subprocess.PIPE, stdin=subprocess.PIPE)
             self.status = 'started'
-            self.getmanifest()
+            manifest = self.getmanifest()
             return True
         except Exception as e:
             self.plugin.log(e, level='warn')
@@ -129,6 +132,15 @@ class ChildPlugin(object):
                 raise ValueError()
 
             if 'id' in msg and msg['id'] == 0:
+
+                if self.manifest is not None and msg['result'] != self.manifest:
+                    plugin.log(
+                        "Plugin manifest changed between restarts: {new} != {old}\n\n"
+                        "==> You need to restart c-lightning for these changes to be picked up! <==".format(
+                            new=json.dumps(msg['result'], indent=True).replace("\"", "'"),
+                            old=json.dumps(self.manifest, indent=True).replace("\"", "'")
+                    ), level='warn')
+                    raise ValueError()
                 self.manifest = msg['result']
                 break
             self.plugin._write_locked(msg)
@@ -180,6 +192,7 @@ class ChildPlugin(object):
 
 @plugin.init()
 def init(options, configuration, plugin, request):
+    #import remote_pdb; remote_pdb.set_trace()
     if options['autoreload-plugin'] in ['null', None]:
         print("Cannot run the autoreload plugin on its own, please specify --autoreload-plugin")
         plugin.rpc.stop()

autoreload/tests/dummy.py

@@ -0,0 +1,7 @@
+#!/usr/bin/env python3
+
+from pyln.client import Plugin
+
+plugin = Plugin()
+
+plugin.run()

autoreload/tests/dummy2.py

@@ -0,0 +1,11 @@
+#!/usr/bin/env python3
+
+from pyln.client import Plugin
+
+plugin = Plugin()
+
+@plugin.method('dummy')
+def on_dummy():
+    pass
+
+plugin.run()

autoreload/tests/test_autoreload.py

@@ -0,0 +1,59 @@
+from pyln.testing.fixtures import *
+import shutil
+import subprocess
+import time
+import os
+import unittest
+
+def copy_plugin(src, directory, filename=None):
+    base = os.path.dirname(__file__)
+    src = os.path.join(base, src)
+    dst = os.path.join(directory, filename if filename is not None else os.path.basename(src))
+    shutil.copy(src, dst)
+    shutil.copystat(src, dst)
+    return dst
+
+
+def test_restart_on_change(node_factory, directory):
+
+    # Copy the dummy plugin over
+    plugin = copy_plugin("dummy.py", directory, "plugin.py")
+
+    opts = {
+        'plugin': os.path.join(os.path.dirname(__file__), "..", 'autoreload.py'),
+        'autoreload-plugin': plugin,
+    }
+
+    l1 = node_factory.get_node(options=opts)
+
+    subprocess.check_call(['touch', plugin])
+    l1.daemon.wait_for_log(r'Detected a change in the child plugin, restarting')
+
+
+@unittest.skipIf(True, "Doesn't work on travis yet")
+def test_changing_manifest(node_factory, directory):
+    """Change the manifest in-between restarts.
+
+    Adding an RPC method like the switch from dummy.py to dummy2.py does,
+    should result in an error while reloading the plugin.
+
+    """
+
+    # Copy the dummy plugin over
+    plugin = copy_plugin("dummy.py", directory, "plugin.py")
+    plugin_path = os.path.join(os.path.dirname(__file__), "..", 'autoreload.py')
+    opts = {
+        'plugin': os.path.join(os.path.dirname(__file__), "..", 'autoreload.py'),
+        'autoreload-plugin': plugin,
+    }
+
+    l1 = node_factory.get_node(options=opts, allow_broken_log=True)
+
+
+    plugin = copy_plugin("dummy2.py", directory, "plugin.py")
+    time.sleep(10)
+    subprocess.check_call(['touch', plugin_path])
+    time.sleep(10)
+    subprocess.check_call(['touch', plugin_path])
+    l1.daemon.wait_for_log(r'Detected a change in the child plugin, restarting')
+    l1.daemon.wait_for_log(r'You need to restart c-lightning')