Merge pull request #792 from lamweili/refactor/test

test: 100% coverage

Author: titanism

.github/workflows/node.js.yml

@@ -0,0 +1,40 @@
+# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions
+
+name: Node.js CI
+
+on: [push, pull_request]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        include:
+          - node-version: 10.x
+            test-on-old-node: 1
+          - node-version: 12.x
+            test-on-old-node: 1
+          - node-version: 14.x
+          - node-version: 16.x
+          - node-version: 18.x
+
+    steps:
+    - uses: actions/checkout@v3
+    - name: Use Node.js ${{ matrix.node-version }}
+      uses: actions/setup-node@v3
+      with:
+        node-version: ${{ matrix.node-version }}
+        cache: 'npm'
+    - name: Install Dependencies On Old Node ${{ matrix.node-version }}
+      if: ${{ matrix.test-on-old-node == '1' }}
+      run: node ci/remove-deps-4-old-node.js && yarn install --ignore-scripts
+    - name: Install Dependencies On Node ${{ matrix.node-version }}
+      if: ${{ matrix.test-on-old-node != '1' }}
+      run: yarn install
+    - run: npm test
+    - name: Coverage On Node ${{ matrix.node-version }}
+      run:
+        npm run coverage

ci/remove-deps-4-old-node.js

@@ -0,0 +1,22 @@
+const fs = require('fs');
+const path = require('path');
+const package = require('../package.json');
+
+const UNSUPPORT_DEPS_4_OLD = {
+  'eslint': undefined,
+  'mocha': '6.x'
+};
+
+const deps = Object.keys(UNSUPPORT_DEPS_4_OLD);
+for (const item in package.devDependencies) {
+  if (deps.includes(item)) {
+    package.devDependencies[item] = UNSUPPORT_DEPS_4_OLD[item];
+  }
+}
+
+delete package.scripts.lint;
+
+fs.writeFileSync(
+  path.join(__dirname, '../package.json'),
+  JSON.stringify(package, null, 2)
+);

lib/agent.js

@@ -20,12 +20,7 @@ const Test = require('./test.js');
 function TestAgent(app, options) {
   if (!(this instanceof TestAgent)) return new TestAgent(app, options);
   if (typeof app === 'function') app = http.createServer(app); // eslint-disable-line no-param-reassign
-  if (options) {
-    this._ca = options.ca;
-    this._key = options.key;
-    this._cert = options.cert;
-  }
-  Agent.call(this);
+  Agent.call(this, options);
   this.app = app;
 }
 
@@ -44,19 +39,17 @@ TestAgent.prototype.host = function(host) {
 // override HTTP verb methods
 methods.forEach(function(method) {
   TestAgent.prototype[method] = function(url, fn) { // eslint-disable-line no-unused-vars
-    const req = new Test(this.app, method.toUpperCase(), url, this._host);
-    req.ca(this._ca);
-    req.cert(this._cert);
-    req.key(this._key);
+    const req = new Test(this.app, method.toUpperCase(), url);
+
     if (this._host) {
       req.set('host', this._host);
     }
 
     req.on('response', this._saveCookies.bind(this));
     req.on('redirect', this._saveCookies.bind(this));
     req.on('redirect', this._attachCookies.bind(this, req));
-    this._attachCookies(req);
     this._setDefaults(req);
+    this._attachCookies(req);
 
     return req;
   };

package.json

@@ -27,7 +27,7 @@
   "scripts": {
     "lint": "eslint lib/**/*.js test/**/*.js index.js",
     "lint:fix": "eslint --fix lib/**/*.js test/**/*.js index.js",
-    "pretest": "npm run lint",
+    "pretest": "npm run lint --if-present",
     "test": "nyc --reporter=html --reporter=text mocha --exit --require should --reporter spec --check-leaks",
     "coverage": "nyc report --reporter=text-lcov | coveralls"
   },

test/supertest.js

@@ -20,14 +20,14 @@ function shouldIncludeStackWithThisFile(err) {
 describe('request(url)', function () {
   it('should be supported', function (done) {
     const app = express();
-    let s;
+    let server;
 
     app.get('/', function (req, res) {
       res.send('hello');
     });
 
-    s = app.listen(function () {
-      const url = 'http://localhost:' + s.address().port;
+    server = app.listen(function () {
+      const url = 'http://localhost:' + server.address().port;
       request(url)
         .get('/')
         .expect('hello', done);
@@ -37,14 +37,14 @@ describe('request(url)', function () {
   describe('.end(cb)', function () {
     it('should set `this` to the test object when calling cb', function (done) {
       const app = express();
-      let s;
+      let server;
 
       app.get('/', function (req, res) {
         res.send('hello');
       });
 
-      s = app.listen(function () {
-        const url = 'http://localhost:' + s.address().port;
+      server = app.listen(function () {
+        const url = 'http://localhost:' + server.address().port;
         const test = request(url).get('/');
         test.end(function (err, res) {
           this.should.eql(test);
@@ -80,7 +80,7 @@ describe('request(app)', function () {
       res.send('hey');
     });
 
-    server = app.listen(4000, function () {
+    server = app.listen(function () {
       request(server)
         .get('/')
         .end(function (err, res) {
@@ -93,13 +93,15 @@ describe('request(app)', function () {
 
   it('should work with remote server', function (done) {
     const app = express();
+    let server;
 
     app.get('/', function (req, res) {
       res.send('hey');
     });
 
-    app.listen(4001, function () {
-      request('http://localhost:4001')
+    server = app.listen(function () {
+      const url = 'http://localhost:' + server.address().port;
+      request(url)
         .get('/')
         .end(function (err, res) {
           res.status.should.equal(200);
@@ -1269,7 +1271,7 @@ describe('request.get(url).query(vals) works as expected', function () {
       });
   });
 
-  it('handles unknown errors', function (done) {
+  it('handles unknown errors (err without res)', function (done) {
     const app = express();
 
     nock.disableNetConnect();
@@ -1285,6 +1287,8 @@ describe('request.get(url).query(vals) works as expected', function () {
       // https://github.com/visionmedia/supertest/issues/352
       .expect(200)
       .end(function (err, res) {
+        should.exist(err);
+        should.not.exist(res);
         err.should.be.an.instanceof(Error);
         err.message.should.match(/Nock: Disallowed net connect/);
         shouldIncludeStackWithThisFile(err);
@@ -1294,6 +1298,35 @@ describe('request.get(url).query(vals) works as expected', function () {
     nock.restore();
   });
 
+  // this scenario should never happen
+  // there shouldn't be any res if there is an err
+  // meant for test coverage for lib/test.js#169
+  // https://github.com/visionmedia/supertest/blob/5543d674cf9aa4547927ba6010d31d9474950dec/lib/test.js#L169
+  it('handles unknown errors (err with res)', function (done) {
+    const app = express();
+
+    app.get('/', function (req, res) {
+      res.status(200).send('OK');
+    });
+
+    const resError = new Error();
+    resError.status = 400;
+
+    const serverRes = { status: 200 };
+
+    request(app)
+      .get('/')
+      // private api
+      .assert(resError, serverRes, function (err, res) {
+        should.exist(err);
+        should.exist(res);
+        err.should.equal(resError);
+        res.should.equal(serverRes);
+        // close the server explicitly (as we are not using expect/end/then)
+        this.end(done);
+      });
+  });
+
   it('should assert using promises', function (done) {
     const app = express();