Reimplement plugin system

Implement a new plugin system based subclassing pre-defined base clases
and communicating with pre-defined link messages.

CHANGELOG.md

@@ -19,6 +19,9 @@ Version 2.0.0
 - Rewritten the communication between slaves and the master from scratch.  The
   new system is based on a WebSocket connection between the slaves and the
   master server and provides efficient validated bi-directonal communication.
+- Rewritten the plugin system from scratch.  Plugins now inherit from
+  a base class and use the same WebSocket connection Clusterio uses to
+  communicate.
 
 ### Changes
 
@@ -100,6 +103,11 @@ Version 2.0.0
 - Changed the format of database/slaves.json.
 - Removed the output file subscription system.  Breaks inventoryImports,
   playerManager, trainTeleports, serverSelect and researchSync.
+- Removed the factorioOutput hook from instance plugins.  The onOutput
+  hook provides parsed output instead.
+- Removed the onLoadFinish hook from master plugins.
+- Moved plugins from the sharedPlugins directory to plugins directory.
+- Implemented a new plugin system that replaces the old.  Breaks all plugins.
 
 
 Version 1.2.4

client.js

@@ -8,6 +8,7 @@ const fileOps = require("lib/fileOps");
 const hashFile = require('lib/hash').hashFile;
 const factorio = require("lib/factorio");
 const link = require("lib/link");
+const plugin = require("lib/plugin");
 const errors = require("lib/errors");
 
 
