search

CrowdHailer / fn.js

A JavaScript library built to encourage a functional programming style & strategy. - http://eliperelman.com/fn.js
MIT License
399 stars 30 forks source link

delay/delayFor test can fail non-deterministically #4

Closed kjgorman closed 10 years ago

kjgorman commented 10 years ago

The delay/delayFor test is looking to ensure that the delay was at least the provided timeout value (of 50). However it is possible (on my i5 os x 10.8.2 MBA, node 0.8.16) for the callback to trigger such that the two Date.now() calls to occur with only 49ms elapsed.

I think this test should be updated to reflect that the underlying setTimeout mechanism can have varying behaviour based on when the event loop gets a processor timeslice. Perhaps allowing values like 49 <= elapsed <= 51 to pass the test. e.g.: screen shot 2014-01-26 at 9 01 37 pm

eliperelman commented 10 years ago

Hello @kjgorman. Can you please check that b4a941052f4eae9a9b7d9156cea930c1163e240c resolves your issue? This is the latest in master.

kjgorman commented 10 years ago

Hello, yep, that seems enough, can't repro after 20 grunts, where I could reliably repro within 5-10 prior. Out of curiosity, why not to.be.within(49,51) (within)?

eliperelman commented 10 years ago

Hey @kjgorman. Glad to hear that fix worked for you. I had originally thought of going with within, but then decided against putting an upper bound on when the delayed handler would actually be invoked on the event loop. Typically it would be invoked within 1ms, but I didn't want to assume that it always would be.