myrne / performance-now

Implements performance.now (based on process.hrtime).
MIT License
162 stars 26 forks source link

Build broken with v2 release #7

Closed jasonray closed 7 years ago

jasonray commented 7 years ago

EDITED: The master build is currently broken reporting a failed test following the recent v2 modifications.

https://travis-ci.org/braveg1rl/performance-now/builds/189923759

It looks like these are just tweaks to the acceptance criteria (loosening the expectation), but I want to be sure before upgrading.

myrne commented 7 years ago

I think calling the build "broken" is a bit too much. One or more of the tests on all but one of the various test runs Travis CI servers fail. One of the test runs succeed.

It's because the values reported by the scripts are highly variable.

I think the failing tests only reflect some unwise choices on my part on how to assure myself I was writing the good code. I wrote the tests for the very purpose of moving safely from v1.0 to v2.0. First, I made tests that passed for the v1.0 logic, then saw them fail for the 2.0 logic (as expected), and then adjusted them to make them pass again.

Based on my extensive testing, I think the logic of v2.0 is absolutely sound.

I very much need to reconsider how I do the testing, though.

jasonray commented 7 years ago

Excellent, your conclusion is in-line with mine. I appreciate your feedback!