Skip to content

fix: failing tests with dates in local time #4412

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

Merged

Conversation

maciej-ka
Copy link
Contributor

Good morning!

This is a fix for tests that fail because expected dates ignore that machine local time can be in non zero timezones.

Closes: #4411

@maciej-ka
Copy link
Contributor Author

Locally test pass but Travis is failing.
I think it waits for a bribe payment.

@codecov
Copy link

codecov bot commented Dec 2, 2017

Codecov Report

Merging #4412 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4412      +/-   ##
=========================================
+ Coverage   92.79%   92.8%   +0.01%     
=========================================
  Files         119     119              
  Lines        8522    8522              
=========================================
+ Hits         7908    7909       +1     
+ Misses        614     613       -1
Impacted Files Coverage Δ
src/RestWrite.js 93.23% <0%> (ø) ⬆️
src/Routers/PushRouter.js 96.42% <0%> (+3.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8f3fb1...ba89f2a. Read the comment docs.

@addisonElliott
Copy link
Contributor

I can confirm I receive this error message when building tests locally as well.

@@ -1200,10 +1200,12 @@ describe('PushController', () => {
isLocalTime: false
})).toBe('2007-04-05T14:30:00.000Z', 'Timezone offset');

const noTimezone = new Date('2017-09-06T17:14:01.048')
Copy link
Contributor

Choose a reason for hiding this comment

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

@marvelm could you have a look please? As you were originally building that feature.

Thanks!

Copy link
Contributor

@addisonElliott addisonElliott left a comment

Choose a reason for hiding this comment

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

I've checked both of these changes and they are consistent with what should be happening.

PushController.formatPushTime converts to UTC and then strips the timezone if isLocalTime is set to true. The issue here is that the test expects the hour to be the same, but if the local timezone is not set to UTC, the hour will be different.

This change fixes the issue and two tests run as a result.

I submitted a PR to merge the latest changes into this branch and then it should be good to merge imo.
maciej-ka#1

@acinader acinader merged commit 9db63a4 into parse-community:master Mar 1, 2018
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.

Tests fail on machine in CET timezone
4 participants