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

Fixcaldates2 fix calendar module date processing, using [email protected] #3587

Merged
merged 45 commits into from
Dec 7, 2024

Conversation

sdetweil
Copy link
Collaborator

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 [email protected] 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

@sdetweil
Copy link
Collaborator Author

I can't create the timeout on 20.x electron test

 PASS   electron  tests/electron/modules/calendar_spec.js (45.347 s)
  Calendar module
    Test css classes
      ✓ has css class dayBeforeYesterday (2531 ms)
      ✓ has css class yesterday (2539 ms)
      ✓ has css class today (2471 ms)
      ✓ has css class tomorrow (2501 ms)
      ✓ has css class dayAfterTomorrow (2602 ms)
    Events from multiple calendars
      ✓ should show multiple events with the same title and start time from different calendars (2602 ms)
    rrule
      ✓ Issue #3393 recurrence dates past rrule until date (2552 ms)
    Exdate: LA crossover DST before midnight GMT
      ✓ LA crossover DST before midnight GMT should have 2 events (2496 ms)
    Exdate: LA crossover DST at midnight GMT local STD
      ✓ LA crossover DST before midnight GMT should have 2 events (2531 ms)
    Exdate: LA crossover DST at midnight GMT local DST
      ✓ LA crossover DST before midnight GMT should have 2 events (2516 ms)
    Exdate: SYD crossover DST before midnight GMT
      ✓ LA crossover DST before midnight GMT should have 2 events (2443 ms)
    Exdate: SYD crossover DST at midnight GMT local STD
      ✓ LA crossover DST before midnight GMT should have 2 events (2486 ms)
    Exdate: SYD crossover DST at midnight GMT local DST
      ✓ SYD crossover DST at midnight GMT local DST should have 2 events (2577 ms)
    sliceMultiDayEvents
      ✓ Issue #3452 split multiday in Europe (2517 ms)
    sliceMultiDayEvents direct count
      ✓ Issue #3452 split multiday in Europe (2490 ms)
    germany timezone
      ✓ Issue #unknown fullday timezone East of UTC edge (2493 ms)
    germany all day repeating moved (recurrence and exdate)
      ✓ Issue #unknown fullday timezone East of UTC event moved (2448 ms)
    chicago late in timezone
      ✓ Issue #unknown rrule US close to timezone edge (2454 ms)

@khassel
Copy link
Collaborator

khassel commented Oct 17, 2024

