Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

date based electron tests are using today instead of specific date #3597

Closed
khassel opened this issue Oct 23, 2024 · 13 comments
Closed

date based electron tests are using today instead of specific date #3597

khassel opened this issue Oct 23, 2024 · 13 comments

Comments

@khassel
Copy link
Collaborator

khassel commented Oct 23, 2024

see #3596

The helper is not working (anymore) so every (electron) test running with a specific date does not run with this date but uses today.

Above PR should be reverted if this is solved.

@khassel khassel added the bug label Oct 23, 2024
@sdetweil
Copy link
Collaborator

yes, the 'work around' is to code the Date() setting IN the config.js file for the tescase instead of in the calendar_spec.js runner (which uses helper)
my newly added testcases have it in the config.js testcase file.. and also in the runner.. copied other working testcases..

in the config.js for the testcase works for local debugging

@khassel
Copy link
Collaborator Author

khassel commented Oct 23, 2024

but you have to be careful coding this in config because there are config's which are used in several tests ...

@sdetweil
Copy link
Collaborator

yes, this one (ICS)

auth-default.js:						url: "http://localhost:8080/tests/mocks/calendar_test.ics",
basic-auth.js:						url: "http://localhost:8080/tests/mocks/calendar_test.ics",
changed-port.js:						url: "http://localhost:8010/tests/mocks/calendar_test.ics",
default.js:						url: "http://localhost:8080/tests/mocks/calendar_test.ics"
fail-basic-auth.js:						url: "http://localhost:8020/tests/mocks/calendar_test.ics",
old-basic-auth.js:						url: "http://localhost:8080/tests/mocks/calendar_test.ics",
show-duplicates-in-calendar.js:						url: "http://localhost:8080/tests/mocks/calendar_test.ics" 

but each has diff .js file, which contains the Date() function

@sdetweil
Copy link
Collaborator

those other tests are used in e2e and do not care about the number of events posted..

@khassel khassel self-assigned this Oct 24, 2024
rejas pushed a commit that referenced this issue Oct 25, 2024
fixes #3597 

Changes:
- electron tests: add mocking to `electron.js` for mocking the server
side, before only the browser side was mocked
- publish "/tests/configs" and "/tests/mocks" always in `server.js`,
this reverts a change done with latest release, we need this for
debugging (otherwise you get on the screen that your config has errors
but config check is successful ...)
- revert hotfix in
`tests/configs/modules/calendar/show-duplicates-in-calendar.js`
- fix `tests/configs/modules/calendar/custom.js` to allow events in the
past (~~I don't know how this could work before~~ when testing css
classes `yesterday` and `dayBeforeYesterday` --> it worked before
because the server side did not mock and therefore was not in the past)
@sdetweil
Copy link
Collaborator

i have merged develop now that #3599 is merged

BUT the testcases in Electron still have Date().... in them did you intend for these to be changed too so that only date in runner applies?

grep "new Date" *.js
3_move_first_allday_repeating_event.js:	return new Date("01 Oct 2024 10:38:00 GMT+2:00").valueOf();
berlin_end_of_day_repeating.js:	return new Date("08 Oct 2024 12:30:00 GMT+02:00").valueOf();
berlin_multi.js:	return new Date("08 Oct 2024 12:30:00 GMT+02:00").valueOf();
berlin_whole_day_event_moved_over_dst_change.js:	return new Date("08 Oct 2024 12:30:00 GMT+02:00").valueOf();
chicago_late_in_timezone.js:	return new Date("01 Sept 2024 10:38:00 GMT-05:00").valueOf();
diff_tz_start_end.js:	return new Date("08 Oct 2024 12:30:00 GMT-07:00").valueOf();
end_of_day_berlin_moved.js:	return new Date("08 Oct 2024 12:30:00 GMT+02:00").valueOf();
event_with_time_over_multiple_days_non_repeating_display_end.js:	return new Date("15 Sep 2024 12:30:00 GMT").valueOf();
event_with_time_over_multiple_days_non_repeating_no_display_end.js:	return new Date("15 Sep 2024 12:30:00 GMT").valueOf();
exdate_and_recurrence_together.js:	return new Date("08 Oct 2024 12:30:00 GMT+07:00").valueOf();
exdate_la_at_midnight_dst.js:	return new Date("19 Oct 2023 12:30:00 GMT-07:00").valueOf();
exdate_la_at_midnight_std.js:	return new Date("19 Oct 2023 12:30:00 GMT-07:00").valueOf();
exdate_la_before_midnight.js:	return new Date("19 Oct 2023 12:30:00 GMT-07:00").valueOf();
exdate_syd_at_midnight_dst.js:	return new Date("14 Sep 2023 12:30:00 GMT+10:00").valueOf();
exdate_syd_at_midnight_std.js:	return new Date("14 Sep 2023 12:30:00 GMT+10:00").valueOf();
exdate_syd_before_midnight.js:	return new Date("14 Sep 2023 12:30:00 GMT+10:00").valueOf();
fullday_event_over_multiple_days_nonrepeating.js:	return new Date("15 Sep 2024 12:30:00 GMT").valueOf();
germany_at_end_of_day_repeating.js:	return new Date("01 Oct 2024 10:38:00 GMT+2:00").valueOf();
rrule_until.js:	return new Date("07 Mar 2024 10:38:00 GMT-07:00").valueOf();
show-duplicates-in-calendar.js:	return new Date("15 Sep 2024 12:30:00 GMT").valueOf();
sliceMultiDayEvents.js:	return new Date("01 Sept 2024 10:38:00 GMT+2:00").valueOf();

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 25, 2024

