Skip to content

Commit aa7a3c8

Browse files
feat: add DOM tests and fix lockStrings handling
Frontend: - Refactor remote.js for testability: * Export Remote to globalThis.Remote for test access * Encapsulate initialization in Remote.init() * Auto-initialize only when loaded in browser (not in tests) * No breaking changes for production use - Fix lockStrings crash in MMM-Remote-Control.js: * Normalize lockStrings to empty array in sendCurrentData() at source * Use for-of loop instead of indexed for in handleDefaultSettings * Handle undefined lockStrings from legacy settings.json files * Fixes "Cannot read properties of undefined (reading 'length')" error Testing: - Add DOM test suite using happy-dom (13 tests): * remote.js: 10 smoke tests for core functionality * MMM-Remote-Control.js: 3 tests for lockStrings edge cases * Fast (~130ms), no browser required, CI-friendly * Uses vm.runInContext for isolated execution - Update test infrastructure: * Install happy-dom as dev dependency * Add test:dom npm script * Integrate DOM tests into main test command * Update README with DOM test documentation All 109 tests passing (94 unit + 5 integration + 10 DOM)
1 parent 103c5ab commit aa7a3c8

7 files changed

Lines changed: 361 additions & 28 deletions

File tree

MMM-Remote-Control.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ Module.register("MMM-Remote-Control", {
184184
const {moduleData} = payload;
185185
const hideModules = {};
186186
for (const moduleDatum of moduleData) {
187-
for (let k = 0; k < moduleDatum.lockStrings.length; k++) {
188-
if (moduleDatum.lockStrings[k].includes("MMM-Remote-Control")) {
187+
for (const lockString of moduleDatum.lockStrings || []) {
188+
if (lockString.includes("MMM-Remote-Control")) {
189189
hideModules[moduleDatum.identifier] = true;
190190
break;
191191
}
@@ -256,7 +256,7 @@ Module.register("MMM-Remote-Control", {
256256
const modules = MM.getModules();
257257
const currentModuleData = [];
258258
modules.enumerate((module) => {
259-
const moduleData = {...module.data, hidden: module.hidden, lockStrings: module.lockStrings, urlPath: module.name.replaceAll("MMM-", "").replaceAll("-", "").toLowerCase(), config: module.config};
259+
const moduleData = {...module.data, hidden: module.hidden, lockStrings: module.lockStrings || [], urlPath: module.name.replaceAll("MMM-", "").replaceAll("-", "").toLowerCase(), config: module.config};
260260
const modulePrototype = Object.getPrototypeOf(module);
261261
moduleData.defaults = modulePrototype.defaults;
262262
currentModuleData.push(moduleData);

package-lock.json

Lines changed: 50 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@
2929
"lint:fix": "eslint --fix && prettier . --write",
3030
"prepare": "simple-git-hooks || echo 'No problem. Skipping git hooks installation.'",
3131
"postinstall": "node scripts/postinstall.js",
32-
"test": "node --run lint && node --run test:spelling && node --run test:unit && node --run test:integration",
32+
"test": "node --run lint && node --run test:spelling && node --run test:unit && node --run test:integration && node --run test:dom",
3333
"test:coverage": "c8 --all node --test tests/unit/**/*.test.js tests/integration/**/*.test.js",
34+
"test:dom": "node --test tests/dom/**/*.test.js",
3435
"test:integration": "node --test tests/integration/**/*.test.js",
3536
"test:spelling": "cspell .",
3637
"test:unit": "node --test tests/unit/**/*.test.js"
@@ -53,6 +54,7 @@
5354
"eslint-plugin-import-x": "^4.16.1",
5455
"eslint-plugin-unicorn": "^62.0.0",
5556
"globals": "^16.5.0",
57+
"happy-dom": "^20.0.11",
5658
"lint-staged": "^16.2.7",
5759
"prettier": "^3.7.4",
5860
"simple-git-hooks": "^2.13.1"

remote.js

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,32 +2119,47 @@ const buttons = {
21192119
}
21202120
};
21212121

2122-
// Initialize socket connection
2123-
Remote.sendSocketNotification("REMOTE_CLIENT_CONNECTED");
2124-
Remote.sendSocketNotification("REMOTE_ACTION", {data: "translations"});
2125-
Remote.loadButtons(buttons);
2126-
Remote.loadOtherElements();
2127-
2128-
Remote.setStatus("none");
2129-
2130-
if (globalThis.location.hash) {
2131-
Remote.showMenu(globalThis.location.hash.slice(1));
2132-
} else {
2133-
Remote.showMenu("main-menu");
2122+
// Export Remote to window for testability
2123+
if (globalThis.window !== undefined) {
2124+
globalThis.Remote = Remote;
21342125
}
21352126

2136-
globalThis.addEventListener("hashchange", () => {
2137-
if (Remote.skipHashChange) {
2138-
Remote.skipHashChange = false;
2139-
return;
2140-
}
2127+
// Initialize the Remote UI when DOM is ready
2128+
Remote.init = function () {
2129+
// Initialize socket connection
2130+
Remote.sendSocketNotification("REMOTE_CLIENT_CONNECTED");
2131+
Remote.sendSocketNotification("REMOTE_ACTION", {data: "translations"});
2132+
Remote.loadButtons(buttons);
2133+
Remote.loadOtherElements();
2134+
2135+
Remote.setStatus("none");
2136+
21412137
if (globalThis.location.hash) {
21422138
Remote.showMenu(globalThis.location.hash.slice(1));
21432139
} else {
21442140
Remote.showMenu("main-menu");
21452141
}
2146-
});
21472142

2148-
// loading successful, remove error message
2149-
const loadError = document.querySelector("#load-error");
2150-
loadError.remove();
2143+
globalThis.addEventListener("hashchange", () => {
2144+
if (Remote.skipHashChange) {
2145+
Remote.skipHashChange = false;
2146+
return;
2147+
}
2148+
if (globalThis.location.hash) {
2149+
Remote.showMenu(globalThis.location.hash.slice(1));
2150+
} else {
2151+
Remote.showMenu("main-menu");
2152+
}
2153+
});
2154+
2155+
// loading successful, remove error message
2156+
const loadError = document.querySelector("#load-error");
2157+
if (loadError) {
2158+
loadError.remove();
2159+
}
2160+
};
2161+
2162+
// Auto-initialize when loaded in browser
2163+
if (globalThis.window !== undefined && document.querySelector("#load-error")) {
2164+
Remote.init();
2165+
}

tests/README.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ This document describes the state of the automated test suite for **MMM-Remote-C
88
- **Coverage:** `c8` with enforced thresholds (30% statements/lines, 20% functions, 60% branches).
99
- **Quality gates:** Lint (`node --run lint`) and spell check (`node --run test:spelling`) are part of the standard `node --run test` pipeline.
1010
- **Execution shortcuts:**
11-
- All tests: `node --run test` (includes unit + HTTP-layer)
11+
- All tests: `node --run test` (includes unit + integration + DOM)
1212
- Unit tests only: `node --run test:unit`
1313
- HTTP-layer tests only: `node --run test:integration`
14+
- DOM tests only: `node --run test:dom`
1415
- Coverage report: `node --run test:coverage`
1516
- Watch mode: `node --run test:watch`
1617

@@ -20,6 +21,7 @@ This document describes the state of the automated test suite for **MMM-Remote-C
2021
tests/
2122
├── unit/ # Isolated logic tests with mocked dependencies
2223
├── integration/ # HTTP-layer tests with real Express routing
24+
├── dom/ # DOM logic tests using happy-dom (no browser required)
2325
└── shims/ # Minimal stubs for MagicMirror globals
2426
```
2527

@@ -32,7 +34,9 @@ tests/
3234
- JSON parsing/serialization problems
3335
- Authentication/error response formats
3436

35-
Both run in CI/CD without MagicMirror runtime or browser dependencies.
37+
**DOM tests** verify frontend logic (`remote.js`) in a simulated DOM environment using happy-dom. Fast, CI-friendly, no browser required.
38+
39+
All tests run in CI/CD without MagicMirror runtime or browser dependencies.
3640

3741
## What we cover today
3842

@@ -51,6 +55,12 @@ Both run in CI/CD without MagicMirror runtime or browser dependencies.
5155
| `api.helpers.test.js` | JSON payload parsing, delay parameter handling |
5256
| `utils.test.js`, `configUtils.test.js` | String-format helpers and `cleanConfig` regressions |
5357

58+
### DOM tests (`tests/dom/`)
59+
60+
| Suite | Purpose |
61+
| ---------------------- | ---------------------------------------------------------- |
62+
| `remote.smoke.test.js` | Frontend logic tests using happy-dom (no browser required) |
63+
5464
### HTTP-layer tests (`tests/integration/`)
5565

5666
| Suite | Purpose |
@@ -78,7 +88,7 @@ Thresholds now provide meaningful regression protection while remaining achievab
7888

7989
- **Router-to-handler wiring:** Express route-mapping tests duplicated framework behavior and broke on route reordering. Manual smoke tests or integration tests are better.
8090
- **System commands and hardware control (`shutdown`, `reboot`, monitor control):** Depend on Raspberry Pi hardware privileges and side effects we can't safely stub in CI.
81-
- **Front-end DOM or E2E coverage:** Rendering happens in MagicMirror's browser context; Puppeteer/Electron harnesses are heavy relative to payoff.
91+
- **E2E browser tests:** Full Puppeteer/Playwright tests with real browsers are heavy and flaky. We test frontend logic with happy-dom instead (fast, deterministic, CI-friendly).
8292
- **"Pass-through" notification wrappers:** Flows that simply forward parameters (`HIDE_ALERT`, `SHOW_ALERT`, etc.) are exercised indirectly; duplicating them would be noise.
8393
- **Git/network dependent paths:** Update and install code paths require repositories and network access. We guard their public contract via `getExternalApiByGuessing`/menu tests instead.
8494

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
const {test, describe, before, after} = require("node:test");
2+
const assert = require("node:assert/strict");
3+
const {Window} = require("happy-dom");
4+
const fs = require("node:fs");
5+
const path = require("node:path");
6+
const vm = require("node:vm");
7+
8+
describe("MMM-Remote-Control.js module", () => {
9+
let window, Module;
10+
11+
before(() => {
12+
window = new Window({
13+
url: "http://localhost:8080",
14+
settings: {
15+
disableJavaScriptFileLoading: true,
16+
disableJavaScriptEvaluation: false,
17+
disableCSSFileLoading: true
18+
}
19+
});
20+
21+
window.Module = {
22+
register: function (moduleName, moduleDefinition) {
23+
Module = moduleDefinition;
24+
}
25+
};
26+
27+
window.MM = {
28+
getModules: () => ({
29+
enumerate: () => {}
30+
})
31+
};
32+
33+
window.Log = {
34+
info: () => {},
35+
log: () => {},
36+
error: () => {},
37+
warn: () => {}
38+
};
39+
40+
window.location = {hash: ""};
41+
window.globalThis = window;
42+
43+
const modulePath = path.join(__dirname, "../../MMM-Remote-Control.js");
44+
const moduleCode = fs.readFileSync(modulePath, "utf8");
45+
const context = vm.createContext(window);
46+
vm.runInContext(moduleCode, context);
47+
});
48+
49+
after(() => {
50+
window.close();
51+
});
52+
53+
test("module is registered with Module.register", () => {
54+
assert.ok(Module, "Module should be defined");
55+
assert.ok(Module.handleDefaultSettings, "handleDefaultSettings should exist");
56+
});
57+
58+
test("handleDefaultSettings handles missing lockStrings gracefully", () => {
59+
const payload = {
60+
settingsVersion: 1,
61+
moduleData: [
62+
{identifier: "module_1", name: "clock"},
63+
{identifier: "module_2", name: "calendar", lockStrings: ["lock1"]},
64+
{identifier: "module_3", name: "weather", lockStrings: undefined}
65+
],
66+
brightness: 100,
67+
temp: 327
68+
};
69+
70+
assert.doesNotThrow(() => {
71+
Module.handleDefaultSettings.call({
72+
identifier: "MMM-Remote-Control",
73+
settingsVersion: 1,
74+
setBrightness: () => {},
75+
setTemp: () => {}
76+
}, payload);
77+
});
78+
});
79+
80+
test("handleDefaultSettings handles non-array lockStrings", () => {
81+
const payload = {
82+
settingsVersion: 1,
83+
moduleData: [{identifier: "module_1", name: "clock", lockStrings: "not-an-array"}],
84+
brightness: 100,
85+
temp: 327
86+
};
87+
88+
assert.doesNotThrow(() => {
89+
Module.handleDefaultSettings.call({
90+
identifier: "MMM-Remote-Control",
91+
settingsVersion: 1,
92+
setBrightness: () => {},
93+
setTemp: () => {}
94+
}, payload);
95+
});
96+
});
97+
});

0 commit comments

Comments
 (0)