From 67ad410333441e43b500b2891a5a10cbce637325 Mon Sep 17 00:00:00 2001 From: Phil Eaton Date: Tue, 31 May 2016 16:06:47 -0400 Subject: [PATCH 1/4] Add fetch tests --- src/fetch.js | 25 +++++++++++++----- test/actions/api/linodes.spec.js | 8 +++--- test/api-store.spec.js | 10 ++++---- test/contexts.js | 40 +++++++++++++++++++++++++++++ test/linodes/actions/create.spec.js | 16 ++++++------ test/mocks.js | 15 ----------- 6 files changed, 76 insertions(+), 38 deletions(-) create mode 100644 test/contexts.js delete mode 100644 test/mocks.js diff --git a/src/fetch.js b/src/fetch.js index 5a88c7ced9b..1ecc35955fb 100644 --- a/src/fetch.js +++ b/src/fetch.js @@ -1,18 +1,31 @@ import { API_ROOT } from './constants'; -import _fetch from 'isomorphic-fetch'; +import * as isomorphicFetch from 'isomorphic-fetch'; -export function fetch(token, _input, _init) { - const init = { +/* + * Sinon cannot stub out a function in a function-only module. + * https://github.com/sinonjs/sinon/issues/664 + */ +export function _fetch(...args) { + return isomorphicFetch['default'](...args); +} + +export function fetch(token, _path, _options) { + /* + * Get updated reference in case _fetch is a stub (during testing). + * See comment on _fetch. + */ + const fetchRef = module.exports._fetch; + const options = { mode: 'cors', - ..._init, + ..._options, headers: { Accept: 'application/json', Authorization: `token ${token}`, 'Content-Type': 'application/json', }, }; - const input = API_ROOT + _input; - return _fetch(input, init); + const path = API_ROOT + _path; + return fetchRef(path, options); } export { _fetch }; diff --git a/test/actions/api/linodes.spec.js b/test/actions/api/linodes.spec.js index 0310d04b397..0dbbaa0fbfa 100644 --- a/test/actions/api/linodes.spec.js +++ b/test/actions/api/linodes.spec.js @@ -6,7 +6,7 @@ import { rebootLinode, UPDATE_LINODE, } from '../../../src/actions/api/linodes'; -import { mockContext } from '../../mocks'; +import { mockFetchContext } from '../../contexts'; describe('actions/linodes/power', sinon.test(() => { const mockBootingResponse = { @@ -25,7 +25,7 @@ describe('actions/linodes/power', sinon.test(() => { }); it('returns linode power boot status', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = powerOnLinode('foo'); @@ -47,7 +47,7 @@ describe('actions/linodes/power', sinon.test(() => { }; it('returns linode power shutdown status', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = powerOffLinode('foo'); @@ -69,7 +69,7 @@ describe('actions/linodes/power', sinon.test(() => { }; it('returns linode power reboot status', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = rebootLinode('foo'); diff --git a/test/api-store.spec.js b/test/api-store.spec.js index 1be87638a48..d1841e2da74 100644 --- a/test/api-store.spec.js +++ b/test/api-store.spec.js @@ -6,7 +6,7 @@ import makeApiList, { makeUpdateItem, makeDeleteItem, } from '../src/api-store'; -import { mockContext } from './mocks'; +import { mockFetchContext } from './contexts'; const mockFoobarsResponse = { foobars: [ @@ -209,7 +209,7 @@ describe('api-store', () => { }); it('fetches a page of items from the API', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = makeFetchPage('FETCH_FOOBARS', 'foobars'); @@ -227,7 +227,7 @@ describe('api-store', () => { }); it('fetches the requested page', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = makeFetchPage('FETCH_FOOBARS', 'foobars'); @@ -249,7 +249,7 @@ describe('api-store', () => { }); it('fetches an item from the API', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = makeUpdateItem('UPDATE_FOOBAR', 'foobars', 'foobar'); @@ -276,7 +276,7 @@ describe('api-store', () => { const emptyResponse = {}; it('performs the API request', async () => { - await mockContext(sandbox, async ({ + await mockFetchContext(sandbox, async ({ auth, dispatch, getState, fetchStub, }) => { const f = makeDeleteItem('DELETE_FOOBAR', 'foobars'); diff --git a/test/contexts.js b/test/contexts.js new file mode 100644 index 00000000000..3443f029bc3 --- /dev/null +++ b/test/contexts.js @@ -0,0 +1,40 @@ +import sinon from 'sinon'; +import * as fetch from '~/fetch'; + +export const mockContext = async (sandbox, stubInfo, f) => { + const stubs = stubInfo.map(({ obj, accessor, rsp }) => + sinon.stub(obj, accessor).returns(rsp)); + + await f(stubs); + + stubs.forEach(stub => { + stub.restore(); + }); +}; + +export const mockFetchContext = async (sandbox, f, rsp, state = {}) => { + const auth = { token: 'token' }; + const getState = sinon.stub().returns({ + authentication: auth, + ...state, + }); + const dispatch = sinon.spy(); + const stubInfo = [ + { obj: fetch, accessor: 'fetch', rsp: { json: () => rsp } }, + ]; + + const fCurried = async ([fetchStub]) => { + await f({ auth, getState, dispatch, fetchStub }); + }; + return mockContext(sandbox, stubInfo, fCurried, state); +}; + +export const mockInternalFetchContext = async (sandbox, f) => { + const stubInfo = [ + { obj: fetch, accessor: '_fetch', rsp: null }, + ]; + const fCurried = async ([fetchStub]) => { + await f({ fetchStub }); + }; + return mockContext(sandbox, stubInfo, fCurried); +}; diff --git a/test/linodes/actions/create.spec.js b/test/linodes/actions/create.spec.js index 92a5893292c..6c17ebec02d 100644 --- a/test/linodes/actions/create.spec.js +++ b/test/linodes/actions/create.spec.js @@ -2,7 +2,7 @@ import { expect } from 'chai'; import sinon from 'sinon'; import * as actions from '~/linodes/actions/create'; import * as linodeActions from '~/actions/api/linodes'; -import { mockContext } from '~/../test/mocks'; +import { mockFetchContext } from '~/../test/contexts'; import { pushPath } from 'redux-simple-router'; describe('linodes/actions/create', () => { @@ -112,7 +112,7 @@ describe('linodes/actions/create', () => { }); it('should call getState() once', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(getState.calledOnce).to.equal(true); @@ -120,7 +120,7 @@ describe('linodes/actions/create', () => { }); it('should dispatch a TOGGLE_CREATING action', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(dispatch.calledWith({ @@ -130,7 +130,7 @@ describe('linodes/actions/create', () => { }); it('should perform an HTTP POST to /linodes', async () => { - await mockContext(sandbox, async ({ dispatch, getState, fetchStub }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState, fetchStub }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(fetchStub.calledWith( @@ -149,7 +149,7 @@ describe('linodes/actions/create', () => { }); it('should dispatch an UPDATE_LINODE action with the new linode', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(dispatch.calledWith({ @@ -160,7 +160,7 @@ describe('linodes/actions/create', () => { }); it('should dispatch a routing action to navigate to the detail page', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(dispatch.calledWith( @@ -170,7 +170,7 @@ describe('linodes/actions/create', () => { }); it('should dispatch a CLEAR_FORM action', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); await func(dispatch, getState); expect(dispatch.calledWith({ type: actions.CLEAR_FORM })).to.equal(true); @@ -178,7 +178,7 @@ describe('linodes/actions/create', () => { }); it('should update the linode until it finishes provisioning', async () => { - await mockContext(sandbox, async ({ dispatch, getState }) => { + await mockFetchContext(sandbox, async ({ dispatch, getState }) => { const func = actions.createLinode(); const update = sandbox.spy(() => { }); sandbox.stub(linodeActions, 'updateLinodeUntil', update); diff --git a/test/mocks.js b/test/mocks.js deleted file mode 100644 index 885037e4cdd..00000000000 --- a/test/mocks.js +++ /dev/null @@ -1,15 +0,0 @@ -import * as fetch from '~/fetch'; - -export const mockContext = async (sandbox, f, rsp, state = {}) => { - const auth = { token: 'token' }; - const getState = sandbox.stub().returns({ - authentication: auth, - ...state, - }); - const dispatch = sandbox.spy(); - const fetchStub = sandbox.stub(fetch, 'fetch').returns({ - json: () => rsp, - }); - await f({ auth, getState, dispatch, fetchStub }); - fetchStub.restore(); -}; From 75870baf930a301c36e06079f8c0d859f6ec5b1f Mon Sep 17 00:00:00 2001 From: Phil Eaton Date: Tue, 31 May 2016 16:16:53 -0400 Subject: [PATCH 2/4] Add fetch spec --- test/fetch.spec.js | 64 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 test/fetch.spec.js diff --git a/test/fetch.spec.js b/test/fetch.spec.js new file mode 100644 index 00000000000..acb45f6843b --- /dev/null +++ b/test/fetch.spec.js @@ -0,0 +1,64 @@ +import sinon from 'sinon'; +import { expect } from 'chai'; +import { mockInternalFetchContext } from './contexts'; +import { fetch } from '~/fetch'; +import { API_ROOT } from '~/constants'; + +describe('fetch', () => { + let sandbox = null; + + beforeEach(() => { + sandbox = sinon.sandbox.create(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + const token = 'my token'; + const defaultHeaders = { + mode: 'cors', + headers: { + Accept: 'application/json', + Authorization: `token ${token}`, + 'Content-Type': 'application/json', + }, + }; + + it('should default to cors mode and headers for token', async () => { + await mockInternalFetchContext(sandbox, async ({ fetchStub }) => { + await fetch(token, ''); + + expect(fetchStub.calledWith( + API_ROOT, + defaultHeaders, + )).to.equal(true); + }); + }); + + it('should fetch the correct path', async () => { + await mockInternalFetchContext(sandbox, async ({ fetchStub }) => { + await fetch(token, 'path'); + + expect(fetchStub.calledWith( + `${API_ROOT}path`, + defaultHeaders, + )); + }); + }); + + it('should include data', async () => { + await mockInternalFetchContext(sandbox, async ({ fetchStub }) => { + const data = { data: { foo: 'bar' } }; + await fetch(token, '', data); + + expect(fetchStub.calledWith( + API_ROOT, + { + ...defaultHeaders, + ...data, + } + )); + }); + }); +}); From 42487bdea62a9489c305ccace17bfc153ee6ecd6 Mon Sep 17 00:00:00 2001 From: Phil Eaton Date: Tue, 31 May 2016 16:19:38 -0400 Subject: [PATCH 3/4] Remove extra _fetch export --- src/fetch.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/fetch.js b/src/fetch.js index 1ecc35955fb..daebaf9fb14 100644 --- a/src/fetch.js +++ b/src/fetch.js @@ -27,5 +27,3 @@ export function fetch(token, _path, _options) { const path = API_ROOT + _path; return fetchRef(path, options); } - -export { _fetch }; From e39bd3f67d0729eef1bdf22fd9b374878432f79b Mon Sep 17 00:00:00 2001 From: Phil Eaton Date: Tue, 31 May 2016 16:24:48 -0400 Subject: [PATCH 4/4] Rename _fetch to be more useful --- src/fetch.js | 8 ++++---- src/layouts/OAuth.js | 4 ++-- test/contexts.js | 2 +- test/layouts/OAuth.spec.js | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/fetch.js b/src/fetch.js index daebaf9fb14..50406a33bcf 100644 --- a/src/fetch.js +++ b/src/fetch.js @@ -5,16 +5,16 @@ import * as isomorphicFetch from 'isomorphic-fetch'; * Sinon cannot stub out a function in a function-only module. * https://github.com/sinonjs/sinon/issues/664 */ -export function _fetch(...args) { +export function rawFetch(...args) { return isomorphicFetch['default'](...args); } export function fetch(token, _path, _options) { /* - * Get updated reference in case _fetch is a stub (during testing). - * See comment on _fetch. + * Get updated reference in case rawFetch is a stub (during testing). + * See comment on rawFetch. */ - const fetchRef = module.exports._fetch; + const fetchRef = module.exports.rawFetch; const options = { mode: 'cors', ..._options, diff --git a/src/layouts/OAuth.js b/src/layouts/OAuth.js index d6c1c3f2778..c1cc0f6dbeb 100644 --- a/src/layouts/OAuth.js +++ b/src/layouts/OAuth.js @@ -4,7 +4,7 @@ import { setToken } from '../actions/authentication'; import { clientId, clientSecret } from '../secrets'; import { pushPath } from 'redux-simple-router'; import { LOGIN_ROOT } from '../constants'; -import { _fetch as fetch } from '../fetch'; +import { rawFetch } from '../fetch'; export class OAuthCallbackPage extends Component { async componentDidMount() { @@ -18,7 +18,7 @@ export class OAuthCallbackPage extends Component { data.append('client_secret', clientSecret); data.append('code', code); - const resp = await fetch(`${LOGIN_ROOT}/oauth/token`, { + const resp = await rawFetch(`${LOGIN_ROOT}/oauth/token`, { method: 'POST', body: data, }); diff --git a/test/contexts.js b/test/contexts.js index 3443f029bc3..368e25b16ab 100644 --- a/test/contexts.js +++ b/test/contexts.js @@ -31,7 +31,7 @@ export const mockFetchContext = async (sandbox, f, rsp, state = {}) => { export const mockInternalFetchContext = async (sandbox, f) => { const stubInfo = [ - { obj: fetch, accessor: '_fetch', rsp: null }, + { obj: fetch, accessor: 'rawFetch', rsp: null }, ]; const fCurried = async ([fetchStub]) => { await f({ fetchStub }); diff --git a/test/layouts/OAuth.spec.js b/test/layouts/OAuth.spec.js index aa8e2c18715..16caf1b7c40 100644 --- a/test/layouts/OAuth.spec.js +++ b/test/layouts/OAuth.spec.js @@ -43,7 +43,7 @@ describe('layouts/OAuth', () => { }; it('exchanges the code for an OAuth token', async () => { - const fetchStub = sandbox.stub(fetch, '_fetch').returns({ + const fetchStub = sandbox.stub(fetch, 'rawFetch').returns({ json: () => exchangeResponse, }); const dispatch = sandbox.spy(); @@ -65,7 +65,7 @@ describe('layouts/OAuth', () => { }); it('dispatches a setToken action', async () => { - sandbox.stub(fetch, '_fetch').returns({ + sandbox.stub(fetch, 'rawFetch').returns({ json: () => exchangeResponse, }); const dispatch = sandbox.spy(); @@ -88,7 +88,7 @@ describe('layouts/OAuth', () => { }); it('supports the return query string option', async () => { - sandbox.stub(fetch, '_fetch').returns({ + sandbox.stub(fetch, 'rawFetch').returns({ json: () => exchangeResponse, }); const dispatch = sandbox.spy();