Merge pull request #45 from alexreardon/fix-flakey-test-lite

Avoid flakey test - fixes #42

README.md

@@ -101,8 +101,12 @@ describe('stub', () => {
 
 ## Installation
 
-```
-npm i --save-dev raf-stub
+```bash
+## npm
+npm install raf-stub --save-dev
+
+## yarn
+yarn add raf-stub --dev
 ```
 
 ## stub
@@ -440,6 +444,70 @@ This project used [Semantic versioning 2.0.0](http://semver.org/) to ensure a co
 
 A safe `raf-stub` `package.json` dependency would therefore be anything that allows changes to the *minor* or *patch* version
 
+## Frame `currentTime` precision warning
+
+When you a frame is called by `.step()` or `.flush()` it [is given the `currentTime` as the first argument](https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame).
+
+```js
+const stub = createStub();
+const callback = currentTime => console.log(`the time is ${currentTime}`);
+
+stub.add(callback);
+stub.step();
+
+// console.log('the current time is 472759.63');
+```
+
+By default `frameDuration` is `1000 / 60` and startTime is `performance.now`. Both of these numbers are ugly decimals (eg `16.6666667`). When they are added together in `.step()` or `.flush()` this can cause [known precision issues in JavaScript](https://github.com/getify/You-Dont-Know-JS/blob/master/types%20%26%20grammar/ch2.md#small-decimal-values). You can find some further discussion about it's impact [here](https://github.com/alexreardon/raf-stub/issues/42).
+
+**Work arounds**
+If you want to assert the current time inside of a callback - be sure to *add* the expected time values:
+
+```js
+const frameDuration = 1000 / 60;
+const startTime = performance.now();
+const stub = createStub(frameDuration, startTime);
+const child = sinon.stub();
+const parent = sinon.stub().returns(child);
+
+stub.add(callback);
+stub.step();
+stub.step();
+
+// okay
+expect(parent.calledWith(startTime + frameDuration)).to.be.true;
+
+// not okay - will not mimic precision issues
+// doing this can lead to flakey tests
+expect(child.calledWith(startTime + 2 * frameDuration)).to.be.true;
+
+// okay
+expect(child.calledWith(startTime + frameDuration + frameDuration));
+```
+
+Another simple option is to use integers for **both** `frameDuration` and `startTime`.
+
+```js
+const frameDuration = 16;
+const startTime = 100;
+const stub = createStub(frameDuration, startTime);
+const child = sinon.stub();
+const parent = sinon.stub().returns(child);
+
+stub.add(callback);
+stub.step();
+stub.step();
+
+// okay
+expect(parent.calledWith(startTime + frameDuration)).to.be.true;
+
+// okay
+expect(child.calledWith(startTime + 2 * frameDuration)).to.be.true;
+
+// okay
+expect(child.calledWith(startTime + frameDuration + frameDuration));
+```
+
 ## Recipes
 
 ## `frameDuration` and `startTime`
@@ -589,6 +657,7 @@ require('raf-stub').replaceRaf();
 ```
 
 Then everything will work as expected!
+
 ```js
 // test.js
 import render from 'library';

package.json

@@ -28,7 +28,7 @@
   },
   "scripts": {
     "test": "yarn run lint && yarn run typecheck && yarn run test:fast",
-    "test:fast": "mocha test --compilers js:babel-core/register",
+    "test:fast": "mocha test --compilers js:babel-core/register --globals global",
     "typecheck": "flow check",
     "lint": "eslint src test -",
     "lint:fix": "yarn run lint --fix",

src/constants.js

@@ -0,0 +1 @@
+export const defaultFrameDuration = 1000 / 60;

src/index.js

@@ -1,24 +1,29 @@
 // @flow
-const now = require('performance-now');
-const defaultDuration: number = 1000 / 60;
+import now from 'performance-now';
+import { defaultFrameDuration } from './constants';
 
 type Stub = {|
     add: (cb: Function) => number,
     remove: (id: number) => void,
-    flush: (duration: ?number) => void,
+    flush: (duration?: number) => void,
     reset: () => void,
-    step: (steps?: number, duration?: ?number) => void
+    step: (steps?: number, duration?: number) => void
 |};
 
-export default function createStub (frameDuration: number = defaultDuration, startTime: number = now()): Stub {
-    const frames = [];
-    let frameId = 0;
-    let currentTime = startTime;
+type Frame = {|
+    id: number,
+    callback: Function
+|};
+
+export default function createStub (frameDuration: number = defaultFrameDuration, startTime: number = now()): Stub {
+    const frames: Frame[] = [];
+    let frameId: number = 0;
+    let currentTime: number = startTime;
 
     const add = (cb: Function): number => {
         const id = ++frameId;
 
-        const callback = (time) => {
+        const callback = (time: number) => {
             cb(time);
             // remove callback from frames after calling it
             remove(id);
@@ -43,7 +48,7 @@ export default function createStub (frameDuration: number = defaultDuration, sta
         frames.splice(index, 1);
     };
 
-    const flush = (duration?: ?number = frameDuration): void => {
+    const flush = (duration?: number = frameDuration): void => {
         while (frames.length) {
             step(1, duration);
         }
@@ -54,11 +59,13 @@ export default function createStub (frameDuration: number = defaultDuration, sta
         currentTime = startTime;
     };
 
-    const step = (steps?: number = 1, duration?: ?number = frameDuration): void => {
+    const step = (steps?: number = 1, duration?: number = frameDuration): void => {
         if (steps === 0) {
             return;
         }
 
+        // This line can cause a precision error if both `currentTime`
+        // and `duration` are numbers with decimal components.
         currentTime = currentTime + duration;
 
         const shallow = frames.slice(0);
@@ -93,7 +100,7 @@ declare function replaceRaf(...rest: Array<Object>): void;
 declare function replaceRaf(roots?: Object[], options?: ?ReplaceRafOptions): void;
 
 // all calls to replaceRaf get the same stub;
-export function replaceRaf(roots?: Object[] = [], { duration = defaultDuration, startTime = now() }: ReplaceRafOptions = {}) {
+export function replaceRaf(roots?: Object[] = [], { duration = defaultFrameDuration, startTime = now() }: ReplaceRafOptions = {}) {
     // 0.3.x api support
     if (arguments.length && !Array.isArray(roots)) {
         console.warn('replaceRaf(roots) has been depreciated. Please now use replaceRaf([roots], options). See here for more details: https://github.com/alexreardon/raf-stub/releases');

test/index.js

@@ -1,11 +1,10 @@
 // @flow
-import createStub, { replaceRaf } from '../src';
 import sinon from 'sinon';
 import { expect } from 'chai';
 import { it, beforeEach, afterEach, describe } from 'mocha';
 import now from 'performance-now';
-
-const defaultDuration = 1000 / 60;
+import createStub, { replaceRaf } from '../src';
+import { defaultFrameDuration } from '../src/constants';
 
 describe('createStub', () => {
     it('should allow for different stub namespaces', () => {
@@ -309,7 +308,8 @@ describe('instance', () => {
             api.flush();
 
             expect(parent.calledWith(startTime + frameDuration)).to.be.true;
-            expect(child.calledWith(startTime + 2 * frameDuration)).to.be.true;
+            // double adding to replicate multiple addition precision issues
+            expect(child.calledWith(startTime + frameDuration + frameDuration)).to.be.true;
         });
 
         it('should allow you to flush callbacks with a provided frame duration', () => {
@@ -323,7 +323,8 @@ describe('instance', () => {
             api.flush(customDuration);
 
             expect(parent.calledWith(startTime + customDuration)).to.be.true;
-            expect(child.calledWith(startTime + 2 * customDuration)).to.be.true;
+            // double adding to replicate multiple addition precision issues
+            expect(child.calledWith(startTime + customDuration + customDuration)).to.be.true;
         });
 
     });
@@ -493,13 +494,13 @@ describe('replaceRaf', () => {
             root.requestAnimationFrame(callback);
             root.requestAnimationFrame.flush();
 
-            expect(callback.calledWith(startTime + defaultDuration)).to.be.true;
+            expect(callback.calledWith(startTime + defaultFrameDuration)).to.be.true;
         });
 
         it('should use the custom duration if none is provided', () => {
             const root = {};
             const callback = sinon.stub();
-            const customDuration = defaultDuration * 1000;
+            const customDuration = defaultFrameDuration * 1000;
 
             replaceRaf([root], {
                 startTime,
@@ -524,7 +525,7 @@ describe('replaceRaf', () => {
             root.requestAnimationFrame(callback);
             root.requestAnimationFrame.flush();
 
-            expect(callback.calledWith(customStartTime + defaultDuration)).to.be.true;
+            expect(callback.calledWith(customStartTime + defaultFrameDuration)).to.be.true;
         });
     });
 
@@ -553,6 +554,7 @@ describe('replaceRaf', () => {
 
             expect(root1.requestAnimationFrame).to.equal(root2.requestAnimationFrame);
         });
+
         it('should log a deprecation message', () => {
             const root = {};
 
@@ -561,4 +563,4 @@ describe('replaceRaf', () => {
             expect(console.warn.called).to.be.true;
         });
     });
-});
+});