Merge pull request #661 from Hornwitser/windows-rework

Rework Factorio server stop logic

test/host/server.js

@@ -211,27 +211,28 @@ describe("host/server", function() {
 
 		describe(".stop()", function() {
 			it("should handle server quitting on its own during stop", async function() {
-				if (process.platform === "win32") {
-					this.skip();
-				}
-				server.hangTimeoutMs = 20;
+				server.shutdownTimeoutMs = 20;
 				server._server = new events.EventEmitter();
 				server._server.kill = () => true;
 				server._state = "running";
-				server._rconReady = true;
+				server._rconReady = false;
 				server._rconClient = {
+					async sendRcon() { },
 					async end() {
 						server._rconClient = null;
-						process.nextTick(() => {
-							server.emit("_quitting");
-							server._server.emit("exit");
-						});
 					},
 				};
 				server._watchExit();
 
-				await server.stop();
-				await wait(21); // Wait until after hang timeout
+				const stop = server.stop();
+				stop.catch(() => {});
+				process.nextTick(() => {
+					server.emit("rcon-ready");
+					server._server.emit("exit");
+				});
+
+				await stop;
+				await wait(21); // Wait until after shutdown timeout
 			});
 		});
 	});

test/integration/clusterio.js

@@ -13,7 +13,7 @@ const { wait } = lib;
 const testStrings = require("../lib/factorio/test_strings");
 const {
 	TestControl, TestControlConnector, url, controlToken, slowTest,
-	get, exec, execCtl, sendRcon, getControl, spawn, instancesDir,
+	exec, execCtl, sendRcon, getControl, spawn, instancesDir, factorioDir,
 } = require("./index");
 
 
@@ -41,15 +41,29 @@ async function startAltHost() {
 	await execCtl(`host create-config --id 5 --name alt-host --generate-token --output ${config}`);
 	await exec(`node ../../packages/host --config ${config} config set host.tls_ca ../../test/file/tls/cert.pem`);
 	await exec(`node ../../packages/host --config ${config} config set host.instances_directory alt-instances`);
-	await exec(`node ../../packages/host --config ${config} config set host.factorio_directory ../../factorio`);
+	const dir = path.isAbsolute(factorioDir) ? factorioDir : path.join("..", "..", factorioDir);
+	await exec(`node ../../packages/host --config ${config} config set host.factorio_directory ${dir}`);
 	await exec(`node ../../packages/host --config ${config} config set host.mods_directory alt-mods`);
 	return await spawnAltHost(config);
 }
 
 async function stopAltHost(hostProcess) {
 	if (hostProcess && hostProcess.exitCode === null) {
-		hostProcess.kill("SIGINT");
-		await events.once(hostProcess, "exit");
+		const waitForExit = lib.timeout(
+			events.once(hostProcess, "exit"),
+			10e3,
+			"timeout"
+		).catch(() => {});
+		try {
+			await execCtl("host stop 5");
+		} catch (err) {
+			// Some tests cause the host to stop
+		}
+		if (await waitForExit === "timeout" && hostProcess.exitCode === null) {
+			// eslint-disable-next-line no-console
+			console.warn("Stopping alt-host failed. Killing.");
+			hostProcess.kill("SIGINT");
+		}
 	}
 }
 
@@ -148,12 +162,6 @@ describe("Integration of Clusterio", function() {
 	describe("clusteriohost", function() {
 		describe("hostUpdateEventHandler()", function() {
 			it("should trigger when a new host is added", async function() {
-				// On windows there's currently no way to automate graceful shutdown of the host
-				// process as CTRL+C is some weird terminal thing and SIGINT isn't a thing.
-				if (process.platform === "win32") {
-					this.skip();
-				}
-
 				slowTest(this);
 				getControl().hostUpdates = [];
 
@@ -189,12 +197,6 @@ describe("Integration of Clusterio", function() {
 			});
 		});
 		it("should download mods from controller", async function() {
-			// On windows there's currently no way to automate graceful shutdown of the host
-			// process as CTRL+C is some weird terminal thing and SIGINT isn't a thing.
-			if (process.platform === "win32") {
-				this.skip();
-			}
-
 			slowTest(this);
 			await runWithAltHost(async () => {
 				const clusterioLib = path.join("temp", "test", "alt-mods", "clusterio_lib_0.1.2.zip");
@@ -206,12 +208,6 @@ describe("Integration of Clusterio", function() {
 			});
 		});
 		it("should auto start instances with auto_start enabled", async function() {
-			// On windows there's currently no way to automate graceful shutdown of the host
-			// process as CTRL+C is some weird terminal thing and SIGINT isn't a thing.
-			if (process.platform === "win32") {
-				this.skip();
-			}
-
 			slowTest(this);
 
 			let hostProcess;
@@ -310,12 +306,6 @@ describe("Integration of Clusterio", function() {
 		});
 		describe("host revoke-token", async function() {
 			it("should disconnect existing host", async function() {
-				// On windows there's currently no way to automate graceful shutdown of the host
-				// process as CTRL+C is some weird terminal thing and SIGINT isn't a thing.
-				if (process.platform === "win32") {
-					this.skip();
-				}
-
 				slowTest(this);
 				let sawDisconnected;
 				await runWithAltHost(async () => {
@@ -774,11 +764,6 @@ describe("Integration of Clusterio", function() {
 					if (remote) {
 						let hostProcess;
 						before(async function() {
-							// On windows there's currently no way to automate graceful shutdown of the host
-							// process as CTRL+C is some weird terminal thing and SIGINT isn't a thing.
-							if (process.platform === "win32") {
-								this.skip();
-							}
 							slowTest(this);
 
 							hostProcess = await startAltHost();
@@ -907,12 +892,8 @@ describe("Integration of Clusterio", function() {
 				slowTest(this);
 				let statusesToCheck = new Set([
 					"unassigned", "stopped", "creating_save", "exporting_data",
-					"starting", "running", "stopping", "deleted",
+					"starting", "running", "stopping", "deleted", "unknown",
 				]);
-				// Windows does not see the unknown status because alt-host is not spawned.
-				if (process.platform !== "win32") {
-					statusesToCheck.add("unknown");
-				}
 				let statusesNotSeen = new Set(statusesToCheck);
 
 				for (let update of getControl().instanceUpdates) {

test/integration/server.js

@@ -14,9 +14,25 @@ const { slowTest, factorioDir } = require("./index");
 describe("Integration of host/src/server", function() {
 	describe("_getVersion()", function() {
 		it("should get a version from factorio's changelog.txt", async function() {
+			function checkVersion(version) {
+				if (!/^\d+\.\d+\.\d+$/.test(version)) {
+					assert.fail(`Detected version '${version}' does not followed the format x.y.z`);
+				}
+			}
+
 			let version = await _getVersion(path.join(factorioDir, "data", "changelog.txt"));
-			if (!/^\d+\.\d+\.\d+$/.test(version)) {
-				assert.fail(`Detected version '${version}' does not followed the format x.y.z`);
+			if (version !== null) {
+				checkVersion(version);
+			} else {
+				for (let entry of await fs.readdir(factorioDir, { withFileTypes: true })) {
+					if (!entry.isDirectory()) {
+						continue;
+					}
+
+					checkVersion(
+						await _getVersion(path.join(factorioDir, entry.name, "data", "changelog.txt"))
+					);
+				}
 			}
 		});
 	});
@@ -93,7 +109,7 @@ describe("Integration of host/src/server", function() {
 				let mapPath = server.writePath("saves", "test.zip");
 				assert(!await fs.exists(mapPath), "save exist before test");
 
-				await server.create("test.zip");
+				await server.create("test.zip", undefined, { width: 50, height: 50 });
 				assert(await fs.exists(mapPath), "test did not create save");
 			});
 		});
@@ -213,16 +229,79 @@ describe("Integration of host/src/server", function() {
 				server.sendRcon("/c while true do end").catch(() => {});
 				await new Promise((resolve) => setTimeout(resolve, 300));
 				log(".stop() for hang detection");
+				server.shutdownTimeoutMs = 2000;
 				await server.stop();
+				server.shutdownTimeoutMs = 0;
+			});
+			it("should not hang if RCON fails to connect while stopping", async function() {
+				slowTest(this);
+
+				const origWaitForReady = server._waitForReady;
+				server._waitForReady = () => {
+					server._waitForReady = origWaitForReady;
+					server.rconPort -= 1; // Mangle port so it fails to connect.
+					return server._waitForReady();
+				};
+				try {
+					await server.start("test.zip");
+					if (server._rconReady) {
+						assert.fail("Test failed, RCON managed to connect");
+					}
+					await server.stop();
+
+				} finally {
+					if (server._state !== "init") {
+						await server.kill();
+						assert.fail("Server did not stop");
+					}
+				}
+			});
+			it("should not hang if RCON fails to connect before stopping", async function() {
+				slowTest(this);
+
+				const origWaitForReady = server._waitForReady;
+				server._waitForReady = () => {
+					server._waitForReady = origWaitForReady;
+					server.rconPassword = "wrong"; // Mangle password so it fails to authenticate.
+					return server._waitForReady();
+				};
+				try {
+					await server.start("test.zip");
+					if (server._rconReady) {
+						assert.fail("Test failed, RCON managed to connect");
+					}
+					await events.once(server, "rcon-ready");
+					await server.stop();
+
+				} finally {
+					if (server._state !== "init") {
+						await server.kill();
+						assert.fail("Server did not stop");
+					}
+				}
+			});
+			it("should work if RCON has dropped out", async function() {
+				slowTest(this);
+
+				try {
+					await server.start("test.zip");
+					if (!server._rconReady) {
+						await events.once(server, "rcon-ready");
+					}
+					await server._rconClient.end();
+					await server.stop();
+
+				} finally {
+					if (server._state !== "init") {
+						await server.kill();
+						assert.fail("Server did not stop");
+					}
+				}
 			});
 		});
 
 		describe(".start() termination detection", function() {
 			it("should tell if Factorio got killed", async function() {
-				// This does not work on Windows
-				if (process.platform === "win32") {
-					this.skip();
-				}
 				slowTest(this);
 				log(".start() for kill detection");