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

CronTriggerImpl.WillFireOn returns wrong result when TimeZone is specified #1187

Closed
odesyatnyk opened this issue Apr 30, 2021 · 2 comments
Closed
Milestone

Comments

@odesyatnyk
Copy link
Contributor

odesyatnyk commented Apr 30, 2021

Hi all

Trying to cover with tests some of mine CRON expressions i ran into an issue that WillFireOn method of CronTriggerImpl returns false when it shouldn't

Version used

.NET Core 3.1, Quartz 3.3.2

To Reproduce

Listed CRON expression is meant to fire everyday at 5:50, 11:50, 17:50 and 23:50 (all GMT)

I have created simple example of test where you can reproduce the issue. Assert.IsTrue fails, when the next line Assert.AreEqual passes the test that in fact the same thing and both should pass.

using NUnit.Framework;
using Quartz;
using Quartz.Impl.Triggers;
using System;

namespace TestProject
{
    public class Tests
    {
        private static readonly TimeSpan Offset = TimeZoneInfo.FindSystemTimeZoneById("GMT Standard Time").BaseUtcOffset;
        private static readonly DateTime StartDate = new DateTime(2021, 1, 1, 0, 0, 0, DateTimeKind.Utc);
        private const string TimezoneGMT = "GMT Standard Time";

        [Test]
        public void Test_GMT_CRON()
        {
            var trigger = TriggerBuilder.Create()
                .WithIdentity("name", TriggerKey.DefaultGroup)
                .WithCronSchedule("0 50 5,11,17,23 ? * *", x =>
                {
                    x.InTimeZone(TimeZoneInfo.FindSystemTimeZoneById(TimezoneGMT));
                    x.WithMisfireHandlingInstructionDoNothing();
                })
                .StartAt(StartDate)
                .Build() as CronTriggerImpl;

            var expected = new DateTimeOffset(StartDate.AddHours(5).AddMinutes(50), Offset);

            Assert.IsTrue(trigger.WillFireOn(expected));
            Assert.AreEqual(expected, trigger.GetFireTimeAfter(StartDate));
        }
    }
}

Assumption
I think the problem is in CronTriggerImpl.cs

At the very first line of the method it overwrites value of test losing offset in this case, so later it is not able to compare correctly.

public bool WillFireOn(DateTimeOffset test, bool dayOnly)
{
    test = new DateTime(test.Year, test.Month, test.Day, test.Hour, test.Minute, test.Second);
    ...
}
odesyatnyk added a commit to odesyatnyk/quartznet that referenced this issue May 1, 2021
make WillFireOn method of CronTriggerImpl to keep the offset of test date
@lahma lahma added this to the 3.3.3 milestone May 5, 2021
@lahma
Copy link
Member

lahma commented May 5, 2021

Thanks for the detailed report and also for fixing the issue, much appreciated!

@lahma lahma closed this as completed May 5, 2021
@odesyatnyk
Copy link
Contributor Author

Thanks for the detailed report and also for fixing the issue, much appreciated!

My pleasure! Thank you for accepting and for feedback!

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

No branches or pull requests

2 participants