Make Timecop an npm module #17

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

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

@austinbv austinbv Added all necessary resources for an NPM module
This change was very minimal, mostly changes to the package.json since timecop.js is already exports properly.  The build task bumps versions.
d4a0dc2

austinbv commented Sep 6, 2012

i have another commit ready to go it is just dependent on joyent/node#3710

Owner

jamesarosen commented Jan 21, 2013

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.

dependency injection for tests is a code smell.

Owner

jamesarosen commented Jan 22, 2013

@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?

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.

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

Owner

jamesarosen commented Jan 22, 2013

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment