test(structure): group config/utils/delayedQuery tests and clarify titles

Author: KristjanESPERANTO

tests/README.md

@@ -23,7 +23,7 @@ Status checkpoint (2025-09-21):
 
 - Phases 1 and 2 completed. We added unit tests for API helpers (`checkDelay`, `answerNotifyApi`), `answerModuleApi` defaults and SHOW flow, and timer behavior for `delayedQuery`.
 - Introduced shims for `logger` and `node_helper` so `node_helper.js` can be required in tests without MagicMirror runtime.
-- One `answerGet` test is currently skipped due to tight coupling to MM core; plan is to re-cover at the API router level or further isolate dependencies.
+- Phase 3 router-level GET coverage added; one direct `answerGet` unit test remains skipped and is deferred to a later phase once NodeHelper/MM core is further isolated.
 
 ## Test Roadmap (Phases)
 
@@ -33,12 +33,11 @@ The project follows an incremental test roadmap. Coverage thresholds start low a
    - Lint, formatter, spellcheck wired; no runtime tests.
 2. Phase 2 — Test runner & utilities (Done)
    - Test runner configured, first unit tests added, coverage baseline established.
-3. Phase 3 — More unit & first integration tests (In progress)
+3. Phase 3 — More unit & first integration tests (Done)
    - Extra edge cases for `cleanConfig` (Done)
-   - GET routes: `/get?data=moduleAvailable`, `/get?data=config`, error path (Pending)
-     - Note: One `answerGet` unit test is currently skipped; target to re-cover via API router tests or further isolation.
-   - Small express/test factory (mocks: fs, simple-git) (Pending)
-   - Raise coverage target (e.g., statements >8%) (Pending)
+   - GET routes covered at router level (Done)
+   - Small express/test factory (mocks: fs, simple-git) (Deferred)
+   - Raise coverage target (Done — thresholds bumped slightly)
 4. Phase 4 — Action / socket logic (core `executeQuery` paths) (In progress)
    - DELAYED timer (start, reset, abort) (Done)
    - BRIGHTNESS, TEMP, NOTIFICATION parsing, HIDE/SHOW/TOGGLE selection logic (Pending)

tests/unit/answerGet.test.js

@@ -1,4 +1,7 @@
 const {test} = require("node:test");
 
-// Skipped: Requires MagicMirror core defaults and NodeHelper; keep as placeholder
+/*
+ * TODO(later phase): Direct unit coverage of answerGet when we further isolate NodeHelper/MM core.
+ * Router-level GET coverage is in tests/unit/api.getRoutes.mapping.test.js (Phase 3 done).
+ */
 test.skip("answerGet GET paths: moduleAvailable/config/error", () => {});

tests/unit/api.answerModuleApi.test.js

@@ -1,5 +1,6 @@
 const assert = require("node:assert/strict");
-const {test} = require("node:test");
+const {test, describe} = require("node:test");
+const group = typeof describe === "function" ? describe : (_n, fn) => fn();
 
 const apiModule = require("../../API/api.js");
 
@@ -21,40 +22,42 @@ function makeCtx (overrides = {}) {
   };
 }
 
