-
Notifications
You must be signed in to change notification settings - Fork 605
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
Date: Create unit tests to verify day parsing uses the correct day range given its corresponding month/year #374
Conversation
In order to properly test the #323 fix (PR #373), I needed to overwrite |
Why not just use Sinon.JS? http://sinonjs.org/ |
Specifically the clock feature: http://sinonjs.org/docs/#clock var clock = sinon.useFakeTimers(now); Where |
…in the correct range by taking into account leap years
lolex's 3664eb8d90dd1d6fa04a098f77b77b19a6bb5850 = 1.1.0 (present on npm, but not git tagged accordingly). Create unit tests to verify day of year parsing uses the correct day range given its corresponding year.
Awesome. I've updated this PR to use Sinon.js instead of my custom implementation on tests. |
Looks good to me. |
Testing this PR+Sinon.js across various browsers resulted in errors/failures (I've merged in the browserstack-runner branch locally to test it). Running browserstack-runner against my custom implementation passes just fine. Potentially, the Sinon.js failures are only related to AMD loading, giving the fact some failing tests intermittently pass. |
We could land this PR + Sinon.js now with no problem. But, it would impact (i.e. delay) testing automation #235 (i.e., to land browserstack-runner branch #367), since we would have to figure out what this failures were. Giving test automation (which is due to 1.0.0) and #323 have higher priorities compared to using Sinon.js to simplify FakeDate here on tests, I'm considering to track that as a separate issue unless anyone has a different idea. So, we can land #373 (fixing #323), land browserstack-runner branch. Then, spend time on it. What do you think? |
Is the sinon/browserstack failure consistent? Can you reproduce it when running the given browser manually? If we're not getting anywhere, I'm okay with keeping |
It's not consistent. I haven't tried it manually. The problem could be simple. But, it will require some time investigating it. I just think this time is best spent somewhere else now. :) |
Okay, let's give sinon another try later. |
remove usage of Thread.current for fallbacks
Fixes #323
Ref #373