-
Notifications
You must be signed in to change notification settings - Fork 7k
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
[bugfix] Fix endOf for daylight saving time #4254
Conversation
src/test/moment/start_end_of.js
Outdated
test('endOf DST day', function (assert) { | ||
// Based on a real story somewhere in Brazil/Sao_Paulo | ||
var dstChangeDay = moment('2017-10-15T12:00:00-03:00'); | ||
var dstChangeDayAfter = moment('2017-10-16T12:00:00-03:00'); |
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.
Assuming you mean America/Sao_Paulo
, the offset would be -02:00
the day after.
The problem here is that this test will be running on whatever the local time zone happens to be - not necessarily in Brazil.
I think though the code change needs to happen in moment, we would need to bring moment-timezone into the unit tests (or move the tests into moment-timezone) in order to start testing DST changes.
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.
I copied another test in same file to make my, both should be moved?
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.
Should I remove the test and left just the code?
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.
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.
@mj1856 , removed tests. I'll try to make them on https://github.com/moment/moment-timezone
At timezone GMT-3, in Brazil, the last day before timezone change had one more hour than it should have.
Seems like it's not needed anymore. |
The endOf formula was not working for the day that the daylight saving
time starts.
Couldn't run tests locally, but running the test code worked.
resolves #4249