also in helper
we don't need this as its all about setting the date, which is done a different way now

	for (const win of global.electronApp.windows()) {
		const title = await win.title();
		expect(["MagicMirror²", "DevTools"]).toContain(title);
		if (title === "MagicMirror²") { <---- here  -------------------
			global.page = win;
			if (systemDate) {
				await global.page.evaluate((systemDate) => {
					Date.now = () => {
						return new Date(systemDate).valueOf();
					};
				}, systemDate);
			}
		}  <---- thru here  -------------------------
	}

@sdetweil
Copy link
Collaborator

this will fix all the testcases.. in the tests/config/modules/calendar folder

grep Date.now *.js | awk -F: '{print $1}' | xargs sed -i -e '/Date.now/,+3d'

@sdetweil
Copy link
Collaborator

my helper suggestion however causes failure to locate content

@khassel
Copy link
Collaborator Author

khassel commented Oct 25, 2024

I did several tests yesterday and as I wrote already in the PR, I think we need both, the date mocking in the browser and the date mocking in the backend (maybe except for modules running completly in the browser as weather).

But I'm happy to merge a simpler solution ...

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 25, 2024

sorry,, I missed the implementation detail of mocking on both. I 'assumed' that mocking on the server would set the date for the browser that was started later.

i removed all the config Date.now from the testcases.... and all were executed correctly, regarless of my system timezone

so I think the env sets the side for node_helpers.. and the helper global setup still does the same for the browser..
but the Date... in the actual testcases is not needed now..

this way there is only ONE place for the execution date( the runner)
course that makes debugging a little harder..

@khassel
Copy link
Collaborator Author

khassel commented Oct 25, 2024

but the Date... in the actual testcases is not needed now..

do you mean this e.g. in exdate_syd_at_midnight_dst.js:

Date.now = () => {
	return new Date("19 Oct 2023 12:30:00 GMT-07:00").valueOf();
};

was not aware of setting dates in the configs.

So now I understand your grep statement, will you provide a PR for removing this? Otherwise I can do this ...

@sdetweil
Copy link
Collaborator

see #3600

