Repository: kelektiv/node-cron
Pull request #639 (2022-12-15 16:04:21)

Fix infinite looping bugs with recurrent tasks around DST
This fixes issue #638 

it does this by 

1) detecting a forward jump in time happened after changing the time of a date in _getNextDate()
2) Scanning the jumped-over range for matches with the time pattern.

What made (2) extra tricky is that apparently, DST is not uniformly 1 hour. That made things a little more complicated.
Right now the skipped range scanner assumes a forward jump is 1 minute to 23 hrs 59 minutes long, and does not roll over days.

This PR also fixes a test that was failing on the master branch, by disambiguating a DateTime (the ISO string represented 2 possible times due to a DST backwards shift, and luxon picked the unfavorable one).

**An important** thing to know about this change, is that it also fixes another, more minor bug with _getNextDate(), namely that a jumped-over time candidate is not considered. E.g. an `every *:30:00'`job does not activate at 2:30 if the time jumps from 1 to 3. (I would expect it to, and it now does, activate at 1:30, 3:00 and 3:30 respectively). Because the time of 3:00 represents every time of day between 2 and 3 all at once.

For example, how the [original Cron handles DST:](https://www.unix.com/man-page/Linux/8/cron/)

```
 Special considerations exist when the clock is changed by less than 3 hours, for example at the beginning and end of daylight savings time.
       If the time has moved forwards, those jobs which would have run in the time that was skipped will be  run  soon	after  the  change.   Con-
       versely, if the time has moved backwards by less than 3 hours, those jobs that fall into the repeated time will not be re-run.

       Only  jobs  that run at a particular time (not specified as @hourly, nor with '*' in the hour or minute specifier) are affected. Jobs which
       are specified with wildcards are run based on the new time immediately.
```

Backwards jumps were handled just like this, but forward ones not as much.

This is however a change you probably want to be aware of as maintainers of this library, Maybe you know of use cases for node-cron where actually the opposite, current behavior is preferred?

ncb000gt: @GijsvandenHoven Thanks for the patience. I'll try to take a look at the code changes tonight. This would be wonderful to land. Much appreciated!
ncb000gt: It looks like there is a lot of looping in the code. While it seems like a lot at first glance it looks to be bounded. I just wanted to confirm with you that this is the case and it's not going to add additional load in performing the checks it's performing. If not this looks good to me.

Would you mind fixing the conflicts?

Thanks!
GijsvandenHoven: @ncb000gt Hey, sorry for the brief silent period.

I went back to my own code and I agree, that's amount of `while` loops is even making me nervous.
For the ones in `_checkTimeInSkippedRange` (that's all but one of them), I thought I could at least turn them into for-loops instead.

While doing this I encountered some code later in my own function, for which i'm pretty sure it would be incorrect. 
Which is only a little embarassing.
The cases for which it would bug out were 'fortunately' only for DST jumps that do not exist currently,
(https://en.wikipedia.org/wiki/Daylight_saving_time_by_country), but this motivated me to both **add tests that poke this specific function** for these cases, as well as fix up the function itself because I was unhappy with it.

-----

The new version, instead of doing all this abstract work to account for arbitrary DST jumps, first checks how spicy of a DST jump it is dealing with first, to prevent unnecessary complicated work.

This results in a 3-way branch:

**Normal** 
Your every day DST jump falls under this. If you jump by exactly 60 minutes, starting at a 0th minute. 
We just check if this skipped-over hour would be valid for a CronJob and that's all that needs to be done. Since any arbitrary minute x second combination would fall under it, and the scheduled CJ time is the same millisecond timestamp no matter what.

**Australian**
The people of Lord Howe's Island have 30 minute DST jumps, and they are the whole reason I made this thing deal with arbitrary DST jumps instead of just hardcoding 1 hour. 
If a jump doesn't change the hour number we first check the hour for validity, and then if necessary loop over the jumped-over minutes to see if a valid time exists.

**Quirky**
Anything else, like non-hour jumps that change the hour number or over-an-hour jumps end up here.
This does the same kind of work the old function used to do, constructing arrays of ranges for minutes and hours to check.
However, instead of clunky 'while' loops, filtering the other cases let me easily construct these ranges directly with `Array.from` expressions.
Then we just check every valid hour x minute combination to see if there was a valid time.

The commit where I changed this function is [b26c884](https://github.com/kelektiv/node-cron/pull/639/commits/b26c88423896f7a41dd53f3676b21fb2403d57e9), but the diff is pretty garbled so you may want to review it locally instead. The changes are contained to the `_checkTimeInSkippedRange` function and its two subroutines.

----

>  I just wanted to confirm with you that this is the case and it's not going to add additional load in performing the checks it's performing

I think it should be fine, since all of this code is exclusive to the case where it has actually detected a DST forward jump beforehand (i.e. once per year per cronjob event). 

When it detects one of these, it will scan the skipped over range, but after this is done the `_getNextDateFrom` loop continues with whatever resulting date the "addition that passed over a jump" had. Meaning the range-scanning function is not called a second time.

The 'real' overhead that I add to the checks the main loop in `_getNextDateFrom` is this check each iteration to see if any jumps happened after adding time to the date, which is just a bit of arithmetic. Only if this check returns true, all the other code in the PR runs.

![sdTlZWX](https://user-images.githubusercontent.com/60928209/211924560-1d5fce17-fc34-4f3b-9c2b-6b2ef995bb60.png)

Anyways with my overhaul of the function, there should be only one loop now instead of 3. Only if the future adds quirky DST increments will it do the 'original' magnitude of work, but even then it's reduced now.
GijsvandenHoven: Oh also I went and added an iteration limit to the final remaining while loop, *just* in case. (Up to one day of backwards checking for a DST jump)  87a3ad8.
It shouldn't be possible to trigger this since the checks in `_getNextDateFrom` before calling this make it so that the #iterations is bounded by the amount of minutes skipped by the jump (usually 60), but _if_ an arbitrary date ends up there, I figured it'd be good to prevent a buggy call from checking backwards up to a year.
GijsvandenHoven: @ncb000gt Sorry to poke you, I understand that you may be busy with other projects. 

I was wondering if you had a chance to review these last changes. I think you will agree the changes from my (overly long) previous reply improved the PR, and the unit tests I've added give me confidence it will handle any unusual forward jump. 

Please let me know if there's anything I can do to help move this process forward. I'm looking forward to your feedback and appreciate your time and attention.
intcreator: hey @GijsvandenHoven I took a preliminary look at these and I think it's headed in the right direction. I want to take a bit more time to make sure I understand it though. thanks for your work on it so far!
intcreator: @GijsvandenHoven I went through the whole thing and I think I understand what's going on in most of it. thanks for all your hard work writing the new functionality, your explanations here and in the code, and the thorough tests. I left a few comments where I still had a hard time following what was going on but besides that I think this looks very good! I'm excited to merge this and hopeful that it will fix a lot of the issues people have been seeing
GijsvandenHoven: @intcreator Thank you for the thorough review! It was beneficial, that broken code for the loop in particular was something.

I have pushed a few commits that should address everything you said. Let me know if there are any further comments.

Thanks in advance! 🚀 
intcreator: changes look good! could you rebase again? then I think we're ready to merge @GijsvandenHoven 
GijsvandenHoven: Should be rebased now. Thank you for the helpful reviews and see you next mission! @intcreator 
intcreator: hmm, looks like some merge conflicts need to be resolved now? you'll probably have to force push the rebased branch once you're done if you haven't already: 
![image](https://user-images.githubusercontent.com/19642312/222942337-678c62e0-1a86-4dbf-a872-83ff47d22126.png)

GijsvandenHoven: You are right, I did not push the right buttons. I synced my pull request branch with my forked master branch at first. Which had no conflicts.

This master branch had to be rebased on node-cron, of course! And now there were conflicts 🦀 🦀 . Fortunately just package.json though. Then rebasing my pull request branch again had a few more surprise conflicts, but it should be all good now. Very thankful for GUI merge tools today.

I also triple checked that no code went missing during this operation, and I couldn't find any. Nevertheless I backed up this branch before the rebasing, so if something seems off I can always bring it back.
intcreator: perfect! merging now...

Patch:
diff --git a/lib/time.js b/lib/time.js
index 6a950ff..cf2ad4b 100644
--- a/lib/time.js
+++ b/lib/time.js
@@ -283,6 +283,12 @@ function CronTime(luxon) {
 				) {
 					date = date.plus({ months: 1 });
 					date = date.set({ day: 1, hour: 0, minute: 0, second: 0 });
+
+					if (this._forwardDSTJump(0, 0, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
 					continue;
 				}
 
@@ -296,6 +302,12 @@ function CronTime(luxon) {
 				) {
 					date = date.plus({ days: 1 });
 					date = date.set({ hour: 0, minute: 0, second: 0 });
+
+					if (this._forwardDSTJump(0, 0, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
 					continue;
 				}
 
@@ -309,14 +321,34 @@ function CronTime(luxon) {
 				) {
 					date = date.plus({ days: 1 });
 					date = date.set({ hour: 0, minute: 0, second: 0 });
+					if (this._forwardDSTJump(0, 0, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
 					continue;
 				}
 
 				if (!(date.hour in this.hour) && Object.keys(this.hour).length !== 24) {
-					date = date.set({
-						hour: date.hour === 23 && diff > 86400000 ? 0 : date.hour + 1
-					});
+					const expectedHour =
+						date.hour === 23 && diff > 86400000 ? 0 : date.hour + 1;
+					const expectedMinute = date.minute; // expect no change.
+
+					date = date.set({ hour: expectedHour });
 					date = date.set({ minute: 0, second: 0 });
+
+					// When this is the case, Asking luxon to go forward by 1 hour actually made us go forward by more hours...
+					// This indicates that somewhere between these two time points, a forward DST adjustment has happened.
+					// When this happens, the job should be scheduled to execute as though the time has come when the jump is made.
+					// Therefore, the job should be scheduled on the first tick after the forward jump.
+					if (this._forwardDSTJump(expectedHour, expectedMinute, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
+					// backwards jumps do not seem to have any problems (i.e. double activations),
+					// so they need not be handled in a similar way.
+
 					continue;
 				}
 
@@ -324,10 +356,21 @@ function CronTime(luxon) {
 					!(date.minute in this.minute) &&
 					Object.keys(this.minute).length !== 60
 				) {
-					date = date.set({
-						minute: date.minute === 59 && diff > 3600000 ? 0 : date.minute + 1
-					});
+					const expectedMinute =
+						date.minute === 59 && diff > 3600000 ? 0 : date.minute + 1;
+					const expectedHour = date.hour + (expectedMinute === 60 ? 1 : 0);
+
+					date = date.set({ minute: expectedMinute });
 					date = date.set({ second: 0 });
+
+					// Same case as with hours: DST forward jump.
+					// This must be accounted for if a minute increment pushed us to a jumping point.
+					if (this._forwardDSTJump(expectedHour, expectedMinute, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
+
 					continue;
 				}
 
@@ -335,14 +378,37 @@ function CronTime(luxon) {
 					!(date.second in this.second) &&
 					Object.keys(this.second).length !== 60
 				) {
-					date = date.set({
-						second: date.second === 59 && diff > 60000 ? 0 : date.second + 1
-					});
+					const expectedSecond =
+						date.second === 59 && diff > 60000 ? 0 : date.second + 1;
+					const expectedMinute = date.minute + (expectedSecond === 60);
+					const expectedHour = date.hour + (expectedMinute === 60 ? 1 : 0);
+
+					date = date.set({ second: expectedSecond });
+
+					// Seconds can cause it too, imagine 21:59:59 -> 23:00:00.
+					if (this._forwardDSTJump(expectedHour, expectedMinute, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
+
 					continue;
 				}
 
 				if (date.toMillis() === firstDate) {
-					date = date.set({ second: date.second + 1 });
+					const expectedSecond = date.second + 1;
+					const expectedMinute = date.minute + (expectedSecond === 60);
+					const expectedHour = date.hour + (expectedMinute === 60 ? 1 : 0);
+
+					date = date.set({ second: expectedSecond });
+
+					// Same as always.
+					if (this._forwardDSTJump(expectedHour, expectedMinute, date)) {
+						const [done, newDate] = this._findPreviousDSTJump(date);
+						date = newDate;
+						if (done) break;
+					}
+
 					continue;
 				}
 
@@ -352,6 +418,241 @@ function CronTime(luxon) {
 			return date;
 		},
 
+		/**
+		 * Search backwards in time 1 minute at a time, to detect a DST forward jump.
+		 * When the jump is found, the range of the jump is investigated to check for acceptable cron times.
+		 *
+		 * A pair is returned, whose first is a boolean representing if an acceptable time was found inside the jump,
+		 * and whose second is a DateTime representing the first millisecond after the jump.
+		 *
+		 * The input date is expected to be decently close to a DST jump.
+		 * Up to a day in the past is checked before an error is thrown.
+		 * @param date
+		 * @return [boolean, DateTime]
+		 */
+		_findPreviousDSTJump: function (date) {
+			/** @type number */
+			let expectedMinute, expectedHour, actualMinute, actualHour;
+			/** @type DateTime */
+			let maybeJumpingPoint = date;
+
+			// representing one day of backwards checking. If this is hit, the input must be wrong.
+			const iterationLimit = 60 * 24;
+			let iteration = 0;
+			do {
+				if (++iteration > iterationLimit) {
+					throw new Error(
+						`ERROR: This DST checking related function assumes the input DateTime (${date.toISO()}) is within 24 hours of a DST jump.`
+					);
+				}
+
+				expectedMinute = maybeJumpingPoint.minute - 1;
+				expectedHour = maybeJumpingPoint.hour;
+
+				if (expectedMinute < 0) {
+					expectedMinute += 60;
+					expectedHour = (expectedHour + 24 - 1) % 24; // Subtract 1 hour, but we must account for the -1 case.
+				}
+
+				maybeJumpingPoint = maybeJumpingPoint.minus({ minute: 1 });
+
+				actualMinute = maybeJumpingPoint.minute;
+				actualHour = maybeJumpingPoint.hour;
+			} while (expectedMinute === actualMinute && expectedHour === actualHour);
+
+			// Setting the seconds and milliseconds to zero is necessary for two reasons:
+			// Firstly, the range checking function needs the earliest moment after the jump.
+			// Secondly, this DateTime may be used for scheduling jobs, if there existed a job in the skipped range.
+			const afterJumpingPoint = maybeJumpingPoint
+				.plus({ minute: 1 }) // back to the first minute _after_ the jump
+				.set({ seconds: 0, millisecond: 0 });
+
+			// Get the lower bound of the range to check as well. This only has to be accurate down to minutes.
+			const beforeJumpingPoint = afterJumpingPoint.minus({ second: 1 });
+
+			if (
+				date.month in this.month &&
+				date.day in this.dayOfMonth &&
+				date.getWeekDay() in this.dayOfWeek
+			) {
+				return [
+					this._checkTimeInSkippedRange(beforeJumpingPoint, afterJumpingPoint),
+					afterJumpingPoint
+				];
+			}
+
+			// no valid time in the range for sure, units that didn't change from the skip mismatch.
+			return [false, afterJumpingPoint];
+		},
+
+		/**
+		 * Given 2 DateTimes, which represent 1 second before and immediately after a DST forward jump,
+		 * checks if a time in the skipped range would have been a valid CronJob time.
+		 *
+		 * Could technically work with just one of these values, extracting the other by adding or subtracting seconds.
+		 * However, this couples the input DateTime to actually being tied to a DST jump,
+		 * which would make the function harder to test.
+		 * This way the logic just tests a range of minutes and hours, regardless if there are skipped time points underneath.
+		 *
+		 * Assumes the DST jump started no earlier than 0:00 and jumped forward by at least 1 minute, to at most 23:59.
+		 * i.e. The day is assumed constant, but the jump is not assumed to be an hour long.
+		 * Empirically, it is almost always one hour, but very, very rarely 30 minutes.
+		 *
+		 * Assumes dayOfWeek, dayOfMonth and month match all match, so only the hours, minutes and seconds are to be checked.
+		 * @param {DateTime} beforeJumpingPoint
+		 * @param {DateTime} afterJumpingPoint
+		 * @returns {boolean}
+		 */
+		_checkTimeInSkippedRange: function (beforeJumpingPoint, afterJumpingPoint) {
+			// start by getting the first minute & hour inside the skipped range.
+			const startingMinute = (beforeJumpingPoint.minute + 1) % 60;
+			const startingHour =
+				(beforeJumpingPoint.hour + (startingMinute === 0)) % 24;
+
+			const hourRangeSize = afterJumpingPoint.hour - startingHour + 1;
+			const isHourJump = startingMinute === 0 && afterJumpingPoint.minute === 0;
+
+			// There exist DST jumps other than 1 hour long, and the function is built to deal with it.
+			// It may be overkill to assume some cases, but it shouldn't cost much at runtime.
+			// https://en.wikipedia.org/wiki/Daylight_saving_time_by_country
+			if (hourRangeSize === 2 && isHourJump) {
+				// Exact 1 hour jump, most common real-world case.
+				// There is no need to check minutes and seconds, as any value would suffice.
+				return startingHour in this.hour;
+			} else if (hourRangeSize === 1) {
+				// less than 1 hour jump, rare but does exist.
+				return (
+					startingHour in this.hour &&
+					this._checkTimeInSkippedRangeSingleHour(
+						startingMinute,
+						afterJumpingPoint.minute
+					)
+				);
+			} else {
+				// non-round or multi-hour jump. (does not exist in the real world at the time of writing)
+				return this._checkTimeInSkippedRangeMultiHour(
+					startingHour,
+					startingMinute,
+					afterJumpingPoint.hour,
+					afterJumpingPoint.minute
+				);
+			}
+		},
+
+		/**
+		 * Component of checking if a CronJob time existed in a DateTime range skipped by DST.
+		 * This subroutine makes a further assumption that the skipped range is fully contained in one hour,
+		 * and that all other larger units are valid for the job.
+		 *
+		 * for example a jump from 02:00:00 to 02:30:00, but not from 02:00:00 to 03:00:00.
+		 * @see _checkTimeInSkippedRange
+		 *
+		 * This is done by checking if any minute in startMinute - endMinute is valid, excluding endMinute.
+		 * For endMinute, there is only a match if the 0th second is a valid time.
+		 */
+		_checkTimeInSkippedRangeSingleHour: function (startMinute, endMinute) {
+			for (let minute = startMinute; minute < endMinute; ++minute) {
+				if (minute in this.minute) return true;
+			}
+
+			// Unless the very last second of the jump matched, there is no match.
+			return endMinute in this.minute && 0 in this.second;
+		},
+
+		/**
+		 * Component of checking if a CronJob time existed in a DateTime range skipped by DST.
+		 * This subroutine assumes the jump touches at least 2 hours, but the jump does not necessarily fully contain these hours.
+		 *
+		 * @see _checkTimeInSkippedRange
+		 *
+		 * This is done by defining the minutes to check for the first and last hour,
+		 * and checking all 60 minutes for any hours in between them.
+		 *
+		 * If any hour x minute combination is a valid time, true is returned.
+		 * The endMinute x endHour combination is only checked with the 0th second, since the rest would be out of the range.
+		 *
+		 * @param startHour {number}
+		 * @param startMinute {number}
+		 * @param endHour {number}
+		 * @param endMinute {number}
+		 */
+		_checkTimeInSkippedRangeMultiHour: function (
+			startHour,
+			startMinute,
+			endHour,
+			endMinute
+		) {
+			if (startHour >= endHour) {
+				throw new Error(
+					`ERROR: This DST checking related function assumes the forward jump starting hour (${startHour}) is less than the end hour (${endHour})`
+				);
+			}
+
+			/** @type number[] */
+			const firstHourMinuteRange = Array.from(
+				{ length: 60 - startMinute },
+				(_, k) => startMinute + k
+			);
+			/** @type {number[]} The final minute is not contained on purpose. Every minute in this range represents one for which any second is valid. */
+			const lastHourMinuteRange = Array.from(
+				{ length: endMinute },
+				(_, k) => k
+			);
+			/** @type number[] */
+			const middleHourMinuteRange = Array.from({ length: 60 }, (_, k) => k);
+
+			/** @type (number) => number[] */
+			const selectRange = forHour => {
+				if (forHour === startHour) {
+					return firstHourMinuteRange;
+				} else if (forHour === endHour) {
+					return lastHourMinuteRange;
+				} else {
+					return middleHourMinuteRange;
+				}
+			};
+
+			// Include the endHour: Selecting the right range still ensures no values outside the skip are checked.
+			for (let hour = startHour; hour <= endHour; ++hour) {
+				if (!(hour in this.hour)) continue;
+
+				// The hour matches, so if the minute is in the range, we have a match!
+				const usingRange = selectRange(hour);
+
+				for (const minute of usingRange) {
+					// All minutes in any of the selected ranges represent minutes which are fully contained in the jump,
+					// So we need not check the seconds. If the minute is in there, it is a match.
+					if (minute in this.minute) return true;
+				}
+			}
+
+			// The endMinute of the endHour was not checked in the loop, because only the 0th second of it is in the range.
+			// Arriving here means no match was found yet, but this final check may turn up as a match.
+			return (
+				endHour in this.hour && endMinute in this.minute && 0 in this.second
+			);
+		},
+		/**
+		 * Given expected and actual hours and minutes, report if a DST forward jump occurred.
+		 *
+		 * This is the case when the expected is smaller than the acutal.
+		 *
+		 * It is not sufficient to check only hours, because some parts of the world apply DST by shifting in minutes.
+		 * Better to account for it by checking minutes too, before an Australian of Lord Howe Island call us.
+		 * @param expectedHour
+		 * @param expectedMinute
+		 * @param {DateTime} actualDate
+		 */
+		_forwardDSTJump: function (expectedHour, expectedMinute, actualDate) {
+			const actualHour = actualDate.hour;
+			const actualMinute = actualDate.minute;
+
+			const hoursJumped = expectedHour % 24 < actualHour;
+			const minutesJumped = expectedMinute % 60 < actualMinute;
+
+			return hoursJumped || minutesJumped;
+		},
+
 		/**
 		 * wildcard, or all params in array (for to string)
 		 */


Test patch:
diff --git a/tests/crontime.test.js b/tests/crontime.test.js
index 2110c39..0120714 100644
--- a/tests/crontime.test.js
+++ b/tests/crontime.test.js
@@ -360,7 +360,164 @@ describe('crontime', () => {
 			currentDate = nextDate;
 		}
 	});
-	it('should generate the right  N next days for * * * * *', () => {
+	it('Should schedule jobs inside time zone changes that shifts time forward to the end of the shift, for weekly jobs', () => {
+		let currentDate = luxon.DateTime.fromISO('2018-03-29T23:15', {
+			zone: 'Asia/Amman'
+		});
+		const cronTime = new cron.CronTime('30 0 * * 5'); // the next 0:30 is March 30th, but it will jump from 0:00 to 1:00.
+		let nextDate = cronTime._getNextDateFrom(currentDate, 'Asia/Amman');
+		expect(nextDate - currentDate).toEqual(1000 * 60 * 45); // 45 minutes is 30T00:00, which jumps to 1:00 which is past the trigger of 0:30.
+		// the next one should just be at 0:30 again. i.e. a week minus 30 minutes.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(3600000 * 24 * 7 - 60000 * 30);
+		// the next one is again at 0:30, but now we're 'back to normal' with weekly offsets.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 3600 * 24 * 7);
+	});
+	it('Should schedule jobs inside time zone changes that shifts the time forward to the end of the shift, for daily jobs', () => {
+		let currentDate = luxon.DateTime.fromISO('2018-03-29T23:45', {
+			zone: 'Asia/Amman'
+		});
+		const cronTime = new cron.CronTime('30 0 * * *'); // the next 0:30 is March 30th, but it will jump from 0:00 to 1:00.
+		let nextDate = cronTime._getNextDateFrom(currentDate, 'Asia/Amman');
+		expect(nextDate - currentDate).toEqual(1000 * 60 * 15); // 15 minutes is 30T00:00, which jumps to 1:00 which is past the trigger of 0:30.
+		// the next one is tomorrow at 0:30, so 23h30m.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 3600 * 24 - 1000 * 60 * 30);
+		// back to normal.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 3600 * 24);
+	});
+	it('Should schedule jobs inside time zone changes that shifts the time forward to the end of the shift, for hourly jobs', () => {
+		let currentDate = luxon.DateTime.fromISO('2018-03-29T23:45', {
+			zone: 'Asia/Amman'
+		});
+		const cronTime = new cron.CronTime('30 * * * *'); // the next 0:30 is March 30th, but it will jump from 0:00 to 1:00.
+		let nextDate = cronTime._getNextDateFrom(currentDate, 'Asia/Amman');
+		expect(nextDate - currentDate).toEqual(1000 * 60 * 15); // 15 minutes is 30T00:00, which jumps to 1:00 which is past the trigger of 0:30.
+		// the next one is at 1:30, so 30m.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 60 * 30);
+		// back to normal.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 3600);
+	});
+	it('Should schedule jobs inside time zone changes that shifts the time forward to the end of the shift, for minutely jobs', () => {
+		let currentDate = luxon.DateTime.fromISO('2018-03-29T23:59', {
+			zone: 'Asia/Amman'
+		});
+		const cronTime = new cron.CronTime('* * * * *'); // the next minute is 0:00 is March 30th, but it will jump from 0:00 to 1:00.
+		let nextDate = cronTime._getNextDateFrom(currentDate, 'Asia/Amman');
+		expect(nextDate - currentDate).toEqual(1000 * 60);
+		// the next one is at 1:01:00, this should still be 60 seconds in the future.
+		currentDate = nextDate;
+		nextDate = cronTime._getNextDateFrom(currentDate);
+		expect(nextDate - currentDate).toEqual(1000 * 60);
+	});
+	// Do not think a similar test for secondly job is necessary, the minutely one already ensured no double hits in the overlap zone.
+	it('Should throw when dates that are not within 24 hours of DST jumps are checked for past DST jumps', () => {
+		const cronTime = new cron.CronTime('* * * * *');
+		const tryFindPastDST = isoTime => () => {
+			const maybeBadDate = luxon.DateTime.fromISO(isoTime, {
+				zone: 'Asia/Amman'
+			});
+			expect(maybeBadDate.invalid).toEqual(null);
+			cronTime._findPreviousDSTJump(maybeBadDate);
+		};
+
+		// This timezone jumps from 0:00 to 1:00 on March 30th, so the cutoff is March 31st 1:00:00
+		expect(tryFindPastDST('2018-03-31T00:59:00')).not.toThrow();
+		expect(tryFindPastDST('2018-03-31T01:00:00')).toThrow();
+	});
+	// The following few DST related tests do not need specific dates that are actually DST,
+	// the functions they are calling assume the given parameters encapsulate a DST jump,
+	// and use the raw hour and minute data to check it from there.
+	it('Should correctly scan time periods as if they are DST jumps, half hour jumps', () => {
+		let endDate = luxon.DateTime.fromISO('2023-01-01T16:00:00.000', {
+			zone: 'Europe/Amsterdam'
+		});
+		let startDate = endDate.minus({ minute: 30, second: 1 });
+		const cronTime = new cron.CronTime('5 16 * * *'); // at 16:05:00.
+		let jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+
+		endDate = endDate.plus({ minute: 30 }); // 16:30:00
+		startDate = endDate.minus({ minute: 30, second: 1 }); // 15:59:59
+		jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(true);
+	});
+	it('Should not include seconds in the minute after the DST jump as part of the jump scan', () => {
+		const endDate = luxon.DateTime.fromISO('2023-01-01T16:00:00.000', {
+			zone: 'Europe/Amsterdam'
+		});
+		// 1 hour jump case
+		let startDate = endDate.minus({ hour: 1, second: 1 });
+		const cronTime = new cron.CronTime('1 5 16 * * *'); // at 16:00:01.
+		let jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+		// 'quirky' jump case
+		startDate = endDate.minus({ hour: 1, minute: 45, second: 1 });
+		jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+	});
+	it('Should correctly scan time periods as if they are DST jumps, full hour jumps', () => {
+		let endDate = luxon.DateTime.fromISO('2023-01-01T16:00:00.000', {
+			zone: 'Europe/Amsterdam'
+		});
+		let startDate = endDate.minus({ hour: 1, second: 1 });
+		const cronTime = new cron.CronTime('5 16 * * *'); // at 16:05:00.
+		let jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+
+		endDate = endDate.plus({ hour: 1 }); // 17:00:00
+		startDate = endDate.minus({ hour: 1, second: 1 }); // 15:59:59
+		jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(true);
+	});
+	// A 'quirky' DST jump is one that should not break the implementation, but does not exist in real life (yet)
+	it('Should correctly scan time periods as if they are DST jumps, quirky jumps (1)', () => {
+		// Testing a jump that is less than an hour long, but wraps around an hour.
+		let endDate = luxon.DateTime.fromISO('2023-01-01T16:15:00.000', {
+			zone: 'Europe/Amsterdam'
+		});
+		let startDate = endDate.minus({ minute: 45, second: 1 }); // 15:29:59
+		const cronTime = new cron.CronTime('30 16 * * *'); // at 16:30:00.
+		let jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+
+		endDate = endDate.plus({ minute: 30 }); // 16:45:00
+		startDate = endDate.minus({ minute: 50, second: 1 }); // 15:54:59
+		jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(true);
+	});
+	it('Should correctly scan time periods as if they are DST jumps, quirky jumps (2)', () => {
+		// Testing a jump that is over an hour long.
+		let endDate = luxon.DateTime.fromISO('2023-01-01T16:15:00.000', {
+			zone: 'Europe/Amsterdam'
+		});
+		let startDate = endDate.minus({ hour: 3, minute: 45, second: 1 }); // 12:29:59
+		const cronTime = new cron.CronTime('30 16 * * *'); // at 16:30:00.
+		let jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(false);
+
+		endDate = endDate.plus({ minute: 30 }); // 16:45:00
+		startDate = endDate.minus({ hour: 3, minute: 45, second: 1 }); // 12:59:59
+		jobInRange = cronTime._checkTimeInSkippedRange(startDate, endDate);
+		expect(jobInRange).toEqual(true);
+	});
+	it('Enforces the hour difference assumption for handling multi-hour DST jumps', () => {
+		const cronTime = new cron.CronTime('30 16 * * *');
+		expect(() => {
+			cronTime._checkTimeInSkippedRangeMultiHour(15, 0, 15, 30);
+		}).toThrow();
+	});
+	it('should generate the right N next days for * * * * *', () => {
 		const cronTime = new cron.CronTime('* * * * *');
 		let currentDate = luxon.DateTime.local().set({ second: 0, millisecond: 0 });
 		for (let i = 0; i < 100; i++) {