-test("answerModuleApi DEFAULTS calls answerGet with defaultConfig", () => {
-  const captured = {};
-  const ctx = makeCtx({
-    mergeData: () => ({success: true, data: [{identifier: "module_1_weather", name: "weather", urlPath: "weather", actions: {}}]}),
-    answerGet: (query, res) => {
-      captured.query = query;
-      if (res && res.json) res.json({ok: true});
-    }
+group("Module API", () => {
+  test("DEFAULTS action requests defaultConfig via answerGet", () => {
+    const captured = {};
+    const ctx = makeCtx({
+      mergeData: () => ({success: true, data: [{identifier: "module_1_weather", name: "weather", urlPath: "weather", actions: {}}]}),
+      answerGet: (query, res) => {
+        captured.query = query;
+        if (res && res.json) res.json({ok: true});
+      }
+    });
+    const answerModuleApi = apiModule.answerModuleApi.bind(ctx);
+
+    const req = {params: {moduleName: "weather", action: "defaults"}};
+    const res = {status: () => ({json: () => {}}), json: () => {}};
+    answerModuleApi(req, res);
+
+    assert.deepEqual(captured.query, {data: "defaultConfig", module: "weather"});
   });
-  const answerModuleApi = apiModule.answerModuleApi.bind(ctx);
 
-  const req = {params: {moduleName: "weather", action: "defaults"}};
-  const res = {status: () => ({json: () => {}}), json: () => {}};
-  answerModuleApi(req, res);
-
-  assert.deepEqual(captured.query, {data: "defaultConfig", module: "weather"});
-});
-
-test("answerModuleApi SHOW all triggers executeQuery with module all", () => {
-  const captured = {};
-  const ctx = makeCtx({
-    // Minimal module data so filtering passes when moduleName === 'all'
-    configData: {moduleData: [{identifier: "module_1_test", name: "test", urlPath: "test"}]},
-    executeQuery: (query) => { captured.query = query; },
-    sendSocketNotification: () => {},
-    // No delay in this test
-    checkDelay: (q) => q
+  test("SHOW action on 'all' triggers executeQuery with module all", () => {
+    const captured = {};
+    const ctx = makeCtx({
+      // Minimal module data so filtering passes when moduleName === 'all'
+      configData: {moduleData: [{identifier: "module_1_test", name: "test", urlPath: "test"}]},
+      executeQuery: (query) => { captured.query = query; },
+      sendSocketNotification: () => {},
+      // No delay in this test
+      checkDelay: (q) => q
+    });
+    const answerModuleApi = apiModule.answerModuleApi.bind(ctx);
+
+    const req = {params: {moduleName: "all", action: "show"}};
+    const res = {json: () => {}};
+    answerModuleApi(req, res);
+
+    assert.equal(captured.query.action, "SHOW");
+    assert.equal(captured.query.module, "all");
   });
-  const answerModuleApi = apiModule.answerModuleApi.bind(ctx);
-
-  const req = {params: {moduleName: "all", action: "show"}};
-  const res = {json: () => {}};
-  answerModuleApi(req, res);
-
-  assert.equal(captured.query.action, "SHOW");
-  assert.equal(captured.query.module, "all");
 });

tests/unit/api.delayedFlow.test.js

@@ -1,5 +1,6 @@
 const assert = require("node:assert/strict");
-const {test} = require("node:test");
+const {test, describe} = require("node:test");
+const group = typeof describe === "function" ? describe : (_n, fn) => fn();
 
 const apiModule = require("../../API/api.js");
 
@@ -21,25 +22,27 @@ function makeCtx (overrides = {}) {
   };
 }
 
-test("answerNotifyApi wraps into DELAYED when /delay used", () => {
-  const captured = {};
-  const ctx = makeCtx({
-    delayedQuery: (query) => { captured.query = query; }
-  });
-  const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
+group("Delayed flow (/delay)", () => {
+  test("answerNotifyApi wraps query into DELAYED and preserves payload", () => {
+    const captured = {};
+    const ctx = makeCtx({
+      delayedQuery: (query) => { captured.query = query; }
+    });
+    const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
 
-  const req = {
-    method: "GET",
-    params: {notification: "HELLO", delayed: "delay"},
-    query: {did: "ID1", timeout: 5}
-  };
-  const res = {json: () => {}};
+    const req = {
+      method: "GET",
+      params: {notification: "HELLO", delayed: "delay"},
+      query: {did: "ID1", timeout: 5}
+    };
+    const res = {json: () => {}};
 
-  answerNotifyApi(req, res);
+    answerNotifyApi(req, res);
 
-  assert.equal(captured.query.action, "DELAYED");
-  assert.equal(captured.query.did, "ID1");
-  assert.equal(captured.query.timeout, 5);
-  assert.equal(captured.query.query.action, "NOTIFICATION");
-  assert.equal(captured.query.query.notification, "HELLO");
+    assert.equal(captured.query.action, "DELAYED");
+    assert.equal(captured.query.did, "ID1");
+    assert.equal(captured.query.timeout, 5);
+    assert.equal(captured.query.query.action, "NOTIFICATION");
+    assert.equal(captured.query.query.notification, "HELLO");
+  });
 });

tests/unit/api.helpers.test.js

@@ -1,5 +1,6 @@
 const assert = require("node:assert/strict");
-const {test} = require("node:test");
+const {test, describe} = require("node:test");
+const group = typeof describe === "function" ? describe : (_n, fn) => fn();
 
 // We'll import the module and bind methods to a fake context so we can test pure-ish helpers
 const apiModule = require("../../API/api.js");
@@ -20,71 +21,66 @@ function makeCtx (overrides = {}) {
   };
 }
 
-test("checkDelay adds DELAYED wrapper and UUID if '/delay' is used", () => {
-  const ctx = makeCtx();
-  const checkDelay = apiModule.checkDelay.bind(ctx);
-  const req = {params: {delayed: "delay"}, query: {}, body: {}};
-  const q = {action: "RESTART"};
-  const out = checkDelay(q, req);
-  assert.equal(out.action, "DELAYED");
-  assert.ok(out.did && typeof out.did === "string");
-  assert.equal(out.timeout, 10);
-  assert.deepEqual(out.query, q);
-});
-
-test("checkDelay keeps original query when no '/delay'", () => {
-  const ctx = makeCtx();
-  const checkDelay = apiModule.checkDelay.bind(ctx);
-  const req = {params: {}, query: {}, body: {}};
-  const q = {action: "REFRESH"};
-  const out = checkDelay(q, req);
-  assert.equal(out, q);
-});
+group("API helpers", () => {
+  group("checkDelay", () => {
+    test("wraps query as DELAYED when '/delay' is used (adds ID and default timeout)", () => {
+      const ctx = makeCtx();
+      const checkDelay = apiModule.checkDelay.bind(ctx);
+      const req = {params: {delayed: "delay"}, query: {}, body: {}};
+      const q = {action: "RESTART"};
+      const out = checkDelay(q, req);
+      assert.equal(out.action, "DELAYED");
+      assert.ok(out.did && typeof out.did === "string");
+      assert.equal(out.timeout, 10);
+      assert.deepEqual(out.query, q);
+    });
 
-test("answerNotifyApi builds payload from query and params (GET)", () => {
-  const captured = {};
-  const ctx = makeCtx({
-    sendSocketNotification: (what, payload) => {
-      captured.what = what;
-      captured.payload = payload;
-    }
+    test("keeps original query when '/delay' is not used", () => {
+      const ctx = makeCtx();
+      const checkDelay = apiModule.checkDelay.bind(ctx);
+      const req = {params: {}, query: {}, body: {}};
+      const q = {action: "REFRESH"};
+      const out = checkDelay(q, req);
+      assert.equal(out, q);
+    });
   });
-  const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
 
-  const req = {
-    method: "GET",
-    params: {notification: "TEST_ACTION"},
-    query: {foo: "bar"}
-  };
-  const res = {json: (obj) => { captured.response = obj; }};
-  answerNotifyApi(req, res);
+  group("answerNotifyApi", () => {
+    test("builds payload from GET params and returns success", () => {
+      const captured = {};
+      const ctx = makeCtx({
+        sendSocketNotification: (what, payload) => {
+          captured.what = what;
+          captured.payload = payload;
+        }
+      });
+      const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
 
-  assert.equal(captured.what, "NOTIFICATION");
-  assert.equal(captured.payload.notification, "TEST_ACTION");
-  assert.deepEqual(captured.response, {success: true, notification: "TEST_ACTION", payload: {foo: "bar"}});
-});
+      const req = {method: "GET", params: {notification: "TEST_ACTION"}, query: {foo: "bar"}};
+      const res = {json: (obj) => { captured.response = obj; }};
+      answerNotifyApi(req, res);
 
-test("answerNotifyApi merges POST body into payload and supports action payload", () => {
-  const captured = {};
-  const ctx = makeCtx({
-    sendSocketNotification: (what, payload) => { captured.payload = payload; }
-  });
-  const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
+      assert.equal(captured.what, "NOTIFICATION");
+      assert.equal(captured.payload.notification, "TEST_ACTION");
+      assert.deepEqual(captured.response, {success: true, notification: "TEST_ACTION", payload: {foo: "bar"}});
+    });
 
-  const req = {
-    method: "POST",
-    params: {notification: "TEST_ACTION"},
-    query: {alpha: 1},
-    body: {beta: 2}
-  };
-  const res = {json: () => {}};
+    test("merges POST body and action payload into final payload", () => {
+      const captured = {};
+      const ctx = makeCtx({sendSocketNotification: (what, payload) => { captured.payload = payload; }});
+      const answerNotifyApi = apiModule.answerNotifyApi.bind(ctx);
 
-  const action = {notification: "TEST_ACTION", payload: {gamma: 3}};
-  answerNotifyApi(req, res, action);
+      const req = {method: "POST", params: {notification: "TEST_ACTION"}, query: {alpha: 1}, body: {beta: 2}};
+      const res = {json: () => {}};
+      const action = {notification: "TEST_ACTION", payload: {gamma: 3}};
 
-  // payload should contain alpha, beta, and action payload gamma
-  assert.equal(captured.payload.notification, "TEST_ACTION");
-  assert.equal(captured.payload.payload.alpha, 1);
-  assert.equal(captured.payload.payload.beta, 2);
-  assert.equal(captured.payload.payload.gamma, 3);
+      answerNotifyApi(req, res, action);
+
+      // payload should contain alpha, beta, and action payload gamma
+      assert.equal(captured.payload.notification, "TEST_ACTION");
+      assert.equal(captured.payload.payload.alpha, 1);
+      assert.equal(captured.payload.payload.beta, 2);
+      assert.equal(captured.payload.payload.gamma, 3);
+    });
+  });
 });

tests/unit/configUtils.edgecases.test.js

@@ -1,31 +1,34 @@
 const assert = require("node:assert/strict");
-const {test} = require("node:test");
+const {test, describe} = require("node:test");
 const {cleanConfig} = require("../../lib/configUtils");
+const group = typeof describe === "function" ? describe : (_n, fn) => fn();
 
-test("cleanConfig leaves non-default top-level keys and removes defaults deeply (arrays/objects)", () => {
-  const defaultConfig = {language: "en", header: {enabled: true}, list: [1, 2, 3]};
-  const moduleDefaultsMap = {foo: {arr: [1, 2], obj: {a: 1}}};
-  const cfg = {
-    language: "en", // should be removed
-    header: {enabled: true}, // should be removed (deep equal)
-    list: [1, 2, 3, 4], // differs -> keep
-    modules: [{module: "foo", header: "", config: {arr: [1, 2], obj: {a: 1}, extra: 9, position: ""}}]
-  };
-  cleanConfig({config: cfg, defaultConfig, moduleDefaultsMap});
-  assert.ok(!("language" in cfg));
-  assert.ok(!("header" in cfg));
-  assert.deepEqual(cfg.list, [1, 2, 3, 4]);
-  const m = cfg.modules[0];
-  assert.ok(!("arr" in m.config));
-  assert.ok(!("obj" in m.config));
-  assert.equal(m.config.extra, 9);
-  assert.ok(!("position" in m.config));
-  assert.ok(!("header" in m));
-});
+group("configUtils.cleanConfig edge cases", () => {
+  test("removes deep-equal defaults; preserves differing arrays/objects", () => {
+    const defaultConfig = {language: "en", header: {enabled: true}, list: [1, 2, 3]};
+    const moduleDefaultsMap = {foo: {arr: [1, 2], obj: {a: 1}}};
+    const cfg = {
+      language: "en", // should be removed
+      header: {enabled: true}, // should be removed (deep equal)
+      list: [1, 2, 3, 4], // differs -> keep
+      modules: [{module: "foo", header: "", config: {arr: [1, 2], obj: {a: 1}, extra: 9, position: ""}}]
+    };
+    cleanConfig({config: cfg, defaultConfig, moduleDefaultsMap});
+    assert.ok(!("language" in cfg));
+    assert.ok(!("header" in cfg));
+    assert.deepEqual(cfg.list, [1, 2, 3, 4]);
+    const m = cfg.modules[0];
+    assert.ok(!("arr" in m.config));
+    assert.ok(!("obj" in m.config));
+    assert.equal(m.config.extra, 9);
+    assert.ok(!("position" in m.config));
+    assert.ok(!("header" in m));
+  });
 
-test("cleanConfig tolerates nulls and unknown modules", () => {
-  const cfg = {language: null, modules: [{module: "unknown", config: {x: 1}}]};
-  cleanConfig({config: cfg, defaultConfig: {language: null}, moduleDefaultsMap: {}});
-  assert.ok(!("language" in cfg));
-  assert.equal(cfg.modules[0].config.x, 1);
+  test("tolerates nulls at top level and unknown modules", () => {
+    const cfg = {language: null, modules: [{module: "unknown", config: {x: 1}}]};
+    cleanConfig({config: cfg, defaultConfig: {language: null}, moduleDefaultsMap: {}});
+    assert.ok(!("language" in cfg));
+    assert.equal(cfg.modules[0].config.x, 1);
+  });
 });