-
Notifications
You must be signed in to change notification settings - Fork 124
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
Add test for default timezone offset of DateTime #1423
Conversation
Formatting check succeeded! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1423 +/- ##
=========================================
Coverage 64.38% 64.39%
+ Complexity 1921 1920 -1
=========================================
Files 494 494
Lines 28020 28020
Branches 5559 5559
=========================================
+ Hits 18042 18044 +2
Misses 7737 7737
+ Partials 2241 2239 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only the test, but doesn't the DateTimeEvaluator need to be updated as well?
I walked through the code with Anton. The code looks correct to me. I think I was wrong before, the DateTimeEvaluator is using the timezone of the evaluation request. |
Hmm.... I walked through this the other day and it wasn't, just did again and I don't see how it is using the evaluation request timestamp: |
The only place that's used is here: Either the ELM has the offset embedded, or the current offset from the State object is used. That state is set up as part of the evaluation. |
Quality Gate passedIssues Measures |
I see, I was assuming the evaluator tree was still controlling the visit, but I see that it's the visitor now. |
Closes #1420.
This PR adds tests for calling
DateTime(y, m, d, h, m, s, ms)
without specifying the timezone offset. As per the spec, it should default to the timezone offset of the evaluation request.Note that when
engine.expression(...)
orengine.evaluate(...)
is called without passing in the evaluation date time explicitly, it is set toZonedDateTime.now()
.