@@ -20,6 +21,8 @@ class Instance extends link.Link{
 		link.attachAllMessages(this);
 		this._dir = dir;
 
+		this.plugins = new Map();
+
 		// This is expected to change with the config system rewrite
 		this.config = {
 			id: instanceConfig.id,
@@ -41,11 +44,34 @@ class Instance extends link.Link{
 
 		this.server.on('output', (output) => {
 			link.messages.instanceOutput.send(this, { instance_id: this.config.id, output })
+
+			for (let [name, plugin] of this.plugins) {
+				plugin.onOutput(output).catch(err => {
+					console.error(`Plugin ${name} raised error in onOutput:`, err);
+				});
+			}
 		});
 	}
 
-	async init() {
+	async init(pluginInfos) {
 		await this.server.init();
+
+		// load plugins
+		for (let pluginInfo of pluginInfos) {
+			if (!pluginInfo.enabled || !pluginInfo.instanceEntrypoint) {
+				continue;
+			}
+
+			// require plugin class and initialize it
+			let pluginLoadStarted = Date.now();
+			let { InstancePlugin } = require(`./plugins/${pluginInfo.name}/${pluginInfo.instanceEntrypoint}`);
+			let instancePlugin = new InstancePlugin(pluginInfo, this);
+			await instancePlugin.init();
+			this.plugins.set(pluginInfo.name, instancePlugin);
+			plugin.attachPluginMessages(this, pluginInfo, instancePlugin);
+
+			console.log(`Clusterio | Loaded plugin ${pluginInfo.name} in ${Date.now() - pluginLoadStarted}ms`);
+		}
 	}
 
 	/**
@@ -133,8 +159,8 @@ class Instance extends link.Link{
 		// plugins that are not disabled.  This will most likely change in the
 		// future when the plugin refactor is done.
 		let modules = [];
-		for (let pluginName of await fs.readdir("sharedPlugins")) {
-			let pluginDir = path.join("sharedPlugins", pluginName);
+		for (let pluginName of await fs.readdir("plugins")) {
+			let pluginDir = path.join("plugins", pluginName);
 			if (await fs.pathExists(path.join(pluginDir, "DISABLED"))) {
 				continue;
 			}
@@ -177,6 +203,10 @@ class Instance extends link.Link{
 		});
 
 		await this.server.start(saveName);
+
+		for (let pluginInstance of this.plugins.values()) {
+			await pluginInstance.onStart();
+		}
 	}
 
 	/**
@@ -185,6 +215,10 @@ class Instance extends link.Link{
 	async stop() {
 		// XXX this needs more thought to it
 		if (this.server._state === "running") {
+			for (let pluginInstance of this.plugins.values()) {
+				await pluginInstance.onStop();
+			}
+
 			await this.server.stop();
 		}
 	}
@@ -295,6 +329,10 @@ class InstanceConnection extends link.Link {
 		super('slave', 'instance', connector);
 		this.slave = slave;
 		link.attachAllMessages(this);
+
+		for (let pluginInfo of slave.pluginInfos) {
+			plugin.attachPluginMessages(this, pluginInfo, null);
+		}
 	}
 
 	async forwardEventToInstance(message, event) {
@@ -355,9 +393,15 @@ class SlaveConnector extends link.SocketIOClientConnector {
 class Slave extends link.Link {
 	// I don't like God classes, but the alternative of putting all this state
 	// into global variables is not much better.
-	constructor(connector, slaveConfig) {
+	constructor(connector, slaveConfig, pluginInfos) {
 		super('slave', 'master', connector);
 		link.attachAllMessages(this);
+
+		this.pluginInfos = pluginInfos;
+		for (let pluginInfo of pluginInfos) {
+			plugin.attachPluginMessages(this, pluginInfo, null);
+		}
+
 		this.config = {
 			id: slaveConfig.id,
 			name: slaveConfig.name,
@@ -449,7 +493,7 @@ class Slave extends link.Link {
 		let instance = new Instance(
 			connectionClient, instanceInfo.path, this.config.factorioDir, instanceInfo.config
 		);
-		await instance.init();
+		await instance.init(this.pluginInfos);
 
 		// XXX: race condition on multiple simultanious calls
 		this.instanceConnections.set(instanceId, instanceConnection);
@@ -709,7 +753,6 @@ async function startClient() {
 	const slaveConfig = JSON.parse(await fs.readFile(args.config));
 
 	await fs.ensureDir(slaveConfig.instanceDirectory);
-	await fs.ensureDir("sharedPlugins");
 	await fs.ensureDir("sharedMods");
 
 	// Set the process title, shows up as the title of the CMD window on windows
@@ -737,8 +780,9 @@ async function startClient() {
 		return;
 	}
 
+	let pluginInfos = await plugin.getPluginInfos("plugins");
 	let slaveConnector = new SlaveConnector(slaveConfig);
-	let slave = new Slave(slaveConnector, slaveConfig);
+	let slave = new Slave(slaveConnector, slaveConfig, pluginInfos);
 
 	// Handle interrupts
 	let secondSigint = false

docs/writing-plugins.md

@@ -0,0 +1,303 @@
+Writing Plugins
+===============
+
+Plugins for Clusterio are classes written in JavaScript that run under
+Node.js.  The plugin classes have pre-defined hooks that are called
+during various stages and operations of Clusterio.
+
+
+Plugin Structure
+----------------
+
+The basic file structure of a plugin is the following.
+
+    plugin_name/
+      +- info.js
+      +- package.json
+      +- master.js
+      +- instance.js
+      +- test/
+      |  +- plugin.js
+      +- lua/
+         +- plugin.lua
+
+The most important file is the `info.js` file.  Without it the plugin
+will not recognized by Clusterio.  Here's an example of it:
+
+    const link = require('lib/link'); // For messages
+
+    module.exports = {
+        name: "foo_frobber",
+        title: "Foo Frobber",
+        description: "Does advance frobnication",
+        version: "0.8.1",
+        instanceEntrypoint: "instance",
+        masterEntrypoint: "master",
+        messages: {
+            /* See below */
+        },
+    };
+
+The following properties are recognized:
+
+**name**:
+    Internal name of the plugin.  Must be the same as the plugin's
+    directory name
+
+**title**:
+    Name of the plugin as shown to users.  Currently not used.
+
+**description**:
+    Brief description of what the plugin does.  Currently not used.
+
+**version**:
+    Semver compatible version string of the plugin.  Currently not used.
+
+**instanceEntrypoint**:
+    Path to a Node.js module relative to the plugin directory which
+    contains the InstancePlugin class definition for this plugin.  This
+    is an optional paramater.  A plugin can be for instances only but
+    for it to be able to send any messages to other instances it will
+    still have to be loaded on the master server.
+
+**masterEntrypoint**:
+    Path to a Node.js module relative to the plugin directory which
+    contains the MasterPlugin class definiton for this plugin.  This is
+    an optional parameter.  A plugin can be made that only runs on the
+    master server.
+
+**messages**:
+    Object with link messages definitions for this plugin.  See guide
+    for [defining link messages](#defining-link-messages) below.
+
+While there is no standard for how to organize a plugin it's recommended
+to put the MasterPlugin class definition into master.js and the
+InstancePlugin class definition into instance.js.  You can put them into
+whatever file you want (even the same one for both)
+
+For both instanceEntrypoint and masterEntrypoint the path should not end
+with .js and it should use forward slashes for directory sepparators if
+any.
+
+
+Defining the plugin class
+-------------------------
+
+The plugin class should derive from its respective base class defined in
+`lib/plugin`.  For example to define a MasterPlugin class the following
+code can be used:
+
+    const plugin = require('lib/plugin');
+
+    class MasterPlugin extends plugin.BaseMasterPlugin {
+        async init() {
+            this.foo = 42;
+            await this.startFrobnication();
+        }
+
+        // ...
+    }
+
+    module.exports = {
+        MasterPlugin,
+    }
+
+For the instance plugin it's exactly the same except "Master" is
+replaced with "Instance".  The available hooks that you can override is
+documented on the base class [in lib/plugin.js](../lib/plugin.js).
+
+It's best to avoid defining a constructor, but if you insist on defining
+one forward all arguments to be the base class.  E.g.:
+
+        constructor(...args) {
+            super(...args);
+
+            // Code here
+        }
+
+The arguments passed may change, and attempting to modify them will
+result in unpredicatable behaviour.  The async init method always called
+immediatly after the constructor so there's little reason to do this.
+
+
+Defining Link Messages
+----------------------
+
+You will most likely have to communicate with the master or other
+instances in your plugin for it to do anything useful.  For this there's
+a WebSocket communication channel established between the slaves and the
+master server that plugins can define their own messages to send over
+it.  This channel is bi-directional and all messages send over it are
+validated with a JSON schema (see [this guide][guide] for an
+introduction to writing JSON schema.)
+
+[guide]: https://json-schema.org/learn/getting-started-step-by-step.html
+
+There are currently two kinds of messages that can be defined.  Events
+and requests.  Events are simple one way notifications that invoke a
+handler on target it's sent to.  Requests are pairs of request and
+response messages where the request is sent to the target and the
+response is the reply back from the target.  The requests are similar to
+HTTP requests only that both parties of a link may innitiate one.
+
+
+### Defining Events
+
+Events are defined as properties of the messages object exported by
+`info.js` that map to instances of the `Event` class from `lib/link`.
+The name of the property correspond to the handler invoked on the plugin
+class.  The Event constructor takes an object of properties that define
+the event, for example the following could be defined in `info.js`:
+
+    messages: {
+        startFrobnication: new link.Event({
+            type: 'foo_frobber:start_frobnication',
+            links: ['master-slave', 'slave-instance'],
+            forwardTo: 'instance',
+            eventProperties: {
+                "frobnication_type": { type: "string" },
+            },
+        }),
+    },
+
+This specifies an event that can be sent from the master to a slave,
+and from a slave to an instance.  And that the event must contain the
+property `frobnication_type` with a string value in the data payload.
+It will also be forwarded by slaves to a specific instance.
+
+The following properties are recognized by the Event constructor:
+
+**type**:
+    The message type sent over the wire.  This can be any string but
+    it must be unique across all plugins and it's recommended that it is
+    of format `plugin_name:message_name`.  The suffix `_event` will be
+    appened to the type.
+
+**links**:
+    An array of strings describing which links this event can be sent
+    over.  Direction matters, `'master-slave'` means the event can be
+    sent from the master to slave, but can't be sent back the other way
+    unless `'slave-master'` is also present in the links array.
+
+**forwardTo**:
+    Target to forward event to.  Can be either `'master'` to indicate a
+    slave should forward it to the master server or `'instance'` to
+    indicate it should be forwarded to the instances specified by the
+    `instance_id` event property.  This works by using a default handler
+    for the event at the links that forward it.
+
+**broadcastTo**:
+    Target to broadcast this message towards.  Currently only
+    `'instance'` is supported and means the event will be broadcast to
+    all instances downstream of the target it's sent to, but not back
+    from where it came from.  This means that sending an event to a
+    slave from an instance will cause it to be broadcast to all
+    instances of that slave except for the instance it came from.
+
+**eventProperties**:
+    Object with properties mapping to a JSON schema of that property
+    that specifies what's valid to send in the event.  This is
+    equivalent to using the `properties` keyword in JSON schema except
+    that the properties specified are implicitly required and additional
+    properties are not allowed.  See [this guide][guide] for an
+    introduction to writing JSON schemas
+
+The forwardTo and broadcastTo can be combined such that specifying
+`'master'` as the forwardTo value and `'instance'` as the broadcastTo
+value will cause the event to be broadcast to all instances in the
+cluster.  For this to work you will need to specify instance-slave,
+slave-master and master-slave, slave-instance as the links.
+
+Keep in mind when forwarding events that if the target an event is being
+forwarded to is not online the event will be dropped.  If you need a
+confirmation that the message was received use a request.
+
+
+### Definining Requests
+
+Requests are defined as properties of the messages object exported by
+`info.js` that map to instances of the `Request` class from `lib/link`.
+The name of the property correspond to the handler invoked on the plugin
+class.  The Request constructor takes an object of properties that define
+the event, for example the following could be defined in `info.js`:
+
+    messages: {
+        reportFrobnication: new link.Request({
+            type: 'foo_frobber:report_frobnication',
+            links: ['master-slave', 'slave-instance'],
+            forwardTo: 'instance',
+            requestProperties: {
+                "verbosity": { type: "integer" },
+            },
+            responseProperties: {
+                "report": {
+                    type: "array",
+                    items: { type: "string" },
+                },
+            },
+        }),
+    },
+
+This specifies a request that can be sent from the master to a slave,
+and from a slave to an instance.  The request data must contain the
+property `verbosity` with an integer number as the value as well as the
+`instance_id` property (implied by `forwardTo: 'instance'`), and the
+response sent must contain a `report` property mapping to an array of
+strings.  When received by a slave it will also be forwarded to the
+instance specified by `instance_id`.
+
+The following properties are recognized by the Request constructor:
+
+**type**:
+    The message type sent over the wire.  This can be any string but
+    it must be unique across all plugins and it's recommended that it is
+    of format `plugin_name:message_name`.  The suffix `_request` will be
+    appened to the type for the request message sent, and the suffix
+    `_response` will be appended to the type for the response.
+
+**links**:
+    An array of strings describing which links this request can be sent
+    over.  Direction matters, `'master-slave'` means the request can be
+    sent from the master to slave and the slave can reply to the master,
+    but the slave can't sent a request to the master unless
+    `'slave-master'` is also present in the links array.
+
+**forwardTo**:
+    Target to forward request to.  Can be either `'master'` to indicate
+    a slave should forward it to the master server when receiving it
+    from an instance or `'instance'` to indicate it should be forwarded
+    to the instances specified by the `instance_id` event property.
+    This works by using a default handler for the event at the links
+    that forward it.
+
+**requestProperties**:
+    Object with properties mapping to a JSON schema of that property
+    that specifies what's valid to send in the request.  This is
+    equivalent to using the `properties` keyword in JSON schema except
+    that the properties specified are implicitly required and additional
+    properties are not allowed.  See [this guide][guide] for an
+    introduction to writing JSON schemas
+
+**responseProperties**:
+    Same as the requestProperties only for the response sent back by the
+    target.
+
+
+Sending Link Messages
+---------------------
+
+Link messages are sent by calling the `.send()` method on the
+Event/Request instance with the link you want to send it over and the
+data to send.  For instance plugins the link is the instance itself
+which is accessible through the `.instance` property on the plugin.  The
+`.info` property of the plugin class exposes the data exported from the
+plugin's `info.js` module.  In other words:
+
+    // In an InstancePlugin class
+    async frobnicate() {
+        this.info.messages.exampleEvent.send(this.instance, { foo: "bar" });
+    }
+
+For the Request class the send method is async and returns the response
+data received from the target it was sent to, or throws an error if the
+request failed.

lib/errors.js

@@ -15,10 +15,20 @@ class StartupError extends Error { }
 // Errors outside of our control.
 class EnvironmentError extends Error { }
 
+// Errors caused by plugins
+class PluginError extends Error {
+	constructor(pluginName, original) {
+		super(`PluginError: ${original.message}`);
+		this.pluginName = pluginName;
+		this.original = original;
+	}
+}
+
 module.exports = {
 	CommandError,
 	RequestError,
 	InvalidMessage,
 	StartupError,
 	EnvironmentError,
+	PluginError,
 }

lib/plugin.js

@@ -0,0 +1,126 @@
+"use strict";
+const fs = require("fs-extra");
+const path = require("path");
+
+const errors = require("lib/errors");
+
+
+/**
+ * Base class for instance plugins
+ */
+class BaseInstancePlugin {
+	constructor(info, instance) {
+		this.info = info;
+		this.instance = instance;
+	}
+
+	/**
+	 * Called immediately after the class is instantiated
+	 */
+	async init() { }
+
+	/**
+	 * Called after the Factorio server is started
+	 */
+	async onStart() { }
+
+	/**
+	 * Called before the Factorio server is stopped
+	 *
+	 * This will not be called if for example the Factorio server crashes or
+	 * is killed.
+	 */
+	async onStop() { }
+
+	/**
+	 * Called when the instance is shutting down
+	 */
+	async onExit() { }
+
+	/**
+	 * Called when the Factorio outputs a line
+	 *
+	 * Called when the Factorio server outputs a line on either stdout or
+	 * stderr.  The output is a parsed form of the line and contains the
+	 * following properties (those marked with ? are optional):
+	 *
+	 * - source: Where the message came from, one of "stdout" and "stderr".
+	 * - format: Timestamp format, one of "date", "seconds" and "none".
+	 * - time?: Timestamp of the message.  Not present if format is "none".
+	 * - type: Type of message, one of "log", "action" and "generic".
+	 * - level?: Log level for "log" type.  i.e "Info" normally.
+	 * - file?: File reported for "log" type.
+	 * - action?: Kind of action for "action" type. i.e "CHAT" for chat.
+	 * - message: Main content of the line.
+	 */
+	async onOutput(output) { }
+}
+
+class BaseMasterPlugin {
+	constructor(info) {
+		this.info = info;
+	}
+
+	/**
+	 * Called immediately after the class is instantiated
+	 */
+	async init() { }
+
+	/**
+	 * Called when the master server is shutting down
+	 */
+	async onExit() { }
+}
+
+async function getPluginInfos(baseDir) {
+	let plugins = [];
+	for (let pluginDir of await fs.readdir(baseDir)) {
+		let pluginInfo;
+
+		try {
+			// Note: Require path is relative to this module
+			pluginInfo = require(path.posix.join('..', baseDir, pluginDir, 'info'));
+
+		} catch (err) {
+			if (err.code === 'MODULE_NOT_FOUND') {
+				continue;
+			} else {
+				throw new errors.PluginError(pluginDir, err);
+			}
+		}
+
+		if (pluginInfo.name !== pluginDir) {
+			throw new errors.EnvironmentError(
+				`Plugin dir ${baseDir}/${pluginDir} does not match the name of the plugin (${pluginInfo.name})`
+			);
+		}
+
+		pluginInfo.enabled = !await fs.exists(path.join(baseDir, pluginDir, "DISABLED"));
+		plugins.push(pluginInfo);
+	}
+	return plugins;
+}
+
+function attachPluginMessages(link, pluginInfo, plugin) {
+	let messageDefinitions = pluginInfo.messages || [];
+	for (let [name, messageFormat] of Object.entries(messageDefinitions)) {
+		let handler = name + messageFormat.constructor.name + 'Handler';
+		if (plugin === null || !plugin[handler]) {
+			messageFormat.attach(link);
+
+		} else {
+			messageFormat.attach(link, async function(message, format) {
+				return await plugin[handler](message, format, this);
+			});
+		}
+	}
+}
+
+
+module.exports = {
+	BaseInstancePlugin,
+	BaseMasterPlugin,
+
+	getPluginInfos,
+	attachPluginMessages,
+};

master.js

@@ -37,6 +37,7 @@ const factorio = require("lib/factorio");
 const schema = require("lib/schema");
 const link = require("lib/link");
 const errors = require("lib/errors");
+const plugin = require("lib/plugin");
 
 // homemade express middleware for token auth
 const authenticate = require("lib/authenticate");
@@ -69,7 +70,6 @@ app.set('views', ['views', 'sharedPlugins']);
 app.use(function(req, res, next){
 	res.locals.res = res;
 	res.locals.req = req;
-	res.locals.masterPlugins = masterPlugins;
 	res.locals.slaves = db.slaves;
 	res.locals.moment = moment;
 	next();
@@ -142,10 +142,8 @@ function registerMoreMetrics(){
 	}
 
 	// plugins
-	for (let plugin of masterPlugins) {
-		if (plugin.main && plugin.main.onMetrics && typeof plugin.main.onMetrics == "function") {
-			plugin.main.onMetrics();
-		}
+	for (let masterPlugin of masterPlugins.values()) {
+		masterPlugin.onMetrics();
 	}
 
 	// Slave count
@@ -186,13 +184,10 @@ async function shutdown() {
 		await saveMap(config.databaseDirectory, "slaves.json", db.slaves);
 		await saveMap(config.databaseDirectory, "instances.json", db.instances);
 
-		for(let i in masterPlugins){
-			let plugin = masterPlugins[i];
-			if(plugin.main && plugin.main.onExit && typeof plugin.main.onExit == "function"){
-				let startTime = Date.now();
-				await plugin.main.onExit();
-				console.log("Plugin "+plugin.pluginConfig.name+" exited in "+(Date.now()-startTime)+"ms");
-			}
+		for (let [pluginName, masterPlugin] of masterPlugins) {
+			let startTime = Date.now();
+			await masterPlugin.onExit();
+			console.log(`Plugin ${pluginName} exited in ${Date.now() - startTime}ms`);
 		}
 		console.log("Clusterio cleanly exited in "+(Date.now()-exitStartTime)+"ms");
 		process.exit(0);
@@ -252,6 +247,9 @@ class BaseConnection extends link.Link {
 	constructor(target, connector) {
 		super('master', target, connector);
 		link.attachAllMessages(this);
+		for (let masterPlugin of masterPlugins.values()) {
+			plugin.attachPluginMessages(this, masterPlugin.info, masterPlugin);
+		}
 	}
 
 	async forwardRequestToInstance(message, request) {
@@ -518,13 +516,43 @@ io.on('connection', function (socket) {
 });
 
 // handle plugins on the master
-var masterPlugins = [];
+var masterPlugins = new Map();
 async function pluginManagement(){
 	let startPluginLoad = Date.now();
-	// masterPlugins = await getPlugins();
+	masterPlugins = await loadPlugins();
 	console.log("All plugins loaded in "+(Date.now() - startPluginLoad)+"ms");
 }
 
+async function loadPlugins() {
+	let plugins = new Map();
+	for (pluginInfo of await plugin.getPluginInfos("plugins")) {
+		if (!pluginInfo.enabled) {
+			continue;
+		}
+
+		let pluginLoadStarted = Date.now();
+		let MasterPlugin = plugin.BaseMasterPlugin;
+		if (pluginInfo.masterEntrypoint) {
+			({ MasterPlugin } = require(`./plugins/${pluginInfo.name}/${pluginInfo.masterEntrypoint}`));
+		}
+
+		let masterPlugin = new MasterPlugin(pluginInfo);
+		await masterPlugin.init();
+		plugins.set(pluginInfo.name, masterPlugin);
+
+		console.log(`Clusterio | Loaded plugin ${pluginInfo.name} in ${Date.now() - pluginLoadStarted}ms`);
+		/*
+			main:new masterPlugin({
+			TODO?
+				config, socketio: io, express: app,
+				db, Prometheus, prometheusPrefix, endpointHitCounter,
+			}),
+			pluginConfig,
+		});*/
+	}
+	return plugins;
+}
+
 /**
  * Calls listen on server capturing any errors that occurs
  * binding to the port.
@@ -666,6 +694,13 @@ I           version of clusterio.  Expect things to break. I
 | Unable to to start master server |
 +----------------------------------+`
 			);
+		} else if (err instanceof errors.PluginError) {
+			console.error(`
+Error: ${err.pluginName} plugin threw an unexpected error
+       during startup, please report it to the plugin author.
+--------------------------------------------------------------`
+			);
+			err = err.original;
 		} else {
 			console.error(`
 +---------------------------------------------------------------+

package.json

@@ -5,7 +5,7 @@
   "repository": "https://github.com/clusterio/factorioClusterio",
   "main": "master.js",
   "scripts": {
-    "test": "mocha --exit --check-leaks -R spec --recursive test sharedPlugins/*/test",
+    "test": "mocha --exit --check-leaks -R spec --recursive test plugins/*/test",
     "fast-test": "FAST_TEST=y npm test",
     "cover": "nyc npm test",
     "fast-cover": "FAST_TEST=y nyc npm test",
@@ -17,7 +17,7 @@
   "nyc": {
     "exclude": [
       "test/**",
-      "sharedPlugins/*/test/**"
+      "plugins/*/test/**"
     ]
   },
   "keywords": [

test/client.js

@@ -8,17 +8,20 @@ const client = require("../client");
 describe("Client testing", function() {
 	describe("class Instance", function() {
 		let instance = new client._Instance(new link.VirtualConnector(), "dir", "factorioDir", { name:"foo" });
-		it("should give the name on .name", function() {
-			assert.equal(instance.name, "foo");
-		})
+		describe(".name", function() {
+			it("should give the name of the instance", function() {
+				assert.equal(instance.name, "foo");
+			})
+		});
 
-		it("should give the path to it on .path()", function() {
-			assert.equal(instance.path(), "dir");
-		})
-
-		it("should join path on .path(...parts)", function() {
-			assert.equal(instance.path("bar"), path.join("dir", "bar"));
-		})
+		describe(".path()", function() {
+			it("should give the path when called without arguments", function() {
+				assert.equal(instance.path(), "dir");
+			})
+			it("should join path with arguments", function() {
+				assert.equal(instance.path("bar"), path.join("dir", "bar"));
+			})
+		});
 	});
 
 	describe("checkFilename()", function() {

test/integration/clusterio.js

@@ -3,6 +3,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const link = require("lib/link");
+const plugin = require("lib/plugin");
 const master = require("../../master");
 const client = require("../../client");
 const clusterctl = require("../../clusterctl");
@@ -62,7 +63,7 @@ describe("Integration of Clusterio", function() {
 			masterURL: "http://invalid",
 			masterAuthToken: "invalid",
 			publicIP: "invalid",
-		});
+		}, await plugin.getPluginInfos("plugins"));
 		testSlaveConnection = new master._SlaveConnection({ agent: "test", version, name: "slave", id: 4}, slaveServer);
 		master._slaveConnections.set(4, testSlaveConnection);
 	});

test/lib/plugin.js

@@ -0,0 +1,124 @@
+const assert = require('assert').strict;
+const fs = require('fs-extra');
+const path = require('path');
+
+const mock = require('../mock');
+const link = require('lib/link');
+const plugin = require('lib/plugin');
+const errors = require('lib/errors');
+
+
+describe("lib/plugin", function() {
+	describe("class BaseInstancePlugin", function() {
+		let instancePlugin;
+		it("should be constructible", async function() {
+			instancePlugin = new plugin.BaseInstancePlugin();
+			await instancePlugin.init();
+		})
+		it("should define defaults for hooks", async function() {
+			await instancePlugin.onStart();
+			await instancePlugin.onStop();
+			await instancePlugin.onExit();
+			await instancePlugin.onOutput({});
+		})
+	});
+
+	describe("class BaseMasterPlugin", function() {
+		let masterPlugin;
+		it("should be constructible", async function() {
+			masterPlugin = new plugin.BaseMasterPlugin();
+			await masterPlugin.init();
+		})
+		it("should define defaults for hooks", async function() {
+			await masterPlugin.onExit();
+		})
+	});
+
+	describe("getPluginInfos()", function() {
+		let baseDir = path.join('test', 'temp', 'plugin');
+		let emptyDir = path.join(baseDir, 'emptyDir');
+		let emptyPlugin = path.join(baseDir, 'emptyPlugin');
+		let testPlugin = path.join(baseDir, 'testPlugin');
+		let disabledPlugin = path.join(baseDir, 'disabledPlugin');
+		let brokenPlugin = path.join(baseDir, 'brokenPlugin');
+		let invalidPlugin = path.join(baseDir, 'invalidPlugin');
+		before(async function() {
+			await fs.ensureDir(emptyDir);
+			await fs.ensureDir(path.join(emptyPlugin, 'empty'));
+
+			async function writePlugin(pluginPath, name, infoName = name) {
+				await fs.outputFile(
+					path.join(pluginPath, name, 'info.js'),
+					`module.exports = { name: "${infoName}" };`
+				);
+			}
+
+			await writePlugin(testPlugin, 'test');
+			await writePlugin(disabledPlugin, 'disabled');
+			await fs.outputFile(path.join(disabledPlugin, 'disabled', 'DISABLED'), "");
+			await writePlugin(brokenPlugin, 'broken');
+			await fs.outputFile(path.join(brokenPlugin, 'broken', 'info.js'), "Syntax Error");
+			await writePlugin(invalidPlugin, 'invalid', 'wrong');
+		});
+
+		it("should return an empty array for an empty directory", async function() {
+			assert.deepEqual(await plugin.getPluginInfos(emptyDir), []);
+		});
+		it("should ignore plugin dirs without info module", async function() {
+			assert.deepEqual(await plugin.getPluginInfos(emptyPlugin), []);
+		});
+		it("should discover test plugin", async function() {
+			assert.deepEqual(
+				await plugin.getPluginInfos(testPlugin),
+				[{ name: 'test', enabled: true }]
+			);
+		});
+		it("should discover disabled plugin", async function() {
+			assert.deepEqual(
+				await plugin.getPluginInfos(disabledPlugin),
+				[{ name: 'disabled', enabled: false }]
+			);
+		});
+		it("should discover disabled plugin", async function() {
+			assert.deepEqual(
+				await plugin.getPluginInfos(disabledPlugin),
+				[{ name: 'disabled', enabled: false }]
+			);
+		});
+		it("should reject on broken plugin", async function() {
+			await assert.rejects(
+				plugin.getPluginInfos(brokenPlugin),
+				{ message: "PluginError: Unexpected identifier" }
+			);
+		});
+		it("should reject on invalid plugin", async function() {
+			await assert.rejects(
+				plugin.getPluginInfos(invalidPlugin),
+				{ message: `Plugin dir ${invalidPlugin}/invalid does not match the name of the plugin (wrong)` }
+			);
+		});
+
+		after(async function() {
+			await fs.unlink(path.join(brokenPlugin, 'broken', 'info.js'));
+		});
+	});
+
+	describe("attachPluginMessages()", function() {
+		let mockLink = new link.Link('source', 'target', new mock.MockConnector());
+		let mockEvent = new link.Event({ type: 'test', links: ['target-source'] });
+		it("should accept pluginInfo without messages", function() {
+			plugin.attachPluginMessages(mockLink, {}, null);
+		});
+		it("should attach handler for the given message", function() {
+			let mockEventEventHandler = function() {};
+			plugin.attachPluginMessages(mockLink, { messages: { mockEvent }}, { mockEventEventHandler });
+			assert(mockLink._handlers.get('test_event'), "handler was not registered");
+		});
+		it("should throw if missing handler for the given message", function() {
+			assert.throws(
+				() => plugin.attachPluginMessages(mockLink, { messages: { mockEvent }}, {}),
+				new Error("Missing handler for test_event on source-target link")
+			);
+		});
+	});
+});