Repository: node-nock/nock
Pull request #1070 (2018-02-24 04:10:42)

Remove versioned tests with different request libraries
This is a follow up for https://github.com/node-nock/nock/pull/859#issuecomment-367541476

> if folks are interested to monitor compatibility with the request library of their liking, they can create separate repositories, pin the nock version, and have greenkeeper make sure that their tests are always run against latest nock. If something breaks they can help fix it.
> 
> I'm trying to make nock easier to maintain and that is quite a burden we put on who ever is maintaining nock right now. Unless there are any objections I'll move forward and remove these tests and also install the latest request version instead of pinning the version, and have greenkeeper make sure for us that our tests pass as new versions get released


Patch:
diff --git a/.travis.yml b/.travis.yml
index d28b512..fdd8808 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,23 +1,12 @@
-sudo: false
 language: node_js
 cache:
   directories:
     - ~/.npm
-notifications:
-  email: false
 node_js:
   - '8'
   - '6'
   - '4'
 
-# Retry install on fail to avoid failing a build on network/disk/external errors
-install:
-  - travis_retry npm install
-
-script:
-  - npm run lint
-  - npm run unit
-  - npm run integration
 after_success:
   - npm run coverage
   - npm run coveralls
diff --git a/lib/recorder.js b/lib/recorder.js
index 43bd74b..33aca01 100644
--- a/lib/recorder.js
+++ b/lib/recorder.js
@@ -52,17 +52,19 @@ var getBodyFromChunks = function(chunks, headers) {
   //  the body shouldn't be merged but instead persisted as an array
   //  of hex strings so that the responses can be mocked one by one.
   if(common.isContentEncoded(headers)) {
-    return _.map(chunks, function(chunk) {
-      if(!Buffer.isBuffer(chunk)) {
-        if (typeof chunk === 'string') {
-          chunk = new Buffer(chunk);
-        } else {
-          throw new Error('content-encoded responses must all be binary buffers');
+    return {
+      body: _.map(chunks, function(chunk) {
+        if(!Buffer.isBuffer(chunk)) {
+          if (typeof chunk === 'string') {
+            chunk = new Buffer(chunk);
+          } else {
+            throw new Error('content-encoded responses must all be binary buffers');
+          }
         }
-      }
 
-      return chunk.toString('hex');
-    });
+        return chunk.toString('hex');
+      })
+    };
   }
 
   var mergedBuffer = common.mergeChunks(chunks);
@@ -72,39 +74,55 @@ var getBodyFromChunks = function(chunks, headers) {
   //    2.  A string buffer which represents a JSON object.
   //    3.  A string buffer which doesn't represent a JSON object.
 
-  if(common.isBinaryBuffer(mergedBuffer)) {
-    return mergedBuffer.toString('hex');
+  var isBinary = common.isBinaryBuffer(mergedBuffer)
+  if(isBinary) {
+    return {
+      body: mergedBuffer.toString('hex'),
+      isBinary
+    }
   } else {
     var maybeStringifiedJson = mergedBuffer.toString('utf8');
     try {
-      return JSON.parse(maybeStringifiedJson);
+      return {
+        body: JSON.parse(maybeStringifiedJson),
+        isBinary: false
+      };
     } catch(err) {
-      return maybeStringifiedJson;
+      return {
+        body: maybeStringifiedJson,
+        isBinary: false
+      };
     }
   }
-
 };
 
 function generateRequestAndResponseObject(req, bodyChunks, options, res, dataChunks) {
 
+  var response = getBodyFromChunks(dataChunks, res.headers)
   options.path = req.path;
-  return {
+
+  var nockDef = {
     scope:    getScope(options),
     method:   getMethod(options),
     path:     options.path,
-    body:     getBodyFromChunks(bodyChunks),
+    body:     getBodyFromChunks(bodyChunks).body,
     status:   res.statusCode,
-    response: getBodyFromChunks(dataChunks, res.headers),
+    response: response.body,
     rawHeaders: res.rawHeaders || res.headers,
     reqheaders: req._headers
   };
 
+  if (response.isBinary) {
+    nockDef.responseIsBinary = true
+  }
+
+  return nockDef;
 }
 
 function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
 
-  var requestBody = getBodyFromChunks(bodyChunks);
-  var responseBody = getBodyFromChunks(dataChunks, res.headers);
+  var requestBody = getBodyFromChunks(bodyChunks).body;
+  var responseBody = getBodyFromChunks(dataChunks, res.headers).body;
 
   // Remove any query params from options.path so they can be added in the query() function
   var path = options.path;
diff --git a/lib/scope.js b/lib/scope.js
index f8b07be..39804c1 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -314,6 +314,8 @@ function define(nockDefs) {
     var response;
     if (!nockDef.response) {
       response = '';
+    } else if (nockDef.responseIsBinary) {
+      response = Buffer.from(nockDef.response, 'hex')
     } else {
       response = _.isString(nockDef.response) ? tryJsonParse(nockDef.response) : nockDef.response;
     }
diff --git a/package.json b/package.json
index 2119420..3240fcd 100644
--- a/package.json
+++ b/package.json
@@ -201,9 +201,8 @@
   },
   "scripts": {
     "unit": "tap --harmony ./tests/test_*.js",
-    "preintegration": "./tests/bin/install_sub_deps versioned",
-    "integration": "tap --harmony ./tests/versioned/*/*.tap.js",
-    "test": "npm run lint && npm run unit && npm run integration",
+    "pretest": "npm run -s lint",
+    "test": "npm run -s unit",
     "coverage": "nyc tap --harmony ./tests/test_*.js",
     "coveralls": "cat ./coverage/lcov.info | coveralls",
     "lint": "eslint '**/*.js'",


Test patch:
diff --git a/tests/bin/install_sub_deps b/tests/bin/install_sub_deps
deleted file mode 100755
index 0dbe51b..0000000
--- a/tests/bin/install_sub_deps
+++ /dev/null
@@ -1,61 +0,0 @@
-#!/usr/bin/env node
-/* Modeled after: https://github.com/newrelic/node-newrelic/blob/master/test/bin/install_sub_deps */
-'use strict'
-
-const path = require('path')
-const glob = require('glob')
-const assert = require('assert')
-const exec = require('child_process').exec
-
-const folder = process.argv[2];
-const retries = 2
-
-assert(folder, 'Missing folder!');
-
-glob(`**/tests/${folder}/**/package.json`, {}, (err, packages) => {
-  if (err) { fail(err); }
-
-  install(packages.filter((line) => !/node_modules|example/.test(line)));
-});
-
-const install = (packages, retry)  => {
-  if (!retry) retry = 0
-
-  const failed = []
-  let counter = 0
-
-  packages.forEach((packageJsonPath) => {
-    const packagePath = path.resolve(packageJsonPath).split('/').slice(0, -1).join('/');
-
-    console.log('installing deps in %s', packagePath)
-    exec(`cd ${packagePath} && rm -rf node_modules && npm i`, {
-      maxBuffer: 1024 * 500
-    }, (err, stdout, stderr) => {
-      if (err || isErrorOutput(stderr)) {
-        printError(packageJsonPath, err || stderr)
-        if (retry < retries) {
-          failed.push(packageJsonPath)
-        }
-      }
-
-      if (++counter === packages.length) {
-        if (failed.length > 0 && retry < retries) {
-          console.log('%s package(s) failed to install, retrying...', failed.length)
-          exec('npm cache clean', () => install(failed, ++retry));
-        }
-      }
-    })
-  })
-}
-
-// determines if stderr contains errors
-function isErrorOutput(text) {
-  return (text && text.indexOf('npm ERR') > -1)
-}
-
-function printError(packagePath, error) {
-  console.log('-----------------------------------------------------------')
-  console.log(packagePath + ' failed installing with the following error:')
-  console.log(error)
-  console.log('-----------------------------------------------------------')
-}
\ No newline at end of file
diff --git a/tests/fixtures/browserify-script.js b/tests/fixtures/browserify-script.js
deleted file mode 100644
index 8f562f8..0000000
--- a/tests/fixtures/browserify-script.js
+++ /dev/null
@@ -1,23 +0,0 @@
-/* eslint-env browser */
-
-'use strict';
-
-var assert = require('assert');
-var http = require('http');
-var nock = require('../../');
-
-nock.disableNetConnect();
-nock('http://browserifyland.com').get('/beep').reply(200, 'boop');
-
-http.get('http://browserifyland.com/beep', function(res) {
-  res.setEncoding('utf8');
-  var body = '';
-  res
-    .on('data', function(d) {
-      body += d;
-    })
-    .once('end', function() {
-      assert.equal(body, 'boop');
-      document.getElementById('content').innerHTML = body;
-    });
-});
diff --git a/tests/test_recorder.js b/tests/test_recorder.js
index 03ec045..8272f2d 100644
--- a/tests/test_recorder.js
+++ b/tests/test_recorder.js
@@ -932,6 +932,92 @@ test("respects http.request() consumers", function(t) {
   });
 });
 
+test('records and replays binary response correctly', (t) => {
+  nock.restore()
+  nock.recorder.clear()
+  t.equal(nock.recorder.play().length, 0)
+
+  nock.recorder.rec({
+    output_objects: true,
+    dont_print: true
+  })
+
+  const transparentGifHex = '47494638396101000100800000000000ffffff21f90401000000002c000000000100010000020144003b'
+  const transparentGifBuffer = Buffer.from(transparentGifHex, 'hex')
+
+  // start server that always respondes with transparent gif at available port
+  const server = http.createServer(function (request, response) {
+    response.writeHead(201, {
+      'Content-Type': 'image/gif',
+      'Content-Length': transparentGifBuffer.length
+    })
+    response.write(transparentGifBuffer, 'binary')
+    response.end()
+  })
+
+  server.listen(0, (error) => {
+    t.error(error)
+
+    // send post request upload the same image to server
+    const postRequestOptions = {
+      method: 'PUT',
+      host: 'localhost',
+      port: server.address().port,
+      path: '/clear.gif',
+      headers: {
+        'Content-Type': 'image/gif',
+        'Content-Length': transparentGifBuffer.length
+      }
+    }
+    const postRequest1 = http.request(postRequestOptions, function (response) {
+      var data = []
+
+      response.on('data', function (chunk) {
+        data.push(chunk)
+      })
+
+      response.on('end', function () {
+        const result = Buffer.concat(data).toString('hex')
+        t.equal(result, transparentGifHex, 'received gif equals check value')
+
+        const recordedFixtures = nock.recorder.play()
+
+        // stope server, stope recording, start intercepting
+        server.close((error) => {
+          t.error(error)
+
+          nock.restore()
+          nock.activate()
+          nock.define(recordedFixtures)
+
+          // send same post request again
+          const postRequest2 = http.request(postRequestOptions, function (response) {
+            var data = []
+
+            response.on('data', function (chunk) {
+              data.push(chunk)
+            })
+
+            response.on('end', function () {
+              const result = Buffer.concat(data).toString('hex')
+
+              // expect same outcome, end tests
+              t.equal(result, transparentGifHex, 'received gif equals check value')
+              t.end()
+            })
+          })
+
+          postRequest2.write(transparentGifBuffer)
+          postRequest2.end()
+        })
+      })
+    })
+
+    postRequest1.write(transparentGifBuffer)
+    postRequest1.end()
+  })
+})
+
 test("teardown", function(t) {
   var leaks = Object.keys(global)
     .splice(globalCount, Number.MAX_VALUE);
diff --git a/tests/versioned/common.js b/tests/versioned/common.js
deleted file mode 100644
index 1fd303d..0000000
--- a/tests/versioned/common.js
+++ /dev/null
@@ -1,49 +0,0 @@
-'use strict';
-
-const tap = require('tap');
-const nock = require('../../');
-
-/**
- * Run common test suite against an HTTP client library.
- *
- * The makeRequest fn should be a wrapper around an
- * HTTP client, which maps our arguments to those accepted by the client.
- *
- * The client is expected to accept an options object with props:
- * - uri
- * - method
- * - timeout
- * And return a promise, which in success has properties
- * - statusCode (number)
- * - body (string)
- * And in failure is a standard error object.
- *
- * @param {Function} makeRequest Request fn
- * @param {string} name Lib we're testing
- */
-const runCommonTests = (makeRequest, name) => {
-  const test = (description, cb) => {
-    return tap.test(`${name}: ${description}`, cb);
-  };
-
-  test('basic intercept', (t) => {
-    nock('http://www.example.com')
-      .get('/')
-      .reply(200, 'OK');
-
-    return makeRequest({
-        uri: 'http://www.example.com'
-      })
-      .then((res) => {
-        t.equal(res.statusCode, 200);
-        t.deepEqual(res.body, 'OK');
-
-        t.end();
-      })
-      .catch(t.threw);
-  });
-};
-
-module.exports = {
-  runCommonTests
-};
diff --git a/tests/versioned/got-latest/package.json b/tests/versioned/got-latest/package.json
deleted file mode 100644
index 89c6f88..0000000
--- a/tests/versioned/got-latest/package.json
+++ /dev/null
@@ -1,14 +0,0 @@
-
-{
-  "name": "got-latest",
-  "version": "0.0.0",
-  "description": "Tests for got latest (>6)",
-  "main": "test.tap.js",
-  "scripts": {
-    "test": "tap test.tap.js"
-  },
-  "dependencies": {
-    "got": "^6"
-  },
-  "private": true
-}
\ No newline at end of file
diff --git a/tests/versioned/got-latest/test.tap.js b/tests/versioned/got-latest/test.tap.js
deleted file mode 100644
index 2533e3d..0000000
--- a/tests/versioned/got-latest/test.tap.js
+++ /dev/null
@@ -1,13 +0,0 @@
-'use strict';
-
-const got = require('got');
-const common = require('../common');
-
-const makeRequest = (options) => {
-  return got(options.uri, {
-    method: options.method,
-    timeout: options.timeout
-  });
-};
-
-common.runCommonTests(makeRequest, 'got@latest');
diff --git a/tests/versioned/popsicle-latest/package.json b/tests/versioned/popsicle-latest/package.json
deleted file mode 100644
index e6c96b6..0000000
--- a/tests/versioned/popsicle-latest/package.json
+++ /dev/null
@@ -1,14 +0,0 @@
-
-{
-  "name": "posicle-latest",
-  "version": "0.0.0",
-  "description": "Tests for popsicle latest (>9)",
-  "main": "test.tap.js",
-  "scripts": {
-    "test": "tap test.tap.js"
-  },
-  "dependencies": {
-    "popsicle": "^9"
-  },
-  "private": true
-}
\ No newline at end of file
diff --git a/tests/versioned/popsicle-latest/test.tap.js b/tests/versioned/popsicle-latest/test.tap.js
deleted file mode 100644
index cd8472a..0000000
--- a/tests/versioned/popsicle-latest/test.tap.js
+++ /dev/null
@@ -1,18 +0,0 @@
-'use strict';
-
-const popsicle = require('popsicle');
-const common = require('../common');
-
-const makeRequest = (options) => {
-  return popsicle.request({
-    url: options.uri,
-    method: options.method || 'GET'
-  })
-  .then((res) => {
-    res.statusCode = res.status;
-
-    return res;
-  })
-};
-
-common.runCommonTests(makeRequest, 'popsicle@latest');
diff --git a/tests/versioned/request-2.71.0/test.tap.js b/tests/versioned/request-2.71.0/test.tap.js
deleted file mode 100644
index 45caf94..0000000
--- a/tests/versioned/request-2.71.0/test.tap.js
+++ /dev/null
@@ -1,22 +0,0 @@
-'use strict';
-
-const request = require('request');
-const common = require('../common');
-
-const makeRequest = (options) => {
-  return new Promise((resolve, reject) => {
-    request({
-      uri: options.uri,
-      method: options.method || 'GET'
-    }, (err, res, body) => {
-      if (err) { return reject(err); }
-
-      res.body = body;
-      resolve(res);
-    });
-  });
-};
-
-common.runCommonTests(makeRequest, 'request@latest');
-
-require('./test_timeout.js')