From c65074ac93d8f2edcf8a46e9e73fce0a0b378317 Mon Sep 17 00:00:00 2001 From: Ramesh Sunkara Date: Wed, 28 Jun 2017 16:47:47 -0400 Subject: [PATCH 1/5] testing recordOnlySuccess --- index.js | 17 ++++++++++++++++- test/yakbak.js | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 6a1fdea..7c52d7f 100644 --- a/index.js +++ b/index.js @@ -18,6 +18,7 @@ var debug = require('debug')('yakbak:server'); * @param {Object} opts * @param {String} opts.dirname The tapes directory * @param {Boolean} opts.noRecord if true, requests will return a 404 error if the tape doesn't exist + * @param {Boolean} opts.recordOnlySuccess if true, requests which return only 2XX will be recorded. * @returns {Function} */ @@ -31,6 +32,7 @@ module.exports = function (host, opts) { return buffer(req).then(function (body) { var file = path.join(opts.dirname, tapename(req, body)); + var successfulResponse = /^[2][0|2][0-8]$/; return Promise.try(function () { return require.resolve(file); @@ -40,7 +42,20 @@ module.exports = function (host, opts) { throw new RecordingDisabledError('Recording Disabled'); } else { return proxy(req, body, host).then(function (pres) { - return record(pres.req, pres, file); + console.log('Am i here', opts); // eslint-disable-line + if (opts.recordOnlySuccess === true) { + console.log('Am i here-1', opts, pres.statusCode); // eslint-disable-line + if (successfulResponse.test(pres.statusCode)) { + console.log('Am i here-2', opts); // eslint-disable-line + return record(pres.req, pres, file); + } else { + console.log('Am i here-3', opts); // eslint-disable-line + throw new RecordingDisabledError('Only Successful responses will be recorded'); + } + } else { + console.log('Am i here-4', opts); // eslint-disable-line + return record(pres.req, pres, file); + } }); } diff --git a/test/yakbak.js b/test/yakbak.js index c16bd81..d0e6568 100644 --- a/test/yakbak.js +++ b/test/yakbak.js @@ -12,6 +12,7 @@ var fs = require('fs'); var crypto = require('crypto'); var url = require('url'); +/* eslint-dsiable*/ describe('yakbak', function () { var server, tmpdir, yakbak; @@ -24,10 +25,12 @@ describe('yakbak', function () { }); beforeEach(function (done) { + console.log('Create dir'); tmpdir = createTmpdir(done); }); afterEach(function (done) { + console.log('director deletio dir'); tmpdir.teardown(done); }); @@ -135,6 +138,24 @@ describe('yakbak', function () { }); }); }); + + describe.only("when onlySuccessResponse is enabled", function () { + beforeEach(function () { + yakbak = subject(server.host, { dirname: tmpdir.dirname, recordOnlySuccess: true }); + }); + + it('does not write the tape to disk', function (done) { + request(yakbak) + .get('/blahblah/2') + .set('host', 'localhost:4001') + .expect(404) + .end(function (err) { + console.log('I am in end error', tmpdir.dirname, err); // eslint-disable-line + // assert(!fs.existsSync(tmpdir.join('3234ee470c8605a1837e08f218494326.js'))); + done(); + }); + }); + }); }); describe('playback', function () { From 68111c5136d875c23904d337aecbbf16b181809a Mon Sep 17 00:00:00 2001 From: Ramesh Sunkara Date: Wed, 28 Jun 2017 22:31:11 -0400 Subject: [PATCH 2/5] accept errorResponse as parameter to server --- index.js | 5 ----- lib/proxy.js | 1 + test/helpers/server.js | 7 ++++--- test/yakbak.js | 17 ++++++++--------- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/index.js b/index.js index 7c52d7f..f497285 100644 --- a/index.js +++ b/index.js @@ -42,18 +42,13 @@ module.exports = function (host, opts) { throw new RecordingDisabledError('Recording Disabled'); } else { return proxy(req, body, host).then(function (pres) { - console.log('Am i here', opts); // eslint-disable-line if (opts.recordOnlySuccess === true) { - console.log('Am i here-1', opts, pres.statusCode); // eslint-disable-line if (successfulResponse.test(pres.statusCode)) { - console.log('Am i here-2', opts); // eslint-disable-line return record(pres.req, pres, file); } else { - console.log('Am i here-3', opts); // eslint-disable-line throw new RecordingDisabledError('Only Successful responses will be recorded'); } } else { - console.log('Am i here-4', opts); // eslint-disable-line return record(pres.req, pres, file); } }); diff --git a/lib/proxy.js b/lib/proxy.js index b2cd7d2..85b95bd 100644 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -24,6 +24,7 @@ var mods = { 'http:': http, 'https:': https }; module.exports = function proxy(req, body, host) { return new Promise(function (resolve /* , reject */) { + console.log('Request host:', host); var uri = url.parse(host); var mod = mods[uri.protocol] || http; var preq = mod.request({ diff --git a/test/helpers/server.js b/test/helpers/server.js index c0a07b7..ac22fa2 100644 --- a/test/helpers/server.js +++ b/test/helpers/server.js @@ -6,13 +6,14 @@ var http = require('http'); /** * Creates a test HTTP server. * @param {Function} done + * @param {boolean} errorResponse - Specifies whether response has to be error or not * @returns {http.Server} */ -module.exports = function createServer(cb) { - +module.exports = function createServer(cb, errorResponse) { + console.log('In create server:', cb, errorResponse); var server = http.createServer(function (req, res) { - res.statusCode = 201; + res.statusCode = errorResponse === true ? 404 : 201; res.setHeader('Content-Type', 'text/html'); res.setHeader('Date', 'Sat, 26 Oct 1985 08:20:00 GMT'); diff --git a/test/yakbak.js b/test/yakbak.js index d0e6568..cd9f0f1 100644 --- a/test/yakbak.js +++ b/test/yakbak.js @@ -12,7 +12,6 @@ var fs = require('fs'); var crypto = require('crypto'); var url = require('url'); -/* eslint-dsiable*/ describe('yakbak', function () { var server, tmpdir, yakbak; @@ -25,12 +24,10 @@ describe('yakbak', function () { }); beforeEach(function (done) { - console.log('Create dir'); tmpdir = createTmpdir(done); }); afterEach(function (done) { - console.log('director deletio dir'); tmpdir.teardown(done); }); @@ -140,18 +137,20 @@ describe('yakbak', function () { }); describe.only("when onlySuccessResponse is enabled", function () { - beforeEach(function () { + beforeEach(function (done) { + // server.teardown(done); + server = createServer(done, true); yakbak = subject(server.host, { dirname: tmpdir.dirname, recordOnlySuccess: true }); }); - it('does not write the tape to disk', function (done) { + it('does not write the tape to disk if response statusCode is not 2XX', function (done) { request(yakbak) - .get('/blahblah/2') - .set('host', 'localhost:4001') + .get('/record/2') + .set('host', 'localhost:3001') .expect(404) .end(function (err) { - console.log('I am in end error', tmpdir.dirname, err); // eslint-disable-line - // assert(!fs.existsSync(tmpdir.join('3234ee470c8605a1837e08f218494326.js'))); + assert.ifError(err); + assert(!fs.existsSync(tmpdir.join('3234ee470c8605a1837e08f218494326.js'))); done(); }); }); From 5a75bf4236e37600cf2e1d85a3e904d22965d2ea Mon Sep 17 00:00:00 2001 From: Ramesh Sunkara Date: Fri, 30 Jun 2017 14:07:43 -0400 Subject: [PATCH 3/5] Update read me and unit tests --- README.md | 1 + index.js | 9 +++++---- test/yakbak.js | 13 +++++++++++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 36d00f9..4e114c0 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ var handler = yakbak('http://api.flickr.com', { - `dirname` the path where recorded responses will be written (required). - `noRecord` if true, requests will return a 404 error if the tape doesn't exist +- `recordOnlySuccess` if true, only successful requests (response status code = 2XX) will be recorded - `hash(req, body)` provide your own IncomingMessage hash function ### with node's http module diff --git a/index.js b/index.js index f497285..8011a68 100644 --- a/index.js +++ b/index.js @@ -18,7 +18,7 @@ var debug = require('debug')('yakbak:server'); * @param {Object} opts * @param {String} opts.dirname The tapes directory * @param {Boolean} opts.noRecord if true, requests will return a 404 error if the tape doesn't exist - * @param {Boolean} opts.recordOnlySuccess if true, requests which return only 2XX will be recorded. + * @param {Boolean} opts.recordOnlySuccess if true, only successful requests will be recorded * @returns {Function} */ @@ -32,7 +32,7 @@ module.exports = function (host, opts) { return buffer(req).then(function (body) { var file = path.join(opts.dirname, tapename(req, body)); - var successfulResponse = /^[2][0|2][0-8]$/; + var successfulResCodePattern = /^[2][0|2][0-8]$/; return Promise.try(function () { return require.resolve(file); @@ -43,7 +43,7 @@ module.exports = function (host, opts) { } else { return proxy(req, body, host).then(function (pres) { if (opts.recordOnlySuccess === true) { - if (successfulResponse.test(pres.statusCode)) { + if (successfulResCodePattern.test(pres.statusCode)) { return record(pres.req, pres, file); } else { throw new RecordingDisabledError('Only Successful responses will be recorded'); @@ -97,7 +97,8 @@ function ModuleNotFoundError(err) { /** * Error class that is thrown when an unmatched request - * is encountered in noRecord mode + * is encountered in noRecord mode or when recordOnlySuccess is enabled + * and the request failed * @constructor */ diff --git a/test/yakbak.js b/test/yakbak.js index cd9f0f1..c182ac7 100644 --- a/test/yakbak.js +++ b/test/yakbak.js @@ -136,10 +136,19 @@ describe('yakbak', function () { }); }); - describe.only("when onlySuccessResponse is enabled", function () { + describe("when onlySuccessResponse is enabled", function () { beforeEach(function (done) { - // server.teardown(done); + /* tear down the server created in global scope as we + need different server object which can send response with failed status code*/ + server.teardown(done); + }); + + beforeEach(function (done) { + /* Send the failed response for the requests this server handles */ server = createServer(done, true); + }); + + beforeEach(function () { yakbak = subject(server.host, { dirname: tmpdir.dirname, recordOnlySuccess: true }); }); From 0f876a792e9fac52ff9df951c48c61598156e21a Mon Sep 17 00:00:00 2001 From: Ramesh Sunkara Date: Fri, 30 Jun 2017 14:10:18 -0400 Subject: [PATCH 4/5] better comments for the RecordingDisabledError function --- index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/index.js b/index.js index 8011a68..793bf89 100644 --- a/index.js +++ b/index.js @@ -97,8 +97,7 @@ function ModuleNotFoundError(err) { /** * Error class that is thrown when an unmatched request - * is encountered in noRecord mode or when recordOnlySuccess is enabled - * and the request failed + * is encountered in noRecord mode or when a request failed in recordOnlySuccess mode * @constructor */ From 5dac0e478430994f99f0d82376b7df4f48304235 Mon Sep 17 00:00:00 2001 From: Ramesh Sunkara Date: Fri, 30 Jun 2017 14:18:00 -0400 Subject: [PATCH 5/5] Remove console logs and do not hard code error message in unit tests --- index.js | 2 +- lib/proxy.js | 1 - test/helpers/server.js | 7 +++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 793bf89..7a5fdf8 100644 --- a/index.js +++ b/index.js @@ -61,7 +61,7 @@ module.exports = function (host, opts) { return tape(req, res); }).catch(RecordingDisabledError, function (err) { /* eslint-disable no-console */ - console.log('An HTTP request has been made that yakbak does not know how to handle'); + console.log(err.message); console.log(curl.request(req)); /* eslint-enable no-console */ res.statusCode = err.status; diff --git a/lib/proxy.js b/lib/proxy.js index 85b95bd..b2cd7d2 100644 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -24,7 +24,6 @@ var mods = { 'http:': http, 'https:': https }; module.exports = function proxy(req, body, host) { return new Promise(function (resolve /* , reject */) { - console.log('Request host:', host); var uri = url.parse(host); var mod = mods[uri.protocol] || http; var preq = mod.request({ diff --git a/test/helpers/server.js b/test/helpers/server.js index ac22fa2..92886aa 100644 --- a/test/helpers/server.js +++ b/test/helpers/server.js @@ -6,14 +6,13 @@ var http = require('http'); /** * Creates a test HTTP server. * @param {Function} done - * @param {boolean} errorResponse - Specifies whether response has to be error or not + * @param {boolean} failRequest - Specifies whether response has to be error or not * @returns {http.Server} */ -module.exports = function createServer(cb, errorResponse) { - console.log('In create server:', cb, errorResponse); +module.exports = function createServer(cb, failRequest) { var server = http.createServer(function (req, res) { - res.statusCode = errorResponse === true ? 404 : 201; + res.statusCode = failRequest === true ? 404 : 201; res.setHeader('Content-Type', 'text/html'); res.setHeader('Date', 'Sat, 26 Oct 1985 08:20:00 GMT');