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

Allow mock Date object in isDate() check #857

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jamesisaac
Copy link

When carrying out automated testing (e.g. with Selenium), and wanting to test different timezones, it's common to mock the global Date object with a library like: https://github.com/plaa/TimeShift-js

Unfortunately, the isDate check in pickadate has a very strict check, which rejects anything that isn't a native JS Date object, rendering this mocking method unusable.

I propose making this check more lenient, so as long as the object looks like it matches the Date API, it's allowed through.

The issue manifests itself in an interesting way -- you end up with a calendar where, in the best case, every single date shows as today, and in the worst case, browser freezes up in an infinite loop.

@jamesisaac
Copy link
Author

Not sure why Travis CI is failing - can't imagine how this would break things. Maybe to do with it being the last day of the month?

>> Message: Able to disable today
>> Actual: [
>>   2016,
>>   2,
>>   32
>> ]
>> Expected: [
>>   2016,
>>   3,
>>   1
>> ]

@DanielRuf
Copy link
Collaborator

It seems it was due to the overflow. You can check this with the current master on your machine by changing the system clock.

@DanielRuf DanielRuf self-assigned this Feb 24, 2018
@DanielRuf DanielRuf self-requested a review April 1, 2018 21:59
Copy link
Collaborator

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

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

Please rebase on the current head of master

@jamesisaac
Copy link
Author

@DanielRuf I believe that's done now. I've also ticked "Allow edits from maintainers." in case you want to change something further yourself. Thanks.

@DanielRuf DanielRuf self-requested a review May 3, 2018 21:31
Copy link
Collaborator

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

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

The changes probably cause the failing unit tests.

Can you investigate and test them?

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.

2 participants