getting the same error ...

    chicago late in timezone
      ✕ Issue #unknown rrule US close to timezone edge (20080 ms)

  ● Calendar module › chicago late in timezone › Issue #unknown rrule US close to timezone edge

    thrown: "Exceeded timeout of 20000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      178 |
      179 |     describe("chicago late in timezone", () => {
    > 180 |             it("Issue #unknown rrule US close to timezone edge", async () => {
          |             ^
      181 |                     await helpers.startApplication("tests/configs/modules/calendar/chicago_late_in_timezone.js", "01 Sept 2024 10:38:00 GMT-5:00", ["js/electron.js"], "America/Chicago");
      182 |                     await expect(doTestTableContent(".calendar .event", ".time", "10th.Sep, 20:15")).resolves.toBe(true);
      183 |             });

      at it (tests/electron/modules/calendar_spec.js:180:3)
      at describe (tests/electron/modules/calendar_spec.js:179:2)
      at Object.describe (tests/electron/modules/calendar_spec.js:3:1)

@sdetweil
Copy link
Collaborator Author

yeh. thats not the real error. something has made it fail (variable, coding) but i don't see anything locally
manual or the test runs.

only diff is i added logLevel to this js testcase, but logs are nooped in test runs

@sdetweil
Copy link
Collaborator Author

removed the logLevel config.js setting for chicago test..

@sdetweil
Copy link
Collaborator Author

anyone have ideas how to see the stdout/err during the jest run?

i cannot recreate this

@khassel
Copy link
Collaborator

khassel commented Oct 19, 2024

node@89788bc75cde:/opt/magic_mirror$ cat tests/configs/modules/calendar/chicago_late_in_timezone.js
let config = {
        timeFormat: 24,
        modules: [
                {
                        module: "calendar",
                        position: "bottom_bar",
                        config: {
                                fade: false,
                                urgency: 0,
                                dateFormat: "Do.MMM, HH:mm",
                                fullDayEventDateFormat: "Do.MMM",
                                timeFormat: "absolute",
                                getRelative: 0,
                                maximumNumberOfDays: 20,
                                calendars: [
                                        {
                                                maximumEntries: 100,
                                                //url: "http://localhost:8080/tests/mocks/chicago_late_in_timezone.ics"
                                                url: "http://localhost:8080/modules/default/calendar/chicago_late_in_timezone.ics"
                                        }
                                ]
                        }
                }
        ]
};

the referenced file modules/default/calendar/chicago_late_in_timezone.ics doesn't exist.

By the way, the new test config's are missing the lines address and ipWhitelist ...

@sdetweil
Copy link
Collaborator Author

thx
doh!

@sdetweil
Copy link
Collaborator Author

but, how did you see that

@sdetweil
Copy link
Collaborator Author

fixed those

@sdetweil
Copy link
Collaborator Author

sdetweil commented Oct 23, 2024

So, the testcase that is failing here is due to the fact that today+6 months gets April 9 in view of the repeating rule
if the event date is April 10 and not April 9, only 10 events are returned from RRULE.between()

because the code does ==>now<===+maximumNumberOfDay(10000) for the end date of between
number of days until Mar 9 2052 from now = 9999

but that implies that the systemdate in the testcase didn't execute in the helper
testcase code

			await helpers.startApplication("tests/configs/modules/calendar/show-duplicates-in-calendar.js", "15 Sep 2024 
12:30:00 GMT");

helper code

			if (systemDate) {
				await global.page.evaluate((systemDate) => {
					Date.now = () => {
						return new Date(systemDate).valueOf();
					};
				}, systemDate);
			}

if I add this date setting to the testcase .js itself, it works..

};

Date.now = () => {
  return new Date("15 Sep 2024 12:30:00 GMT").valueOf();
};

/*************** DO NOT EDIT THE LINE BELOW ***************/
if (typeof module !== "undefined") {
	module.exports = config;
}

how do we want to fix this testcase?
I have added date to the testcase .js near term

@sdetweil sdetweil self-assigned this Nov 11, 2024
@sdetweil sdetweil changed the title Fixcaldates2 fix calendar module date processing, using [email protected] Fixcaldates2 fix calendar module date processing, using [email protected] Nov 11, 2024
@sdetweil
Copy link
Collaborator Author

last successful run at — Today at 2:57 AM (my time? us central) on merge of the compliments update

my update at 8:45 for calendar failed.

@sdetweil
Copy link
Collaborator Author

sdetweil commented Nov 15, 2024

what is the test system date and timezone?

if the event filter (calendarfetcherutils) really returned a bad date/time value (Ends in Invalid date)
then its got to be related to the system time and offset.. as the code uses that to normalize the UTC dates before rrule.between() which only supports local times.. no tz.

but logging is turned off by test, so I can't get that

the elecrton tests set different dates and timezones.. from US west coast to Australia.

@sdetweil
Copy link
Collaborator Author

sdetweil commented Nov 15, 2024

ok, found it, TZ = UTC

I 'fixed' a lint error

		const [_, h, m] = str.match(/([+-]\d+):(\d+)$/) || [, "+00", "00"];

lint error
/home/sam/mm/modules/default/calendar/calendarfetcherutils.js
621:55 error Unexpected comma in middle of array no-sparse-arrays

I fixed it wrong, corrected now.. the array set if not parsed needs 3 elements.

@sdetweil
Copy link
Collaborator Author

sdetweil commented Dec 4, 2024

ok, ready to merge... no additional testers have found anything, or commited to finish testing.

this will close pr #3622 as not needed

Copy link
Collaborator

@rejas rejas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostyl small nitpicking things...

CHANGELOG.md Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
tests/electron/modules/calendar_spec.js Outdated Show resolved Hide resolved
tests/electron/modules/compliments_spec.js Show resolved Hide resolved
tests/electron/modules/calendar_spec.js Outdated Show resolved Hide resolved
@rejas rejas merged commit 19bd76a into MagicMirrorOrg:develop Dec 7, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants