From 2d1f2c575727c47c7efee05759240b0078c0577f Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sat, 28 Nov 2015 12:00:47 +0100 Subject: [PATCH 1/6] Create Async-tests + the first few tests for Select.Async, using the shallow renderer + removed redundant asyncOptions tests from Select-test + Changes Select.Async thenPromise to return the resulting promise if handled with promises - no change to usage, but makes testing easier as it is possible to just wait for the returned promise from thenPromise to complete before asserting the changes. + Added dev dependency to unexpected-react, to ease testing with the shallow renderer --- package.json | 3 +- src/Async.js | 4 +- test/Async-test.js | 215 +++++++++++++++++++++++++ test/Select-test.js | 380 -------------------------------------------- 4 files changed, 219 insertions(+), 383 deletions(-) create mode 100644 test/Async-test.js diff --git a/package.json b/package.json index 16e730a9e7..c840c7076a 100644 --- a/package.json +++ b/package.json @@ -27,12 +27,13 @@ "mocha": "^2.3.3", "react": "^0.14.1", "react-addons-test-utils": "^0.14.1", + "react-component-gulp-tasks": "^0.7.7", "react-dom": "^0.14.1", "react-gravatar": "^2.2.2", - "react-component-gulp-tasks": "^0.7.7", "sinon": "^1.17.2", "unexpected": "^10.0.2", "unexpected-dom": "^3.0.2", + "unexpected-react": "^0.3.0", "unexpected-sinon": "^8.0.0" }, "peerDependencies": { diff --git a/src/Async.js b/src/Async.js index 74d7087e94..78b645d5d5 100644 --- a/src/Async.js +++ b/src/Async.js @@ -29,7 +29,7 @@ function getFromCache (cache, input) { function thenPromise (promise, callback) { if (!promise || typeof promise.then !== 'function') return; - promise.then((data) => { + return promise.then((data) => { callback(null, data); }, (err) => { callback(err); @@ -118,7 +118,7 @@ const Async = React.createClass({ isLoading: true, }); let responseHandler = this.getResponseHandler(input); - thenPromise(this.props.loadOptions(input, responseHandler), responseHandler); + return thenPromise(this.props.loadOptions(input, responseHandler), responseHandler); }, render () { let { noResultsText } = this.props; diff --git a/test/Async-test.js b/test/Async-test.js new file mode 100644 index 0000000000..ba04f51ef5 --- /dev/null +++ b/test/Async-test.js @@ -0,0 +1,215 @@ +var unexpected = require('unexpected'); +var unexpectedReact = require('unexpected-react'); +var unexpectedSinon = require('unexpected-sinon'); +var expect = unexpected + .clone() + .installPlugin(unexpectedReact) + .installPlugin(unexpectedSinon); + +var React = require('react'); +var ReactDOM = require('react-dom'); +var TestUtils = require('react-addons-test-utils'); +var sinon = require('sinon'); + + +var Select = require('../src/Select'); +if (!global.document) { + global.document = {}; +} + +describe('Async', () => { + + var renderer; + var loadOptions; + + const typeSearchText = function(text) { + const output = renderer.getRenderOutput(); + // onInputChange is actually bound to loadOptions(...), + // loadOptions returns the promise, if there is one, so we can use this + // to chain the assertions onto + return output.props.onInputChange(text); + }; + + beforeEach(() => { + + renderer = TestUtils.createRenderer(); + loadOptions = sinon.stub(); + }); + + describe('using promises', () => { + + + beforeEach(() => { + + // Render an instance of the component + renderer.render(); + }); + + + it('does not call loadOptions', () => { + + expect(loadOptions, 'was not called'); + }); + + it('renders the select with no options', () => { + + return expect(renderer, 'to have rendered', + ); + }); + + + it('shows the returns options when the result returns', () => { + + // Unexpected comes with promises built in - we'll use them + // rather than depending on another library + loadOptions.returns(expect.promise((resolve, reject) => { + resolve({ options: [{ value: 1, label: 'test' }] }); + })); + + typeSearchText('te'); + + return loadOptions.firstCall.returnValue.then(() => { + + return expect(renderer, 'to have rendered', + ); + }); + }); + }); + + describe('with an isLoading prop', () => { + + beforeEach(() => { + + // Render an instance of the component + renderer.render(); + }); + + it('forces the isLoading prop on Select', () => { + return expect(renderer, 'to have rendered', + + ); + }); + }); +}); diff --git a/test/Select-test.js b/test/Select-test.js index d383903260..d3305a4d47 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1155,386 +1155,6 @@ describe('Select', () => { }); }); - describe('with async options', () => { - - // TODO: Need to use the new Select.Async control for this - return; - - var asyncOptions; - - beforeEach(() => { - - asyncOptions = sinon.stub(); - - asyncOptions.withArgs('te').callsArgWith(1, null, { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' }, - { value: 'tell', label: 'TELL three' } - ] - }); - - asyncOptions.withArgs('tes').callsArgWith(1, null, { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' } - ] - }); - - - }); - - describe('with autoload=true', () => { - - beforeEach(() => { - - // Render an instance of the component - wrapper = createControlWithWrapper({ - value: '', - asyncOptions: asyncOptions, - autoload: true - }); - }); - - - it('calls the asyncOptions initially with autoload=true', () => { - - expect(asyncOptions, 'was called with', ''); - }); - - it('calls the asyncOptions again when the input changes', () => { - - typeSearchText('ab'); - - expect(asyncOptions, 'was called twice'); - expect(asyncOptions, 'was called with', 'ab'); - }); - - it('shows the returned options after asyncOptions calls back', () => { - - typeSearchText('te'); - - var optionList = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-menu .Select-option'); - expect(optionList, 'to have length', 3); - expect(optionList[0], 'to have text', 'TEST one'); - expect(optionList[1], 'to have text', 'TEST two'); - expect(optionList[2], 'to have text', 'TELL three'); - }); - - it('uses the options cache when the same text is entered again', () => { - - - typeSearchText('te'); - typeSearchText('tes'); - - expect(asyncOptions, 'was called times', 3); - - typeSearchText('te'); - - expect(asyncOptions, 'was called times', 3); - - }); - - it('displays the correct options from the cache after the input is changed back to a previous value', () => { - - typeSearchText('te'); - typeSearchText('tes'); - typeSearchText('te'); - // Double check the options list is still correct - var optionList = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-menu .Select-option'); - expect(optionList, 'to have length', 3); - expect(optionList[0], 'to have text', 'TEST one'); - expect(optionList[1], 'to have text', 'TEST two'); - expect(optionList[2], 'to have text', 'TELL three'); - }); - - it('re-filters an existing options list if complete:true is provided', () => { - - asyncOptions.withArgs('te').callsArgWith(1, null, { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' }, - { value: 'tell', label: 'TELL three' } - ], - complete: true - }); - - typeSearchText('te'); - expect(asyncOptions, 'was called times', 2); - typeSearchText('tel'); - expect(asyncOptions, 'was called times', 2); - var optionList = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-menu .Select-option'); - expect(optionList, 'to have length', 1); - expect(optionList[0], 'to have text', 'TELL three'); - }); - - it('rethrows the error if err is set in the callback', () => { - - asyncOptions.withArgs('tes').callsArgWith(1, new Error('Something\'s wrong jim'), { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' } - ] - }); - - expect(() => { - typeSearchText('tes'); - }, 'to throw exception', new Error('Something\'s wrong jim')); - }); - - it('calls the asyncOptions function when the value prop changes', () => { - - expect(asyncOptions, 'was called once'); - - wrapper.setPropsForChild({ value: 'test2' }); - - expect(asyncOptions, 'was called twice'); - }); - - - }); - - describe('with autoload=false', () => { - - beforeEach(() => { - - // Render an instance of the component - instance = createControl({ - value: '', - asyncOptions: asyncOptions, - autoload: false - }); - }); - - it('does not initially call asyncOptions', () => { - - expect(asyncOptions, 'was not called'); - }); - - it('calls the asyncOptions on first key entry', () => { - - typeSearchText('a'); - expect(asyncOptions, 'was called with', 'a'); - }); - }); - - describe('with cacheAsyncResults=false', () => { - - beforeEach(() => { - - // Render an instance of the component - wrapper = createControlWithWrapper({ - value: '', - asyncOptions: asyncOptions, - cacheAsyncResults: false - }); - - // Focus on the input, such that mouse events are accepted - searchInputNode = ReactDOM.findDOMNode(instance.getInputNode()).querySelector('input'); - TestUtils.Simulate.focus(searchInputNode); - }); - - it('does not use cache when the same text is entered again', () => { - - typeSearchText('te'); - typeSearchText('tes'); - - expect(asyncOptions, 'was called times', 3); - - typeSearchText('te'); - - expect(asyncOptions, 'was called times', 4); - - }); - - it('updates the displayed value after changing value and refreshing from asyncOptions', () => { - - asyncOptions.reset(); - asyncOptions.callsArgWith(1, null, { - options: [ - { value: 'newValue', label: 'New Value from Server' }, - { value: 'test', label: 'TEST one' } - ] - }); - - wrapper.setPropsForChild({ value: 'newValue' }); - - expect(ReactDOM.findDOMNode(instance), 'queried for first', DISPLAYED_SELECTION_SELECTOR, - 'to have text', 'New Value from Server'); - }); - - - }); - }); - - describe('with async options (using promises)', () => { - - var asyncOptions, callCount, callInput; - - // TODO: Async Options need to use the Select.Async component - return; - - beforeEach(() => { - - asyncOptions = sinon.spy((input) => { - const options = [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' }, - { value: 'tell', label: 'TELL three' } - ].filter((elm) => { - return (elm.value.indexOf(input) !== -1 || elm.label.indexOf(input) !== -1); - }); - - return new Promise((resolve, reject) => { - input === '_FAIL'? reject('nope') : resolve({options: options}); - }); - }); - }); - - describe('[mocked promise]', () => { - - beforeEach(() => { - - // Render an instance of the component - wrapper = createControlWithWrapper({ - value: '', - asyncOptions: asyncOptions, - autoload: true - }); - }); - - it('should fulfill asyncOptions promise', () => { - return expect(instance.props.asyncOptions(''), 'to be fulfilled'); - }); - - it('should fulfill with 3 options when asyncOptions promise with input = "te"', () => { - return expect(instance.props.asyncOptions('te'), 'to be fulfilled with', { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' }, - { value: 'tell', label: 'TELL three' } - ] - }); - }); - - it('should fulfill with 2 options when asyncOptions promise with input = "tes"', () => { - return expect(instance.props.asyncOptions('tes'), 'to be fulfilled with', { - options: [ - { value: 'test', label: 'TEST one' }, - { value: 'test2', label: 'TEST two' } - ] - }); - }); - - it('should reject when asyncOptions promise with input = "_FAIL"', () => { - return expect(instance.props.asyncOptions('_FAIL'), 'to be rejected'); - }); - }); - - describe('with autoload=true', () => { - - beforeEach(() => { - - // Render an instance of the component - wrapper = createControlWithWrapper({ - value: '', - asyncOptions: asyncOptions, - autoload: true - }); - }); - - it('should be called once at the beginning', () => { - expect(asyncOptions, 'was called'); - }); - - it('calls the asyncOptions again when the input changes', () => { - - typeSearchText('ab'); - - expect(asyncOptions, 'was called twice'); - expect(asyncOptions, 'was called with', 'ab'); - }); - - it('shows the returned options after asyncOptions promise is resolved', (done) => { - - typeSearchText('te'); - - return asyncOptions.secondCall.returnValue.then(() => { - setTimeout(() => { - expect(ReactDOM.findDOMNode(instance), 'queried for', '.Select-option', - 'to satisfy', [ - expect.it('to have text', 'TEST one'), - expect.it('to have text', 'TEST two'), - expect.it('to have text', 'TELL three') - ]); - done(); - }); - }); - - }); - - it('doesn\'t update the returned options when asyncOptions is rejected', (done) => { - - typeSearchText('te'); - expect(asyncOptions, 'was called with', 'te'); - - asyncOptions.secondCall.returnValue.then(() => { - setTimeout(() => { - expect(ReactDOM.findDOMNode(instance), 'queried for', '.Select-option', - 'to satisfy', [ - expect.it('to have text', 'TEST one'), - expect.it('to have text', 'TEST two'), - expect.it('to have text', 'TELL three') - ]); - - // asyncOptions mock is set to reject the promise when invoked with '_FAIL' - typeSearchText('_FAIL'); - expect(asyncOptions, 'was called with', '_FAIL'); - - asyncOptions.thirdCall.returnValue.then(null, () => { - setTimeout(() => { - expect(ReactDOM.findDOMNode(instance), 'queried for', '.Select-option', - 'to satisfy', [ - expect.it('to have text', 'TEST one'), - expect.it('to have text', 'TEST two'), - expect.it('to have text', 'TELL three') - ]); - - done(); - }); - - }); - }); - - }); - }); - - }); - - describe('with autoload=false', () => { - - beforeEach(() => { - - // Render an instance of the component - instance = createControl({ - value: '', - asyncOptions: asyncOptions, - autoload: false - }); - }); - - it('does not initially call asyncOptions', () => { - expect(asyncOptions, 'was not called'); - }); - - it('calls the asyncOptions on first key entry', () => { - - typeSearchText('a'); - expect(asyncOptions, 'was called once'); - expect(asyncOptions, 'was called with', 'a'); - }); - }); - }); describe('with multi-select', () => { From 9aa600b078499a7a7072d53e25cb24e259f11d3a Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sat, 28 Nov 2015 12:04:04 +0100 Subject: [PATCH 2/6] Fix edge case bug with complete results and out-of-order responses Documented in the test case Changed the cache checking to start with the full search string, and then decrease the length. --- src/Async.js | 2 +- test/Async-test.js | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/Async.js b/src/Async.js index 78b645d5d5..a8ec0c7a9a 100644 --- a/src/Async.js +++ b/src/Async.js @@ -19,7 +19,7 @@ function updateCache (cache, input, data) { function getFromCache (cache, input) { if (!cache) return; - for (let i = 0; i <= input.length; i++) { + for (let i = input.length; i >= 0; --i) { let cacheKey = input.slice(0, i); if (cache[cacheKey] && (input === cacheKey || cache[cacheKey].complete)) { return cache[cacheKey]; diff --git a/test/Async-test.js b/test/Async-test.js index ba04f51ef5..c9c7554fbd 100644 --- a/test/Async-test.js +++ b/test/Async-test.js @@ -211,5 +211,52 @@ describe('Async', () => { ); + + }); + + }); }); }); From 5f6b7b6b1e92aa48b632488ba2cecd226387f771 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sat, 28 Nov 2015 13:11:34 +0100 Subject: [PATCH 3/6] Emulating DOM even for shallow renderer tests The DOM is still needed for the other tests, and `npm test` may run the async tests first. This means React is already required, and has already decided that there is no DOM, hence `TestUtils.renderIntoDocument` won't work. Adding the DOM emulation to Async-test fixes this problem. --- test/Async-test.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/Async-test.js b/test/Async-test.js index c9c7554fbd..a47a921208 100644 --- a/test/Async-test.js +++ b/test/Async-test.js @@ -1,3 +1,9 @@ +// Emulating the DOM here, only so that if this test file gets +// included first, then React thinks there's a DOM, so the other tests +// (e.g. Select-test.js) that do require a DOM work correctly + +var jsdomHelper = require('../testHelpers/jsdomHelper'); +jsdomHelper(); var unexpected = require('unexpected'); var unexpectedReact = require('unexpected-react'); var unexpectedSinon = require('unexpected-sinon'); @@ -13,9 +19,6 @@ var sinon = require('sinon'); var Select = require('../src/Select'); -if (!global.document) { - global.document = {}; -} describe('Async', () => { From 0a8e19a6d6daf3844dc724f4705e45aaebb07758 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sat, 28 Nov 2015 13:11:49 +0100 Subject: [PATCH 4/6] Add Async tests using callbacks --- test/Async-test.js | 106 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/test/Async-test.js b/test/Async-test.js index a47a921208..268344c033 100644 --- a/test/Async-test.js +++ b/test/Async-test.js @@ -262,4 +262,110 @@ describe('Async', () => { }); }); + + describe('using callbacks', () => { + + beforeEach(() => { + + // Render an instance of the component + renderer.render(); + }); + + it('shows the returns options when the result returns', () => { + + typeSearchText('te'); + + expect(loadOptions, 'was called'); + + const callback = loadOptions.args[0][1]; + + callback(null, { options: [ { value: 1, label: 'test callback' } ] }); + + + return expect(renderer, 'to have rendered', + ); + }); + + it('ignores callbacks from earlier requests (out-of-order responses)', () => { + + typeSearchText('te'); + typeSearchText('tes'); + + expect(loadOptions, 'was called times', 2); + + const callback1 = loadOptions.args[0][1]; + const callback2 = loadOptions.args[1][1]; + + callback2(null, { options: [ { value: 2, label: 'test callback 2' } ] }); + // Callback1 should be ignored + callback1(null, { options: [ { value: 1, label: 'test callback' } ] }); + + return expect(renderer, 'to have rendered', + ); + }); + + it('throws an error when the callback returns an error', () => { + + typeSearchText('te'); + + expect(() => loadOptions.args[0][1](new Error('Something went wrong')), + 'to throw', 'Something went wrong'); + }); + + it('assumes no options when the result has no `options` property', () => { + + typeSearchText('te'); + loadOptions.args[0][1](null, [ { value: 1, label: 'should be wrapped in an object' } ] ); + + return expect(renderer, 'to have rendered', ); }); }); + + describe('with minimumInput', () => { + + describe('=0', () => { + + beforeEach(() => { + renderer.render(); + }); + + it('calls loadOptions immediately', () => { + // componentDidMount is not currently called with the shallowRenderer, so we need to fake it here + // When we upgrade to 0.15, we'll be able to use renderer.getMountedInstance() + // (or, componentDidMount will be called with the shallow renderer) + renderer._instance._instance.componentDidMount(); + expect(loadOptions, 'was called'); + }); + + it('calls loadOptions again when input returns to empty', () => { + + renderer._instance._instance.componentDidMount(); + typeSearchText('t'); + typeSearchText(''); + expect(loadOptions, 'was called times', 3); + }); + }); + + describe('=3', () => { + + beforeEach(() => { + renderer.render(); + }); + + it('does not call loadOptions initially', () => { + + renderer._instance._instance.componentDidMount(); + expect(loadOptions, 'was not called'); + }); + + it('does not call loadOptions when 2 characters are entered', () => { + + typeSearchText('te'); + expect(loadOptions, 'was not called'); + }); + + it('calls loadOptions when 3 characters are entered', () => { + + typeSearchText('tes'); + expect(loadOptions, 'was called'); + }); + + it('resets to `type to search` when text returns to < 3', () => { + + typeSearchText('tes'); + loadOptions.args[0][1](null, { options: [ { value: 1, label: 'test1' } ] }); + + typeSearchText('te'); + return expect(renderer, 'to have rendered', + ); + }); + }); }); describe('with an isLoading prop', () => {