From 19fa003ef867278c654215db68abf105c43f89f6 Mon Sep 17 00:00:00 2001 From: Gregor Date: Tue, 30 Oct 2018 18:57:16 -0700 Subject: [PATCH 1/2] refactor: remove private .fetch API --- index.js | 18 +----------------- lib/fetch.js | 2 ++ lib/request.js | 7 +++++-- lib/with-defaults.js | 11 +++++++++++ 4 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 lib/fetch.js create mode 100644 lib/with-defaults.js diff --git a/index.js b/index.js index d3a3a8175..5f3f12bfc 100644 --- a/index.js +++ b/index.js @@ -1,28 +1,12 @@ const endpoint = require('@octokit/endpoint') -const fetch = require('node-fetch').default const getUserAgent = require('universal-user-agent') -const request = require('./lib/request') const version = require('./package.json').version const userAgent = `octokit-request.js/${version} ${getUserAgent()}` - -function octokitRequest (endpoint, route, options) { - return request(module.exports.fetch, endpoint(route, options)) -} - -function withDefaults (oldEndpoint, newDefaults) { - const endpoint = oldEndpoint.defaults(newDefaults) - const request = octokitRequest.bind(null, endpoint) - request.endpoint = endpoint - request.defaults = withDefaults.bind(null, endpoint) - return request -} +const withDefaults = require('./lib/with-defaults') module.exports = withDefaults(endpoint, { headers: { 'user-agent': userAgent } }) - -// expose internally used `fetch` method for testing/mocking only -module.exports.fetch = fetch diff --git a/lib/fetch.js b/lib/fetch.js new file mode 100644 index 000000000..42b12d41e --- /dev/null +++ b/lib/fetch.js @@ -0,0 +1,2 @@ +// expose internally used `fetch` method for testing/mocking only +module.exports.fetch = require('node-fetch').default diff --git a/lib/request.js b/lib/request.js index 890813ce6..b5af51670 100644 --- a/lib/request.js +++ b/lib/request.js @@ -4,10 +4,13 @@ module.exports = request const isPlainObject = require('is-plain-object') +const mockable = require('./fetch') const getBuffer = require('./get-buffer-response') const HttpError = require('./http-error') -function request (fetch, requestOptions) { +function request (endpoint, route, options) { + const requestOptions = endpoint(route, options) + if (isPlainObject(requestOptions.body) || Array.isArray(requestOptions.body)) { requestOptions.body = JSON.stringify(requestOptions.body) } @@ -15,7 +18,7 @@ function request (fetch, requestOptions) { let headers = {} let status - return fetch(requestOptions.url, Object.assign({ + return mockable.fetch(requestOptions.url, Object.assign({ method: requestOptions.method, body: requestOptions.body, headers: requestOptions.headers, diff --git a/lib/with-defaults.js b/lib/with-defaults.js new file mode 100644 index 000000000..3934def7b --- /dev/null +++ b/lib/with-defaults.js @@ -0,0 +1,11 @@ +module.exports = withDefaults + +const request = require('./request') + +function withDefaults (oldEndpoint, newDefaults) { + const endpoint = oldEndpoint.defaults(newDefaults) + const newApi = request.bind(null, endpoint) + newApi.endpoint = endpoint + newApi.defaults = withDefaults.bind(null, endpoint) + return newApi +} From 5f58e15d3f76018a9386c6605b58d784d94d5e88 Mon Sep 17 00:00:00 2001 From: Gregor Date: Tue, 30 Oct 2018 18:59:06 -0700 Subject: [PATCH 2/2] test: adapt tests for removed .fetch API --- test/defaults-test.js | 5 +++-- test/request-test.js | 31 ++++++++++++++++--------------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/test/defaults-test.js b/test/defaults-test.js index 4ba639328..770b25028 100644 --- a/test/defaults-test.js +++ b/test/defaults-test.js @@ -3,6 +3,7 @@ const fetchMock = require('fetch-mock/es5/server') const sinonChai = require('sinon-chai') const octokitRequest = require('..') +const mockable = require('../lib/fetch') const expect = chai.expect chai.use(sinonChai) @@ -13,7 +14,7 @@ describe('endpoint.defaults()', () => { }) it('README example', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://github-enterprise.acme-inc.com/api/v3/orgs/my-project/repos?per_page=100', [], { headers: { accept: 'application/vnd.github.v3+json', @@ -40,7 +41,7 @@ describe('endpoint.defaults()', () => { }) it('repeated defaults', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get('https://github-enterprise.acme-inc.com/api/v3/orgs/my-project/repos', [], { headers: { accept: 'application/vnd.github.v3+json', diff --git a/test/request-test.js b/test/request-test.js index 094afb36c..c960d6aa9 100644 --- a/test/request-test.js +++ b/test/request-test.js @@ -4,24 +4,25 @@ const fetchMock = require('fetch-mock/es5/server') const sinonChai = require('sinon-chai') const octokitRequest = require('..') +const mockable = require('../lib/fetch') chai.use(sinonChai) const expect = chai.expect -const originalFetch = octokitRequest.fetch +const originalFetch = mockable.fetch const pkg = require('../package.json') const userAgent = `octokit-request.js/${pkg.version} ${getUserAgent()}` describe('octokitRequest()', () => { afterEach(() => { - octokitRequest.fetch = originalFetch + mockable.fetch = originalFetch }) it('is a function', () => { expect(octokitRequest).to.be.a('function') }) it('README example', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://api.github.com/orgs/octokit/repos?type=private', [], { headers: { accept: 'application/vnd.github.v3+json', @@ -44,7 +45,7 @@ describe('octokitRequest()', () => { }) it('README example alternative', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://api.github.com/orgs/octokit/repos?type=private', [], { headers: { accept: 'application/vnd.github.v3+json', @@ -53,7 +54,7 @@ describe('octokitRequest()', () => { } }) - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://api.github.com/orgs/octokit/repos?type=private', []) return octokitRequest({ @@ -72,7 +73,7 @@ describe('octokitRequest()', () => { }) it('Request with body', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://api.github.com/repos/octocat/hello-world/issues', 201, { headers: { 'content-type': 'application/json; charset=utf-8' @@ -102,7 +103,7 @@ describe('octokitRequest()', () => { }) it('Put without request body', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .mock('https://api.github.com/user/starred/octocat/hello-world', 204, { headers: { 'content-length': 0 @@ -123,7 +124,7 @@ describe('octokitRequest()', () => { }) it('HEAD requests (octokit/rest.js#841)', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .head('https://api.github.com/repos/whatwg/html/pulls/1', { status: 200, headers: { @@ -163,7 +164,7 @@ describe('octokitRequest()', () => { }) it.skip('Binary response with redirect (🤔 unclear how to mock fetch redirect properly)', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get('https://codeload.github.com/octokit-fixture-org/get-archive/legacy.tar.gz/master', { status: 200, body: Buffer.from('1f8b0800000000000003cb4f2ec9cfce2cd14dcbac28292d4ad5cd2f4ad74d4f2dd14d2c4acec82c4bd53580007d060a0050bfb9b9a90203c428741ac2313436343307222320dbc010a8dc5c81c194124b8905a5c525894540a714e5e797e05347481edd734304e41319ff41ae8e2ebeae7ab92964d801d46f66668227fe0d4d51e3dfc8d0c8d808284f75df6201233cfe951590627ba01d330a46c1281805a3806e000024cb59d6000a0000', 'hex'), @@ -188,7 +189,7 @@ describe('octokitRequest()', () => { // TODO: fails with "response.buffer is not a function" in browser if (!process.browser) { it('Binary response', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get('https://codeload.github.com/octokit-fixture-org/get-archive/legacy.tar.gz/master', { status: 200, @@ -206,7 +207,7 @@ describe('octokitRequest()', () => { } it('304 etag', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get((url, { headers }) => { return url === 'https://api.github.com/orgs/myorg' && headers['if-none-match'] === 'etag' @@ -227,7 +228,7 @@ describe('octokitRequest()', () => { }) it('Not found', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get('path:/orgs/nope', 404) return octokitRequest('GET /orgs/:org', { @@ -244,7 +245,7 @@ describe('octokitRequest()', () => { }) it('non-JSON response', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get('path:/repos/octokit-fixture-org/hello-world/contents/README.md', { status: 200, body: '# hello-world', @@ -284,7 +285,7 @@ describe('octokitRequest()', () => { } it('custom user-agent', () => { - octokitRequest.fetch = fetchMock.sandbox() + mockable.fetch = fetchMock.sandbox() .get((url, { headers }) => headers['user-agent'] === 'funky boom boom pow', 200) return octokitRequest('GET /', { @@ -295,7 +296,7 @@ describe('octokitRequest()', () => { }) it('passes node-fetch options to fetch only', () => { - octokitRequest.fetch = (url, options) => { + mockable.fetch = (url, options) => { expect(url).to.equal('https://api.github.com/') expect(options.timeout).to.equal(100) return Promise.reject(new Error('ok'))