rmurphey / js-assessment

A test-driven approach to assessing JS skills
5.15k stars 2.05k forks source link

Add sinon.js to make use of fake timer #119

Closed jacobroufa closed 9 years ago

jacobroufa commented 9 years ago

With the intent to resolve #114 per @jmeas' suggestion, this commit adds sinon.js and modifies the count.js test to use fake timers.

ashleygwilliams commented 9 years ago

awesome. waiting on #112 to merge, will likely need a rebase :wink:

jacobroufa commented 9 years ago

@jmeas I could build that out if you like. i don't know that it's necessary, but you're right that it will be far more precise and useful in that way.

jacobroufa commented 9 years ago

@ashleygwilliams I've merged this with the changes from #112.

@jmeas Let me know if the solution in e896f70 satisfies your requirements.

jacobroufa commented 9 years ago

/me shrugs -- No particular reason.

ashleygwilliams commented 9 years ago

@jacobroufa i think it might be more explicit to have this.clock.tick(100) than storing t as a var in the for loop

otherwise i'm good with this :+1:

jacobroufa commented 9 years ago

Fixed. Let me know if you need anything else! :+1:

jacobroufa commented 9 years ago

@jmeas removed the call to done().

jacobroufa commented 9 years ago

Eeek! Sorry!! That was a total bonehead maneuver.

ashleygwilliams commented 9 years ago

@jmeas can you do the final review and merge on this when it's ready? i think you have a better vision of the final than i do

e0da commented 9 years ago

I know I'm late, but :+1:. I prefer this more complete solution to my little band-aid patch in #114. :)

jacobroufa commented 9 years ago

@jmeas have you had a chance to look this over? Is there anything else I need to do prior to merge?

jamesplease commented 9 years ago

Yo! Sorry it took me so long to review this! This is really dope, @jacobroufa . Thanks for puttin' the work together :D

ashleygwilliams commented 9 years ago

:sparkling_heart: :sparkles: