Conversation
|
I see that the Node 8 says they failed, but I just reran it and it's fine. I'll take an actual look at this PR (and your other one) for merging later tonight. Thanks! |
test/timer.js
Outdated
|
|
||
| assert.equal(name, 'name-thingy'); | ||
| assert.ok(parseFloat(time) >= 100); | ||
| assert.ok(parseFloat(time) < 110); |
There was a problem hiding this comment.
I have the same worry here about creating a jittery test, perhaps a little higher?
|
One small comment. Thank you for all of your work here, great to see! Heads up @DavidWittman that there's another solution that came through here. |
|
sure thing @bdeitte I didn't want to step on any toes, but i was curious about the structure of the tests. Seems that the specific including of the test modules and wrapping in a top level describe is redundant if the recursive option for mocha is used. I only noticed because my IDE wont recognise the mocha tests and let me easily run a single one etc. Id be happy to refactor the tests a little if thats something that would be acceptable ? |
|
@chrismatheson Always happy to take improvements. :) The tests were recently refactored to not be in one huge mess of a file, in #79. So it hasn't been around for long in the current form and if there's improvements to be made, I'll always review and pull in. |
2 participants
introduction of function which understands async return and instruments appropriately
should resolve #66