sdetweil added a commit to sdetweil/MagicMirror that referenced this issue Oct 25, 2024
khassel pushed a commit that referenced this issue Oct 25, 2024
…e testcase runner (#3601)

cleanup testcases with hard coded Date settings after #3597
@khassel
Copy link
Collaborator Author

khassel commented Oct 25, 2024

as test stuff is running on develop we don't have to wait for this until next release, so closing this now as completed.

@khassel khassel closed this as completed Oct 25, 2024
sdetweil added a commit that referenced this issue Jan 1, 2025
## [2.30.0] - 2025-01-01

Thanks to: @xsorifc28, @HeikoGr, @bugsounet, @khassel,
@KristjanESPERANTO, @rejas, @sdetweil.

> ⚠️ This release needs nodejs version `v20` or `v22 or higher`, minimum
version is `v20.18.1`

### Added

- [core] Add wayland and windows start options to `package.json` (#3594)
- [docs] Add step for npm publishing in release process (#3595)
- [core] Add GitHub workflow to run spellcheck a few days before each
release (#3623)
- [core] Add test flag to `index.html` to pass to module js for test
mode detection (needed by #3630)
- [core] Add export on animation names (#3644)
- [compliments] Add support for refreshing remote compliments file, and
test cases (#3630)
- [linter] Re-add `eslint-plugin-import`now that it supports ESLint v9
(#3586)
- [linter] Re-activate `eslint-plugin-package-json` to lint
`package.json` (#3643)
- [linter] Add linting for markdown files (#3646)
- [linter] Add some handy ESLint rules.
- [calendar] Add ability to display end date for full date events, where
end is not same day (showEnd=true) (#3650)
- [core] Add text to the config.js.sample file about the locale variable
(#3654, #3655)
- [core] Add fetch timeout for all node_helpers (thru undici, forces
node 20.18.1 minimum) to help on slower systems. (#3660) (3661)

### Changed

- [core] Run code style checks in workflow only once (#3648)
- [core] Fix animations export #3644 only on server side (#3649)
- [core] Use project URL in fallback config (#3656)
- [core] Fix Access Denied crash writing js/positions.js (on synology
nas) #3651. new message, MM starts, but no modules showing (#3652)
- [linter] Switch to 'npx' for lint-staged in pre-commit hook (#3658)

### Removed

- [tests] Remove `node-pty` and `drivelist` from rebuilded test (#3575)
- [deps] Remove `@eslint/js` dependency. Already installed with `eslint`
in deep (#3636)

### Updated

- [repo] Reactivate `stale.yaml` as GitHub action to mark issues as
stale after 60 days and close them 7 days later (if no activity) (#3577,
#3580, #3581)
- [core] Update electron dependency to v32 (test electron rebuild) and
all other dependencies too (#3657)
- [tests] All test configs have been updated to allow full external
access, allowing for easier debugging (especially when running as a
container)
- [core] Run and test with node 23 (#3588)
- [workflow] delete exception `allow-ghsas: GHSA-8hc4-vh64-cxmj` in
`dep-review.yaml` (#3659)

### Fixed

- [updatenotification] Fix pm2 using detection when pm2 script is inside
or outside MagicMirror root folder (#3576) (#3605) (#3626) (#3628)
- [core] Fix loading node_helper of modules: avoid black screen, display
errors and continue loading with next module (#3578)
- [weather] Change default value for weatherEndpoint of provider
openweathermap to "/onecall" (#3574)
- [tests] Fix electron tests with mock dates, the mock on server side
was missing (#3597)
- [tests] Fix testcases with hard coded Date.now (#3597)
- [core] Fix missing `basePath` where `location.host` is used (#3613)
- [compliments] croner library changed filenames used in latest version
(#3624)
- [linter] Fix ESLint ignore pattern which caused that default modules
not to be linted (#3632)
- [core] Fix module path in case of sub/sub folder is used and use
path.resolve for resolve `moduleFolder` and `defaultModuleFolder` in
app.js (#3653)
- [calendar] Update to resolve issues #3098 #3144 #3351 #3422 #3443
#3467 #3537 related to timezone changes
- [calendar] Fix #3267 (styles array), also fixes event with both exdate
AND recurrence(and testcase)
- [calendar] Fix showEndsOnlyWithDuration not working, #3598, applies
ONLY to full day events
- [calendar] Fix showEnd for Full Day events (#3602)
- [tests] Suppress "module is not defined" in e2e tests (#3647)
- [calendar] Fix #3267 (styles array, really this time!)
- [core] Fix #3662 js/positions.js created incorrectly

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Michael Teeuw <michael@xonaymedia.nl>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Ross Younger <crazyscot@gmail.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr>
Co-authored-by: jkriegshauser <joshuakr@nvidia.com>
Co-authored-by: illimarkangur <116028111+illimarkangur@users.noreply.github.com>
Co-authored-by: vppencilsharpener <tim.pray@gmail.com>
Co-authored-by: veeck <michael.veeck@nebenan.de>
Co-authored-by: Paranoid93 <6515818+Paranoid93@users.noreply.github.com>
Co-authored-by: Brian O'Connor <btoconnor@users.noreply.github.com>
Co-authored-by: WallysWellies <59727507+WallysWellies@users.noreply.github.com>
Co-authored-by: Jason Stieber <jrstieber@gmail.com>
Co-authored-by: jargordon <50050429+jargordon@users.noreply.github.com>
Co-authored-by: Daniel <32464403+dkallen78@users.noreply.github.com>
Co-authored-by: Ryan Williams <65094007+ryan-d-williams@users.noreply.github.com>
Co-authored-by: Panagiotis Skias <panagiotis.skias@gmail.com>
Co-authored-by: Marc Landis <dirk.rettschlag@gmail.com>
Co-authored-by: HeikoGr <20295490+HeikoGr@users.noreply.github.com>
Co-authored-by: Pedro Lamas <pedrolamas@gmail.com>
Co-authored-by: veeck <gitkraken@veeck.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants