Skip to content

Commit

Permalink
Merge pull request #366 from wheresrhys/smells
Browse files Browse the repository at this point in the history 
Smells
  • Loading branch information
@wheresrhys
wheresrhys committed Sep 23, 2018
2 parents 9c02db3 + 7b7e6da commit 1ca5078
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 100 deletions.
1 change: 1 addition & 0 deletions src/lib/compile-route.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const getParamsMatcher = ({ params: expectedParams, matcher }) => {

const getUrlMatcher = route => {
const { matcher, query } = route;

if (typeof matcher === 'function') {
return matcher;
}
Expand Down
28 changes: 16 additions & 12 deletions src/lib/fetch-handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ const normalizeRequest = (url, options, Request) => {
}
};

const resolve = async (response, url, opts) => {
while (
typeof response === 'function' ||
typeof response.then === 'function'
) {
if (typeof response === 'function') {
response = response(url, opts);
} else {
// Strange .then is to cope with non ES Promises... god knows why it works
response = await response.then(it => it);
}
}
return response;
};

FetchMock.fetchHandler = function(url, opts, request) {
({ url, opts, request } = normalizeRequest(url, opts, this.config.Request));

Expand Down Expand Up @@ -96,18 +111,7 @@ FetchMock.generateResponse = async function(route, url, opts) {
// Because of this we can't safely check for function before Promisey-ness,
// or vice versa. So to keep it DRY, and flexible, we keep trying until we
// have something that looks like neither Promise nor function
let response = route.response;
while (
typeof response === 'function' ||
typeof response.then === 'function'
) {
if (typeof response === 'function') {
response = response(url, opts);
} else {
// Strange .then is to cope with non ES Promises... god knows why it works
response = await response.then(it => it);
}
}
const response = await resolve(route.response, url, opts);

// If the response says to throw an error, throw it
// Type checking is to deal with sinon spies having a throws property :-0
Expand Down
34 changes: 9 additions & 25 deletions test/specs/inspecting.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,31 +66,15 @@ module.exports = fetchMock => {
afterEach(() => {
fm.filterCalls.restore();
});
it('called() uses the internal filtering method', () => {
fm.called('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
it('calls() uses the internal filtering method', () => {
fm.calls('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
it('lastCall() uses the internal filtering method', () => {
fm.lastCall('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
it('lastUrl() uses the internal filtering method', () => {
fm.lastUrl('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
it('lastOptions() uses the internal filtering method', () => {
fm.lastOptions('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
['called', 'calls', 'lastCall', 'lastUrl', 'lastOptions'].forEach(
method => {
it(`${method}() uses the internal filtering method`, () => {
fm[method]('name', { an: 'option' });
expect(fm.filterCalls.calledWith('name', { an: 'option' })).to.be
.true;
});
}
);
});
});

Expand Down
100 changes: 37 additions & 63 deletions test/specs/set-up-and-tear-down.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,19 @@ chai.use(require('sinon-chai'));
const expect = chai.expect;
const sinon = require('sinon');

const testChainableMethod = (getFetchMock, method, args = []) => {
it('is chainable', () => {
expect(getFetchMock()[method](...args)).to.equal(getFetchMock());
});

it("has 'this'", () => {
sinon.spy(getFetchMock(), method);
getFetchMock()[method](...args);
expect(getFetchMock()[method].lastCall.thisValue).to.equal(getFetchMock());
getFetchMock()[method].restore();
});
};

module.exports = fetchMock => {
describe('Set up and tear down', () => {
let fm;
Expand All @@ -13,16 +26,7 @@ module.exports = fetchMock => {
afterEach(() => fm.restore());

describe('mock', () => {
it('is chainable', () => {
expect(fm.mock(/a/, 200)).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'mock');
fm.mock(/a/, 200);
expect(fm.mock.lastCall.thisValue).to.equal(fm);
fm.mock.restore();
});
testChainableMethod(() => fm, 'mock', [/a/, 200]);

it('can be called multiple times', () => {
expect(() => {
Expand Down Expand Up @@ -97,6 +101,8 @@ module.exports = fetchMock => {

describe('method shorthands', () => {
'get,post,put,delete,head,patch'.split(',').forEach(method => {
testChainableMethod(() => fm, method, [/a/, 200]);

it(`has shorthand for ${method.toUpperCase()}`, () => {
sinon.stub(fm, 'mock');
fm[method]('a', 'b');
Expand All @@ -112,25 +118,29 @@ module.exports = fetchMock => {
fm.restore();
});

it(`shorthand for ${method.toUpperCase()} is chainable`, () => {
const result = fm[method]('a', 'b');
expect(result).to.equal(fm);
testChainableMethod(() => fm, `${method}Once`, [/a/, 200]);

it(`has shorthand for ${method.toUpperCase()} called once`, () => {
sinon.stub(fm, 'mock');
fm[`${method}Once`]('a', 'b');
fm[`${method}Once`]('a', 'b', { opt: 'c' });
expect(fm.mock).calledWith('a', 'b', {
method: method.toUpperCase(),
repeat: 1
});
expect(fm.mock).calledWith('a', 'b', {
opt: 'c',
method: method.toUpperCase(),
repeat: 1
});
fm.mock.restore();
fm.restore();
});
});
});

describe('reset', () => {
it('is chainable', () => {
expect(fm.restore()).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'restore');
fm.restore();
expect(fm.restore.lastCall.thisValue).to.equal(fm);
fm.restore.restore();
});
testChainableMethod(() => fm, 'reset');

it('can be called even if no mocks set', () => {
expect(() => fm.restore()).not.to.throw();
Expand Down Expand Up @@ -161,16 +171,7 @@ module.exports = fetchMock => {
});

describe('resetBehavior', () => {
it('is chainable', () => {
expect(fm.resetBehavior()).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'resetBehavior');
fm.resetBehavior();
expect(fm.resetBehavior.lastCall.thisValue).to.equal(fm);
fm.resetBehavior.restore();
});
testChainableMethod(() => fm, 'resetBehavior');

it('can be called even if no mocks set', () => {
expect(() => fm.resetBehavior()).not.to.throw();
Expand All @@ -190,16 +191,7 @@ module.exports = fetchMock => {
});

describe('resetHistory', () => {
it('is chainable', () => {
expect(fm.resetHistory()).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'resetHistory');
fm.resetHistory();
expect(fm.resetHistory.lastCall.thisValue).to.equal(fm);
fm.resetHistory.restore();
});
testChainableMethod(() => fm, 'resetHistory');

it('can be called even if no mocks set', () => {
expect(() => fm.resetHistory()).not.to.throw();
Expand All @@ -222,16 +214,7 @@ module.exports = fetchMock => {
});

describe('spy', () => {
it('is chainable', () => {
expect(fm.spy()).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'spy');
fm.spy();
expect(fm.spy.lastCall.thisValue).to.equal(fm);
fm.spy.restore();
});
testChainableMethod(() => fm, 'spy');

it('calls catch()', () => {
sinon.spy(fm, 'catch');
Expand All @@ -242,16 +225,7 @@ module.exports = fetchMock => {
});

describe('catch', () => {
it('is chainable', () => {
expect(fm.catch(200)).to.equal(fm);
});

it("has 'this'", () => {
sinon.spy(fm, 'catch');
fm.catch(200);
expect(fm.catch.lastCall.thisValue).to.equal(fm);
fm.catch.restore();
});
testChainableMethod(() => fm, 'catch');
});
});
};

0 comments on commit 1ca5078

Please sign in to comment.