From 2510d47023cdbe1f703059ee10eb42d34fd758e5 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:20:40 +0000 Subject: [PATCH] Add tests --- packages/controller/src/routes.ts | 2 +- packages/lib/src/config/validators.ts | 6 +- test/lib/config/classes.js | 256 +++++++++++++++++++++----- test/lib/config/migrations.js | 2 +- test/lib/config/validators.test.js | 245 ++++++++++++++++++++++++ test/mock.js | 21 ++- 6 files changed, 476 insertions(+), 56 deletions(-) create mode 100644 test/lib/config/validators.test.js diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index 6b578dbd..634fc461 100644 --- a/packages/controller/src/routes.ts +++ b/packages/controller/src/routes.ts @@ -1,5 +1,5 @@ import type { Application, Request, Response } from "express"; -import type Controller from "./Controller"; +import Controller from "./Controller"; import busboy from "busboy"; import crypto from "crypto"; diff --git a/packages/lib/src/config/validators.ts b/packages/lib/src/config/validators.ts index fa26c7c1..3d12f134 100644 --- a/packages/lib/src/config/validators.ts +++ b/packages/lib/src/config/validators.ts @@ -52,7 +52,11 @@ export function integer(value: number) { } } -/** Value must be a valid file path, it does not need to exist */ +/** + * Value must be a valid file path, it does not need to exist + * On posix this will only error for non string values, which isn't very useful + * However, this could be expanded in the future to check system OS + */ export function filePath(value: string) { try { path.resolve(value); diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 4003d9b6..83e447af 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -41,9 +41,49 @@ describe("lib/config/classes", function() { "test.priv": { access: ["local"], type: "string", optional: true }, "test.cred": { credential: ["local"] }, "test.rdo": { readonly: ["local"] }, + "test.base": { type: "number", initialValue: 1 }, + "test.dependent": { + type: "number", + initialValue: 1, + dependsOn: ["test.base"], + validator(value, config) { + if (value < config.get("test.base")) { + throw new Error("dependent must be >= base"); + } + }, + }, + "test.validated": { + type: "number", + optional: true, + validator(value) { + if (value !== null && value < 0) { + throw new Error("must be positive"); + } + }, + }, + "test.objValidated": { + type: "object", + initialValue: {}, + validator(value) { + if (value && value.bad !== undefined) { + throw new Error("bad key"); + } + }, + }, }; } + const testConfigDefault = Object.fromEntries( + Object.entries(TestConfig.fieldDefinitions).map( + ([fieldName, fieldDef]) => ([ + fieldName, + typeof fieldDef.initialValue === "function" + ? fieldDef.initialValue() + : (fieldDef.initialValue ?? null), + ]) + ) + ); + describe("constructor", function() { it("should construct an instance", function() { let config = new TestConfig("local"); @@ -80,41 +120,37 @@ describe("lib/config/classes", function() { const testInstance = new TestConfig("local", undefined, "filepath"); assert.equal(testInstance.filepath, "filepath"); }); + it("should throw if a required field has no initialValue", function() { + this.skip(); // TODO known bug that was discovered but of of scope for #856 + class BrokenConfig extends lib.Config { + static fieldDefinitions = { + "test.required": { type: "string" }, + }; + } + + assert.throws( + () => new BrokenConfig("local"), + ); + }); }); describe(".toJSON()", function() { it("should serialize a basic config", function() { - let testInstance = new TestConfig("local"); - assert.deepEqual(testInstance.toJSON(), { - "alpha.foo": null, - "beta.bar": {}, - "test.enum": "b", - "test.test": null, - "test.func": 42, - "test.bool": false, - "test.json": {}, - "test.rest": {}, - "test.priv": null, - "test.cred": null, - "test.rdo": null, - }); + const testInstance = new TestConfig("local"); + assert.deepEqual(testInstance.toJSON(), testConfigDefault); }); }); describe(".toRemote()", function() { it("should leave out inaccessible fields", function() { - let testInstance = new TestConfig("local"); - assert.deepEqual(testInstance.toRemote("remote"), { - "alpha.foo": null, - "beta.bar": {}, - "test.enum": "b", - "test.test": null, - "test.func": 42, - "test.bool": false, - "test.json": {}, - "test.rest": {}, - "test.rdo": null, - }); + const testInstance = new TestConfig("local"); + const { "test.cred": _cred, "test.priv": _priv, ...expected } = testConfigDefault; + assert.deepEqual(testInstance.toRemote("remote"), expected); + }); + it("should filter fields when filter argument is provided", function() { + const testInstance = new TestConfig("local"); + const filtered = testInstance.toRemote("local", ["alpha.foo"]); + assert.deepEqual(filtered, { "alpha.foo": null }); }); }); @@ -165,6 +201,7 @@ describe("lib/config/classes", function() { it("should preserve unknown fields when serialized back", function() { let testFields = { + ...testConfigDefault, "extra.blah": true, "extra.spam": "foobar", "alpha.foo": "true", @@ -175,10 +212,6 @@ describe("lib/config/classes", function() { "test.bool": false, "test.json": {}, "test.rest": {}, - "test.priv": null, - "test.cred": null, - "test.rdo": null, - "test.alpha": null, "test.beta": "decay", "test.gamma": 99, }; @@ -245,6 +278,7 @@ describe("lib/config/classes", function() { }); it("should preserve unknown fields", async function() { const testFields = { + ...testConfigDefault, "extra.blah": true, "extra.spam": "foobar", "alpha.foo": "true", @@ -255,10 +289,6 @@ describe("lib/config/classes", function() { "test.bool": false, "test.json": {}, "test.rest": {}, - "test.priv": null, - "test.cred": null, - "test.rdo": null, - "test.alpha": null, "test.beta": "decay", "test.gamma": 99, }; @@ -347,19 +377,10 @@ describe("lib/config/classes", function() { "beta.bar": { value: 30 }, }, false); assert.deepEqual(testInstance.toJSON(), { + ...testConfigDefault, "extra.blah": true, "extra.spam": "baz", - "alpha.foo": null, "beta.bar": { value: 30 }, - "test.bool": false, - "test.enum": "b", - "test.func": 42, - "test.json": {}, - "test.rest": {}, - "test.priv": null, - "test.cred": null, - "test.rdo": null, - "test.test": null, }); }); @@ -709,6 +730,29 @@ describe("lib/config/classes", function() { testInstance.set("test.enum", "b"); assert.deepEqual(testInstance.restartRequired, true, "restart required not set"); }); + it("should run field validator successfully", function() { + testInstance.set("test.validated", 5); + assert.equal(testInstance.get("test.validated"), 5); + }); + + it("should throw if validator fails", function() { + assert.throws( + () => testInstance.set("test.validated", -1), + new lib.InvalidValue("Failed validation of test.validated: must be positive") + ); + }); + it("should validate dependent fields", function() { + testInstance.set("test.dependent", 10); + testInstance.set("test.base", 5); + assert.equal(testInstance.get("test.dependent"), 10); + assert.equal(testInstance.get("test.base"), 5); + }); + it("should throw if dependent validation fails", function() { + assert.throws( + () => testInstance.set("test.base", 5), + new lib.InvalidValue("Failed validation of dependent test.dependent: dependent must be >= base") + ); + }); }); describe(".setProp()", function() { @@ -786,6 +830,130 @@ describe("lib/config/classes", function() { testInstance.setProp("test.rest", "test", true); assert.deepEqual(testInstance.restartRequired, true, "restart required not set"); }); + it("should run validator when setting object property", function() { + testInstance.setProp("test.objValidated", "good", true); + assert.deepEqual(testInstance.get("test.objValidated"), { good: true }); + }); + it("should throw if object validator fails", function() { + assert.throws( + () => testInstance.setProp("test.objValidated", "bad", true), + new lib.InvalidValue("Failed validation of test.objValidated: bad key") + ); + }); + }); + + describe("get .staging", function() { + it("should create staging config and reuse it", function() { + const testInstance = new TestConfig("local"); + + const first = testInstance.staging; + const second = testInstance.staging; + + assert.equal(first, second); + }); + it("should return new instance if invalided", function() { + const testInstance = new TestConfig("local"); + + const first = testInstance.staging; + testInstance.set("alpha.foo", "value"); + const second = testInstance.staging; + + assert.notEqual(first, second); + }); + }); + + describe(".stage()", function() { + let testInstance; + beforeEach(function() { + testInstance = new TestConfig("local"); + }); + + it("should stage a value without committing", function() { + testInstance.stage("alpha.foo", "value"); + + assert.equal(testInstance.get("alpha.foo"), null); + assert.equal(testInstance.staging.get("alpha.foo"), "value"); + }); + it("should throw if field does not exist", function() { + assert.throws( + () => testInstance.stage("invalid", "value"), + new lib.InvalidField("No field named 'invalid'") + ); + }); + }); + + describe(".stageProp()", function() { + let testInstance; + beforeEach(function() { + testInstance = new TestConfig("local"); + }); + + it("should stage object property change", function() { + testInstance.stageProp("test.json", "prop", true); + + assert.deepEqual(testInstance.get("test.json"), {}); + assert.deepEqual(testInstance.staging.get("test.json"), { prop: true }); + }); + it("should delete property when value undefined", function() { + testInstance.set("test.json", { a: 1 }); + + testInstance.stageProp("test.json", "a", undefined); + + assert.deepEqual(testInstance.staging.get("test.json"), {}); + assert.deepEqual(testInstance.get("test.json"), { a: 1 }); + }); + it("should throw if field is not object", function() { + assert.throws( + () => testInstance.stageProp("test.enum", "prop", true), + new lib.InvalidValue("Cannot set property on non-object field 'test.enum'") + ); + }); + }); + + describe(".commitStaging()", function() { + let testInstance; + beforeEach(function() { + testInstance = new TestConfig("local"); + }); + + it("should do nothing if staging does not exist", function() { + testInstance.commitStaging(); + assert.equal(testInstance.get("alpha.foo"), null); + }); + it("should commit staged values", function() { + testInstance.stage("alpha.foo", "value"); + + assert.equal(testInstance.get("alpha.foo"), null); + testInstance.commitStaging(); + assert.equal(testInstance.get("alpha.foo"), "value"); + }); + it("should throw if staged validation fails", function() { + testInstance.stage("test.validated", -1); + + assert.throws( + () => testInstance.commitStaging(), + new lib.InvalidValue("Failed validation of test.validated: must be positive") + ); + }); + it("should commit stages values with dependent validation", function() { + assert.throws(() => testInstance.set("test.base", 10)); + testInstance.stage("test.base", 10); + testInstance.stage("test.dependent", 15); + // The above ordering would error if set was used + testInstance.commitStaging(); + assert.equal(testInstance.get("test.base"), 10); + assert.equal(testInstance.get("test.dependent"), 15); + }); + }); + + describe(".revertStaging()", function() { + it("should discard staged changes", function() { + const testInstance = new TestConfig("local"); + testInstance.stage("alpha.foo", "value"); + testInstance.revertStaging(); + assert.equal(testInstance.staging.get("alpha.foo"), null); + assert.equal(testInstance.get("alpha.foo"), null); + }); }); }); }); diff --git a/test/lib/config/migrations.js b/test/lib/config/migrations.js index e18719ff..cb88c6d7 100644 --- a/test/lib/config/migrations.js +++ b/test/lib/config/migrations.js @@ -4,7 +4,7 @@ const assert = require("assert").strict; // Each migration test should indicate which version introduced the migration -describe("lib/config/migrations", function() { +describe("lib/config/definitions/migrations", function() { describe("Controller Config", function() { it("should migrate 'external_address' to 'public_url'", function() { // Alpha 19 const config = lib.ControllerConfig.fromJSON({ diff --git a/test/lib/config/validators.test.js b/test/lib/config/validators.test.js new file mode 100644 index 00000000..876d95c5 --- /dev/null +++ b/test/lib/config/validators.test.js @@ -0,0 +1,245 @@ +"use strict"; +const assert = require("assert").strict; +const lib = require("@clusterio/lib"); +const validators = lib.Config.validators; +const { ControllerConfig, HostConfig, InstanceConfig, ControlConfig } = lib; + +describe("lib/config/validators", function() { + describe("all", function() { + it("should call all validators in order", function() { + let calls = []; + + function v1(value, config) { + calls.push("v1"); + } + function v2(value, config) { + calls.push("v2"); + } + + const combined = validators.all(v1, v2); + combined("value", {}); + + assert.deepEqual(calls, ["v1", "v2"], "validators were not called in order"); + }); + + it("should propagate errors from validators", function() { + function v1() {} + function v2() { + throw new Error("failure"); + } + + const combined = validators.all(v1, v2); + + assert.throws( + () => combined("value", {}), + /failure/, + "error from validator was not propagated" + ); + }); + }); + + describe("optional", function() { + it("should skip validation when value is null", function() { + let called = false; + + function validator() { + called = true; + } + + const wrapped = validators.optional(validator); + wrapped(null, {}); + + assert.equal(called, false, "validator should not have been called"); + }); + + it("should run validator when value is not null", function() { + let called = false; + + function validator() { + called = true; + } + + const wrapped = validators.optional(validator); + wrapped("value", {}); + + assert.equal(called, true, "validator was not called"); + }); + }); + + describe("greaterThan", function() { + it("should allow values greater than min", function() { + const validator = validators.greaterThan(5); + assert.doesNotThrow(() => validator(6)); + }); + + it("should throw if value equals min", function() { + const validator = validators.greaterThan(5); + + assert.throws( + () => validator(5), + /Value must be greater than 5/, + "did not throw on equal value" + ); + }); + + it("should throw if value is less than min", function() { + const validator = validators.greaterThan(5); + + assert.throws( + () => validator(4), + /Value must be greater than 5/, + "did not throw on smaller value" + ); + }); + }); + + describe("greaterThanZero", function() { + it("should allow values greater than zero", function() { + assert.doesNotThrow(() => validators.greaterThanZero(1)); + }); + + it("should throw on zero", function() { + assert.throws( + () => validators.greaterThanZero(0), + /Value must be greater than 0/ + ); + }); + + it("should throw on negative values", function() { + assert.throws( + () => validators.greaterThanZero(-1), + /Value must be greater than 0/ + ); + }); + }); + + describe("greaterThanEqual", function() { + it("should allow values greater than min", function() { + const validator = validators.greaterThanEqual(5); + assert.doesNotThrow(() => validator(6)); + }); + + it("should allow values equal to min", function() { + const validator = validators.greaterThanEqual(5); + assert.doesNotThrow(() => validator(5)); + }); + + it("should throw if value is less than min", function() { + const validator = validators.greaterThanEqual(5); + + assert.throws( + () => validator(4), + /Value must be greater than 5/, + "did not throw on smaller value" + ); + }); + }); + + describe("greaterThanEqualZero", function() { + it("should allow zero", function() { + assert.doesNotThrow(() => validators.greaterThanEqualZero(0)); + }); + + it("should allow positive values", function() { + assert.doesNotThrow(() => validators.greaterThanEqualZero(1)); + }); + + it("should throw on negative values", function() { + assert.throws( + () => validators.greaterThanEqualZero(-1), + /Value must be greater than 0/ + ); + }); + }); + + describe("integer", function() { + it("should allow integer values", function() { + assert.doesNotThrow(() => validators.integer(10)); + }); + + it("should throw on non-integer values", function() { + assert.throws( + () => validators.integer(1.5), + /Value must be an integer/, + "did not throw on float" + ); + }); + }); + + describe("filePath", function() { + it("should accept valid paths", function() { + assert.doesNotThrow(() => validators.filePath("test/file.txt")); + assert.doesNotThrow(() => validators.filePath("/absolute/path")); + }); + + it("should throw if value is not a string", function() { + // This is the only case that will cause path.resolve to throw on posix + assert.throws( + () => validators.filePath(null), + /Value must be a valid path/ + ); + }); + }); +}); + +describe("lib/config/definitions/validators", function() { + describe("Controller Config", function() { + + }); + + describe("Host Config", function() { + + }); + + describe("Instance Config", function() { + it("should validate factorio.version", function() { + const config = new InstanceConfig("controller"); + + // valid cases + assert.doesNotThrow(() => config.set("factorio.version", "latest")); + assert.doesNotThrow(() => config.set("factorio.version", "1.1")); + assert.doesNotThrow(() => config.set("factorio.version", "1.1.87")); + + // invalid case + assert.throws( + () => config.set("factorio.version", "invalidValue"), + /Value must be be 'latest', or match X.Y, or match X.Y.Z/ + ); + }); + + it("should validate factorio.player_online_autosave_slots against autosave slots", function() { + const config = new InstanceConfig("controller", { + "factorio.settings": { + autosave_slots: 5, + }, + }); + + // valid: equal + assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 5)); + + // valid: greater + assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 6)); + + // invalid: less than autosave_slots + assert.throws( + () => config.set("factorio.player_online_autosave_slots", 4), + /Value cannot be less than the number of autosave slots/ + ); + }); + + it("should allow player_online_autosave_slots when autosave_slots is not a number", function() { + const config = new InstanceConfig("controller", { + "factorio.settings": { + autosave_slots: null, + }, + }); + + assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 0)); + assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 1)); + }); + }); + + describe("Control Config", function() { + + }); +}); diff --git a/test/mock.js b/test/mock.js index d505f7f1..dd1c5d2f 100644 --- a/test/mock.js +++ b/test/mock.js @@ -195,6 +195,10 @@ class MockController { this.handles = new Map(); } + get authSecret() { + return Buffer.from(this.config.get("controller.auth_secret"), "base64"); + } + handle(eventClass, handler) { this.handles.set(eventClass, handler); } @@ -237,16 +241,15 @@ async function createInstancePlugin(InstancePluginClass, info) { return plugin; } - module.exports = { - MockLogger, - MockSocket, - MockConnector, - MockServer, - MockInstance, - MockHost, - MockControl, - MockController, + MockLogger: MockLogger, + MockSocket: MockSocket, + MockConnector: MockConnector, + MockServer: MockServer, + MockInstance: MockInstance, + MockHost: MockHost, + MockControl: MockControl, + MockController: MockController, createControllerPlugin, createInstancePlugin,