## Changes
- Replace `indexOf()` with `startsWith()` for cleaner protocol detection
- Use `URL` API for robust cache-busting parameter handling
- Add HTTP response validation and improved error logging
- Add JSDoc type annotations for better documentation
- Remove unused `urlSuffix` instance variable
- Add unit tests
- Fix `.gitignore` pattern
## Motivation
After merging #3967, I noticed some potential for improving reliability
and user experience related to the method `loadComplimentFile`. With
these changes the method now validates URLs upfront to catch
configuration errors early, checks HTTP status codes to detect server
issues (404/500), and provides specific error messages that help users
troubleshoot problems.
The complexity of the code does not really increase with the changes. On
the contrary, the method should now be more intuitive to understand.
## Testing
Added unit tests for `loadComplimentFile()` to validate the
improvements:
- HTTP error handling
- Cache-busting
Since E2E tests already cover the happy path, these unit tests focus on
error cases and edge cases.
## Additional Fix
While adding the test file, I discovered that the `.gitignore` pattern
`modules` was incorrectly matching `tests/unit/modules/`, preventing
test files from being tracked. Changed to `/modules` to only match the
root directory.
### 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
## CI Log Suppression
**Two-level approach for clean test output:**
1. **Suppress debug/info logs**: Call `logger.setLogLevel("ERROR")` in
CI to hide verbose logging
2. **Suppress intentional error logs**: Set `mmTestMode` flag and check
it before logging errors that are part of test assertions (e.g., testing
error handling in `git_helper.js` and `server_functions.js`)
This keeps CI output clean and makes genuine failures immediately
visible, while preserving full logging for local development.
**Before:** 1348 log lines with verbose debug/info output
**After:** 168 log clean lines with only test results
## Calendar Symbol Test Stability
Convert the calendar symbol test from external URL (`calendarlabs.com`)
to existing local mock file (`12_events.ics`). This eliminates CI
timeouts caused by external dependencies and improves test reliability.
The test still validates the same symbol array feature but now runs
faster and deterministically without network dependencies.
This is a big change, but I think it's a good move, as `vitest` is much
more modern than `jest`.
I'm excited about the UI watch feature (run `npm run test:ui`), for
example - it's really helpful and saves time when debugging tests. I had
to adjust a few tests because they had time related issues, but
basically we are now testing the same things - even a bit better and
less flaky (I hope).
What do you think?
Remove the "Recurring event per timezone" test that manipulated
Date.prototype.getTimezoneOffset to simulate 24 different timezones for
testing all-day recurring events.
Reasons for removal:
1. The test approach is incompatible with node-ical 0.22.0's Intl-based
timezone handling (which replaced moment-timezone). Manipulating
Date.prototype.getTimezoneOffset no longer affects Intl.DateTimeFormat,
which reads the system timezone directly.
2. node-ical 0.22.0 handles all-day events (VALUE=DATE) correctly by
preserving the calendar date without timezone conversions, making
cross-timezone testing unnecessary. The library includes comprehensive
tests for this behavior, particularly "keeps whole-day recurrence across
DST" in
[test/advanced.test.js](https://github.com/jens-maus/node-ical/blob/master/test/advanced.test.js).
3. The existing "Recurring event" test already verifies that recurring
events from the same ICS file are displayed correctly, so a simplified
version of "Recurring event per timezone" is not necessary.
The old test attempted to work around timezone conversion issues in
node-ical 0.21.0 that are now properly resolved upstream.
Closes#3928
This fixes security issue
[CVE-2023-42282](https://github.com/advisories/GHSA-78xj-cgh5-2h22),
which is not very likely to be exploitable in MagicMirror² setups, but
still should be fixed.
The [express-ipfilter](https://www.npmjs.com/package/express-ipfilter)
package depends on the obviously unmaintained
[ip](https://github.com/indutny/node-ip) package, which has known
security vulnerabilities. Since no fix is available, this commit
replaces both dependencies with a custom middleware using the better
maintained [ipaddr.js](https://www.npmjs.com/package/ipaddr.js) library.
Changes:
- Add new `js/ip_access_control.js` with lightweight middleware
- Remove `express-ipfilter` dependency, add `ipaddr.js`
- Update `js/server.js` to use new middleware
- In addition, I have formulated the descriptions of the corresponding
tests a little more clearly.
The current logic never showed any different temperature than the
current one, so I looked into a more "explained" formula and found one
in the HomeAssistant forums.
- Remove `sinon` dependency in favor of Jest native mocking
- Unify test helper functions across translation test suites
- Rename `setupDOMEnvironment` to `createTranslationTestEnvironment` for
consistency
- Simplify DOM setup by removing unnecessary Promise/async patterns
- Avoid potential port conflicts by using port 3001 for translator unit
tests
- Improve test reliability and maintainability
This is not a cosmetic change because jest errors when run with
`CI=true`.
I got this error when running unit tests in gitlab:
```bash
FAIL unit tests/unit/functions/updatenotification_spec.js
● Test suite failed to run
Outdated guide link: The snapshot guide link is outdated.Please update all snapshots while upgrading of Jest
Expected: https://jestjs.io/docs/snapshot-testing
Received: https://goo.gl/fbAQLP
at validateSnapshotHeader (node_modules/@jest/snapshot-utils/build/index.js:104:12)
at getSnapshotData (node_modules/@jest/snapshot-utils/build/index.js:156:28)
```
If you run the test without `CI=true` jest will update the file.
Fixes#3253
Adds a configuration option to overwrite the default `User-Agent` header
that is send at least by the calendar and news module. Allows other
modules to use the individual user agent as well.
The configuration accepts either a string or a function:
```
var config =
{
...
userAgent: 'Mozilla/5.0 (My User Agent)',
...
}
```
or
```
var config =
{
...
userAgent: () => 'Mozilla/5.0 (My User Agent)',
...
}
```
---------
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: veeck <gitkraken@veeck.de>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
The weather e2e tests are failing sometimes, failing is not really
reproducable.
After changing `updateDom(0)` to `updateDom(300)` in `weather.js` it
seems to work (we will se if it really works in the long term).
This PR contains some other weather e2e changes/cleanups/simplifying.
- fix newsfeed and calendar e2e tests so they don't need `--forceExit`
anymore
- `--forceExit` should stay in test runs to avoid running until test
limit in case of errors
- remove mocking console, not needed anymore
- configFactory-stuff should not run in browser (otherwise we get errors
`[ReferenceError: exports is not defined]`)
e2e:
- needed window.close(), otherwise the objects are not destroyed
- add missing `await` in clock test
- set maxListeners for all tests
remaining todo (comes with another PR if I find the problem):
- calendar e2e is now the only test which still needs `--forceExit`
I have refactored the translations tests, they should now be clearer and
easier to understand. There should be no functional impact.
I have discarded the original approach of also replacing
`XMLHttpRequest` with `fetch` in the file `js/translator.js`. I had
managed to get it to work functionally, but I couldn't get the tests to
work.
- animateCSS_spec test did throw errors at least with newest
dependencies (running locally or on gitlab)
- dependency updates: New jest v30 breaks our tests so we have to stay
with v29 until fixed (will take a look)
While working on the translations (in #3792 and #3794) I realised that
the e2e "Same keys" tests were not working entirely. There was also a
TODO entry for this in the test, as well as a try-catch-block
workaround. I therefore fixed and refactored it.
I also sorted the translations in `translations/translations.js`, so
that the test outputs are alphabetically.
Refactored calendarfetcherutils to remove as many of the date
conversions as possible and use moment tz when calculating recurring
events, this will make debugging a lot easier and fixes problems from
the past with offsets and DST not being handled properly. Also added
some tests to test the behavior of the refactored methodes to make sure
the correct event dates are returned.
Refactored calendar.js aswell to make sure the unix UTC start and end
date of events are properly converted to a local timezone and displayed
correctly for the user.
This PR relates to:
https://github.com/MagicMirrorOrg/MagicMirror/issues/3797
---------
Co-authored-by: Koen Konst <c.h.konst@avisi.nl>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
I was always unhappy when maintaining dependency updates to have 3
`package.json` files.
This PR moves all deps into the main `package.json` and removes the
folders `fonts` and `vendor`.
If accepted I will update the docs too.
---------
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Hello and thank you for wanting to contribute to the MagicMirror²
project!
**Please make sure that you have followed these 4 rules before
submitting your Pull Request:**
> 1. Base your pull requests against the `develop` branch.
> 2. Include these infos in the description: implement short syntax for
clock week
>
> - Does the pull request solve a **related** issue? n/a
> - If so, can you reference the issue like this `Fixes
#<issue_number>`?
> - What does the pull request accomplish? Use a list if needed.
> - If it includes major visual changes please add screenshots.
>
> 3. Please run `npm run lint:prettier` before submitting so that
> style issues are fixed.
> 4. Don't forget to add an entry about your changes to
> the CHANGELOG.md file.
**Note**: Sometimes the development moves very fast. It is highly
recommended that you update your branch of `develop` before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.
Thanks again and have a nice day!
---------
Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com>
Hello and thank you for wanting to contribute to the MagicMirror²
project!
**Please make sure that you have followed these 4 rules before
submitting your Pull Request:**
> 1. Base your pull requests against the `develop` branch.
> 2. Include these infos in the description:
>
> - Does the pull request solve a **related** issue? No
> - If so, can you reference the issue like this `Fixes
#<issue_number>`? N/A
> - What does the pull request accomplish? Use a list if needed.
Introduce showNextEvent to show/hide next sun event
> - If it includes major visual changes please add screenshots.
>
> 3. Please run `npm run lint:prettier` before submitting so that
> style issues are fixed.
> 4. Don't forget to add an entry about your changes to
> the CHANGELOG.md file.
**Note**: Sometimes the development moves very fast. It is highly
recommended that you update your branch of `develop` before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.
Thanks again and have a nice day!
---------
Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com>
fixes#3267 AGAIN, correctly, add testcase
add testcase for #3679 , broadcast clipping incorrectly
I added a test module (tests/testNotification) to catch the notification
and check the count. (one way to configure)
i put this module in the tests folder, and added /tests to the server
paths.
(can't have a module in a nested folder, like tests/modules/xxx)
but I have a problem. i can run the test config (MM_CONFIG_FILE), and
the two modules work correctly,
but in the spec runner, the calendar module times out on the broadcast
test.. there is only local ics file access, no outside hosts
I forced my system date to 1/1/24 (same as runner) and again the manual
testcase works fine
I added two test config.js,(configs/calendar) one works great (symbols)
, one fails (broadcast test)
I added additional delay in the calendarspec runner to try to debug the
module, but it still not long enough.. no messages of trouble when I get
into the browser.. BUT, this may be because of the log being turned
off... (just thought of this)
I created a special ICS (in mocks) that has 12 events, 1 for each
month.. (so I can check clipping and broadcast) the US holidays one is
sensitive to the current date, and I couldn't get it to work on
1/1/2024..
also, in general, is there a mechanism to run test:just_one_runner?
waiting thru the electron test to get to one testcase.. ugh..
here is an updated test version of the fixes for all kinds of calendar
date problems.
NOTE: the changed branch name
NOTE: this used the node-cal@0.19.0 library UNCHANGED
best to make a new folder and git clone there
git clone https://github.com/sdetweil/MagicMirror
cd MagicMirror
git checkout fixcaldates2 // <------ note this is a changed branch name
npm run install-mm
copy your config.js and custom.css from the prior folder
and the non-default modules you have installed…
this ONLY changes the default calendar
but DOES ship an updated node-ical library too
if you need to fall back, just rename the folders around again so that
your original is called MagicMirror
all the testcases for node-ical and MagicMirror execute successfully.
the ‘BIG’ change here is to get the local NON-TZ dates for the
rrule.between()
all the checking and conversion code is commented out or not used
the node-ical fixes are for excluded dates (exdate) values being
adjusted for DST/STD time… waiting to submit that PR
one fix in calendar.js for checking if a past date was too far back,
but it never checked to see IF the event date was in the past… (before
today) so it chopped off too many
and one change in calendarfetcher.js to put out a better diagnostic
message of the parsed data… (exdate was excluded cause JSON stringify
couldn’t convert the complex structure)
I added the tests you all have documented
please re-pull and checkout the new branch (I deleted the old branch)
and npm run install-mm again
---------
Co-authored-by: Veeck <github@veeck.de>
in some cases the modulename.js may need to detect running in test mode
(compliments pr #3630)
window.name is not set web mode
add a new field to the index.html
window.intest
and use the server_function to replace the hard coded string like we do
for window.mmversion=#VERSION#
then change the two test helpers to set the env variable
app.js detects and sets global.intest=true
server func replace with value of global.intest
then module can use if(window.intest)
