Skip to content

Commit f29f424

Browse files
[core] refactor: replace XMLHttpRequest with fetch and migrate e2e tests to Playwright (#3950)
### 1. Replace `XMLHttpRequest` with the modern `fetch` API for loading translation files #### Changes - **translator.js**: Use `fetch` with `async/await` instead of XHR callbacks - **loader.js**: Align URL handling and add error handling (follow-up to fetch migration) - **Tests**: Update infrastructure for `fetch` compatibility #### Benefits - Modern standard API - Cleaner, more readable code - Better error handling and fallback mechanisms ### 2. Migrate e2e tests to Playwright This wasn't originally planned for this PR, but is related. While investigating suspicious log entries which surfaced after the fetch migration I kept running into JSDOM’s limitations. That pushed me to migrate the E2E suite to Playwright instead. #### Changes - switch e2e harness to Playwright (`tests/e2e/helpers/global-setup.js`) - rewrite specs to use Playwright locators + shared `expectTextContent` - install Chromium via `npx playwright install --with-deps` in CI #### Benefits - much closer to real browser behaviour - and no more fighting JSDOM’s quirks
1 parent 2b08288 commit f29f424

31 files changed

+508
-361
lines changed

.github/workflows/automated-tests.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ jobs:
5959
- name: "Install MagicMirror²"
6060
run: |
6161
node --run install-mm:dev
62+
- name: "Install Playwright browsers"
63+
run: |
64+
npx playwright install --with-deps chromium
6265
- name: "Prepare environment for tests"
6366
run: |
6467
# Fix chrome-sandbox permissions:

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ planned for 2026-01-01
3232
- [tests] replace `node-libgpiod` with `serialport` in electron-rebuild workflow (#3945)
3333
- [calendar] hide repeatingCountTitle if the event count is zero (#3949)
3434
- [core] configure cspell to check default modules only and fix typos (#3955)
35+
- [core] refactor: replace `XMLHttpRequest` with `fetch` in `translator.js` (#3950)
36+
- [tests] migrate e2e tests to Playwright (#3950)
3537

3638
### Fixed
3739

eslint.config.mjs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {flatConfigs as importX} from "eslint-plugin-import-x";
44
import js from "@eslint/js";
55
import jsdocPlugin from "eslint-plugin-jsdoc";
66
import packageJson from "eslint-plugin-package-json";
7+
import playwright from "eslint-plugin-playwright";
78
import stylistic from "@stylistic/eslint-plugin";
89
import vitest from "eslint-plugin-vitest";
910

@@ -59,6 +60,20 @@ export default defineConfig([
5960
"import-x/order": "error",
6061
"init-declarations": "off",
6162
"vitest/consistent-test-it": "warn",
63+
"vitest/expect-expect": [
64+
"warn",
65+
{
66+
assertFunctionNames: [
67+
"expect",
68+
"testElementLength",
69+
"testTextContain",
70+
"doTest",
71+
"runAnimationTest",
72+
"waitForAnimationClass",
73+
"assertNoAnimationWithin"
74+
]
75+
}
76+
],
6277
"vitest/prefer-to-be": "warn",
6378
"vitest/prefer-to-have-length": "warn",
6479
"max-lines-per-function": ["warn", 400],
@@ -125,5 +140,12 @@ export default defineConfig([
125140
rules: {
126141
"@stylistic/quotes": "off"
127142
}
143+
},
144+
{
145+
files: ["tests/e2e/**/*.js"],
146+
extends: [playwright.configs["flat/recommended"]],
147+
rules: {
148+
"playwright/no-standalone-expect": "off"
149+
}
128150
}
129151
]);

js/loader.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,36 @@ const Loader = (function () {
1010

1111
/* Private Methods */
1212

13+
/**
14+
* Get environment variables from config.
15+
* @returns {object} Env vars with modulesDir and customCss paths from config.
16+
*/
17+
const getEnvVarsFromConfig = function () {
18+
return {
19+
modulesDir: config.foreignModulesDir || "modules",
20+
customCss: config.customCss || "css/custom.css"
21+
};
22+
};
23+
1324
/**
1425
* Retrieve object of env variables.
1526
* @returns {object} with key: values as assembled in js/server_functions.js
1627
*/
1728
const getEnvVars = async function () {
18-
const res = await fetch(`${location.protocol}//${location.host}${config.basePath}env`);
19-
return JSON.parse(await res.text());
29+
// In test mode, skip server fetch and use config values directly
30+
if (typeof process !== "undefined" && process.env && process.env.mmTestMode === "true") {
31+
return getEnvVarsFromConfig();
32+
}
33+
34+
// In production, fetch env vars from server
35+
try {
36+
const res = await fetch(new URL("env", `${location.origin}${config.basePath}`));
37+
return JSON.parse(await res.text());
38+
} catch (error) {
39+
// Fallback to config values if server fetch fails
40+
Log.error("Unable to retrieve env configuration", error);
41+
return getEnvVarsFromConfig();
42+
}
2043
};
2144

2245
/**

js/translator.js

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,24 @@
33
const Translator = (function () {
44

55
/**
6-
* Load a JSON file via XHR.
6+
* Load a JSON file via fetch.
77
* @param {string} file Path of the file we want to load.
88
* @returns {Promise<object>} the translations in the specified file
99
*/
1010
async function loadJSON (file) {
11-
const xhr = new XMLHttpRequest();
12-
return new Promise(function (resolve) {
13-
xhr.overrideMimeType("application/json");
14-
xhr.open("GET", file, true);
15-
xhr.onreadystatechange = function () {
16-
if (xhr.readyState === 4 && xhr.status === 200) {
17-
// needs error handler try/catch at least
18-
let fileInfo = null;
19-
try {
20-
fileInfo = JSON.parse(xhr.responseText);
21-
} catch (exception) {
22-
// nothing here, but don't die
23-
Log.error(`[translator] loading json file =${file} failed`);
24-
}
25-
resolve(fileInfo);
26-
}
27-
};
28-
xhr.send(null);
29-
});
11+
const baseHref = document.baseURI;
12+
const url = new URL(file, baseHref);
13+
14+
try {
15+
const response = await fetch(url);
16+
if (!response.ok) {
17+
throw new Error(`Unexpected response status: ${response.status}`);
18+
}
19+
return await response.json();
20+
} catch (exception) {
21+
Log.error(`Loading json file =${file} failed`);
22+
return null;
23+
}
3024
}
3125

3226
return {

package-lock.json

Lines changed: 30 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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"config:check": "node js/check_config.js",
3737
"postinstall": "git clean -df fonts vendor",
3838
"install-mm": "npm install --no-audit --no-fund --no-update-notifier --only=prod --omit=dev",
39-
"install-mm:dev": "npm install --no-audit --no-fund --no-update-notifier",
39+
"install-mm:dev": "npm install --no-audit --no-fund --no-update-notifier && npx playwright install chromium",
4040
"lint:css": "stylelint 'css/main.css' 'css/roboto.css' 'css/font-awesome.css' 'modules/default/**/*.css' --fix",
4141
"lint:js": "eslint --fix",
4242
"lint:markdown": "markdownlint-cli2 . --fix",
@@ -106,6 +106,7 @@
106106
"eslint-plugin-import-x": "^4.16.1",
107107
"eslint-plugin-jsdoc": "^61.1.11",
108108
"eslint-plugin-package-json": "^0.59.1",
109+
"eslint-plugin-playwright": "^2.3.0",
109110
"eslint-plugin-vitest": "^0.5.4",
110111
"express-basic-auth": "^1.2.1",
111112
"husky": "^9.1.7",

tests/e2e/animateCSS_spec.js

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
const { expect } = require("playwright/test");
12
const helpers = require("./helpers/global-setup");
23

34
// Validate Animate.css integration for compliments module using class toggling.
45
// We intentionally ignore computed animation styles (jsdom doesn't simulate real animations).
56
describe("AnimateCSS integration Test", () => {
7+
let page;
8+
69
// Config variants under test
710
const TEST_CONFIG_ANIM = "tests/configs/modules/compliments/compliments_animateCSS.js";
811
const TEST_CONFIG_FALLBACK = "tests/configs/modules/compliments/compliments_animateCSS_fallbackToDefault.js"; // invalid animation names
@@ -11,32 +14,26 @@ describe("AnimateCSS integration Test", () => {
1114

1215
/**
1316
* Get the compliments container element (waits until available).
14-
* @returns {Promise<HTMLElement>} compliments root element
17+
* @returns {Promise<void>}
1518
*/
1619
async function getComplimentsElement () {
1720
await helpers.getDocument();
18-
const el = await helpers.waitForElement(".compliments");
19-
expect(el).not.toBeNull();
20-
return el;
21+
page = helpers.getPage();
22+
await expect(page.locator(".compliments")).toBeVisible();
2123
}
2224

2325
/**
2426
* Wait for an Animate.css class to appear and persist briefly.
2527
* @param {string} cls Animation class name without leading dot (e.g. animate__flipInX)
2628
* @param {{timeout?: number}} [options] Poll timeout in ms (default 6000)
27-
* @returns {Promise<boolean>} true if class detected in time
29+
* @returns {Promise<void>}
2830
*/
2931
async function waitForAnimationClass (cls, { timeout = 6000 } = {}) {
30-
const start = Date.now();
31-
while (Date.now() - start < timeout) {
32-
if (document.querySelector(`.compliments.animate__animated.${cls}`)) {
33-
// small stability wait
34-
await new Promise((r) => setTimeout(r, 50));
35-
if (document.querySelector(`.compliments.animate__animated.${cls}`)) return true;
36-
}
37-
await new Promise((r) => setTimeout(r, 100));
38-
}
39-
throw new Error(`Timeout waiting for class ${cls}`);
32+
const locator = page.locator(`.compliments.animate__animated.${cls}`);
33+
await locator.waitFor({ state: "attached", timeout });
34+
// small stability wait
35+
await new Promise((r) => setTimeout(r, 50));
36+
await expect(locator).toBeAttached();
4037
}
4138

4239
/**
@@ -46,8 +43,10 @@ describe("AnimateCSS integration Test", () => {
4643
*/
4744
async function assertNoAnimationWithin (ms = 2000) {
4845
const start = Date.now();
46+
const locator = page.locator(".compliments.animate__animated");
4947
while (Date.now() - start < ms) {
50-
if (document.querySelector(".compliments.animate__animated")) {
48+
const count = await locator.count();
49+
if (count > 0) {
5150
throw new Error("Unexpected animate__animated class present in non-animation scenario");
5251
}
5352
await new Promise((r) => setTimeout(r, 100));
@@ -58,21 +57,20 @@ describe("AnimateCSS integration Test", () => {
5857
* Run one animation test scenario.
5958
* @param {string} [animationIn] Expected animate-in name
6059
* @param {string} [animationOut] Expected animate-out name
61-
* @returns {Promise<boolean>} true when scenario assertions pass
60+
* @returns {Promise<void>} Throws on assertion failure
6261
*/
6362
async function runAnimationTest (animationIn, animationOut) {
6463
await getComplimentsElement();
6564
if (!animationIn && !animationOut) {
6665
await assertNoAnimationWithin(2000);
67-
return true;
66+
return;
6867
}
6968
if (animationIn) await waitForAnimationClass(`animate__${animationIn}`);
7069
if (animationOut) {
7170
// Wait just beyond one update cycle (updateInterval=2000ms) before expecting animateOut.
7271
await new Promise((r) => setTimeout(r, 2100));
7372
await waitForAnimationClass(`animate__${animationOut}`);
7473
}
75-
return true;
7674
}
7775

7876
afterEach(async () => {
@@ -82,28 +80,28 @@ describe("AnimateCSS integration Test", () => {
8280
describe("animateIn and animateOut Test", () => {
8381
it("with flipInX and flipOutX animation", async () => {
8482
await helpers.startApplication(TEST_CONFIG_ANIM);
85-
await expect(runAnimationTest("flipInX", "flipOutX")).resolves.toBe(true);
83+
await runAnimationTest("flipInX", "flipOutX");
8684
});
8785
});
8886

8987
describe("use animateOut name for animateIn (vice versa) Test", () => {
9088
it("without animation (inverted names)", async () => {
9189
await helpers.startApplication(TEST_CONFIG_INVERTED);
92-
await expect(runAnimationTest()).resolves.toBe(true);
90+
await runAnimationTest();
9391
});
9492
});
9593

9694
describe("false Animation name test", () => {
9795
it("without animation (invalid names)", async () => {
9896
await helpers.startApplication(TEST_CONFIG_FALLBACK);
99-
await expect(runAnimationTest()).resolves.toBe(true);
97+
await runAnimationTest();
10098
});
10199
});
102100

103101
describe("no Animation defined test", () => {
104102
it("without animation (no config)", async () => {
105103
await helpers.startApplication(TEST_CONFIG_NONE);
106-
await expect(runAnimationTest()).resolves.toBe(true);
104+
await runAnimationTest();
107105
});
108106
});
109107
});
Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1+
const { expect } = require("playwright/test");
12
const helpers = require("./helpers/global-setup");
23

34
describe("Custom Position of modules", () => {
5+
let page;
6+
47
beforeAll(async () => {
58
await helpers.fixupIndex();
69
await helpers.startApplication("tests/configs/customregions.js");
710
await helpers.getDocument();
11+
page = helpers.getPage();
812
});
913
afterAll(async () => {
1014
await helpers.stopApplication();
@@ -16,15 +20,12 @@ describe("Custom Position of modules", () => {
1620
const className1 = positions[i].replace("_", ".");
1721
let message1 = positions[i];
1822
it(`should show text in ${message1}`, async () => {
19-
const elem = await helpers.waitForElement(`.${className1}`);
20-
expect(elem).not.toBeNull();
21-
expect(elem.textContent).toContain(`Text in ${message1}`);
23+
await expect(page.locator(`.${className1} .module-content`)).toContainText(`Text in ${message1}`);
2224
});
2325
i = 1;
2426
const className2 = positions[i].replace("_", ".");
2527
let message2 = positions[i];
2628
it(`should NOT show text in ${message2}`, async () => {
27-
const elem = await helpers.waitForElement(`.${className2}`, "", 1500);
28-
expect(elem).toBeNull();
29-
}, 1510);
29+
await expect(page.locator(`.${className2} .module-content`)).toHaveCount(0);
30+
});
3031
});

tests/e2e/env_spec.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
const { expect } = require("playwright/test");
12
const helpers = require("./helpers/global-setup");
23

34
describe("App environment", () => {
5+
let page;
6+
47
beforeAll(async () => {
58
await helpers.startApplication("tests/configs/default.js");
69
await helpers.getDocument();
10+
page = helpers.getPage();
711
});
812
afterAll(async () => {
913
await helpers.stopApplication();
@@ -20,8 +24,6 @@ describe("App environment", () => {
2024
});
2125

2226
it("should show the title MagicMirror²", async () => {
23-
const elem = await helpers.waitForElement("title");
24-
expect(elem).not.toBeNull();
25-
expect(elem.textContent).toBe("MagicMirror²");
27+
await expect(page).toHaveTitle("MagicMirror²");
2628
});
2729
});

0 commit comments

Comments
 (0)