8000 Make Timecop an npm module by austinbv · Pull Request #17 · jamesarosen/Timecop.js · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 20, 2019. It is now read-only.

Make Timecop an npm module #17

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

Conversation

austinbv
Copy link

This change was very minimal, mostly changes to the package.json since timecop.js is already exports properly. The build task bumps versions.

To test you can either npm install from my master branch or just require('./timecop') in node in the root of the project.

All test pass

8000

This change was very minimal, mostly changes to the package.json since timecop.js is already exports properly.  The build task bumps versions.
@stevegraham
Copy link

+1

@austinbv
Copy link
Author
austinbv commented Sep 6, 2012

i have another commit ready to go it is just dependent on nodejs/node-v0.x-archive#3710

@jamesarosen
Copy link
Owner

It looks like that node thread is both very long and very dead. I'm all for making this node-compatible, but it seems that community doesn't like the idea of time travel in tests.

@stevegraham
Copy link

dependency injection for tests is a code smell.

@jamesarosen
Copy link
Owner

@stevegraham I think what you mean is

"The API I want for my Foo library is foo.inNHours(n, fn), but I want to test it without making the tests take three hours to run. One solution is to change the API to foo.inNHours(n, fn, from = new Date()), but that means changing the API to support dependency injection just to make the tests happy, which is bad."

Am I reading you right?

@stevegraham
Copy link

IMO the latter is bad. The last arg is only there for the sake of testing; the outside world does not need to know about the date inside that function. As an example, JWT tokens depend on time and it's useful to mock time when working with them, e.g. writing code that generates them. Other people may disagree with me, and that's fine, let them use dependency injection in lieu of mocks; but a lot of developers are used to the flexibility that mocks provide and I think that it's bad to remove that option because of the opinions of a vocal subset of developers.

@stevegraham
Copy link

Timecop.js is great and deserves to be a first class npm module. IMHO.

@jamesarosen
Copy link
Owner

Is there something we can do here to get around the problems that arise from changing globals.Date in node?

@mackermedia
Copy link

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0