From 6d9b9cb9f362e54beb4bf2288b234d14f33b2e51 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 18 Sep 2022 18:09:39 +0800 Subject: [PATCH 1/7] fix(TestAgent): attach cookies to agent after plugin is used following superagent: PR: https://github.com/visionmedia/superagent/pull/1556 commit: https://github.com/visionmedia/superagent/commit/4babc5d6ac7a7c28fb829697eb02e3f7d72f2b1c --- lib/agent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/agent.js b/lib/agent.js index 0afb7c7..8adf984 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -55,8 +55,8 @@ methods.forEach(function(method) { 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; }; From 5e23869d1f936aa16c7e82e01a8684f6c54af910 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 18 Sep 2022 18:48:01 +0800 Subject: [PATCH 2/7] feat(TestAgent): decoupled TestAgent and superagent's Agent to support more than `ca`, `key`, `cert` `options` object is now a direct passthrough, TestAgent can support whichever superagent's Agent is expecting i.e. `ca`, `key`, `cert`, `pfx`, `rejectUnauthorized` --- lib/agent.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/agent.js b/lib/agent.js index 8adf984..1b4aa27 100644 --- a/lib/agent.js +++ b/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; } @@ -45,9 +40,7 @@ TestAgent.prototype.host = function(host) { 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); + if (this._host) { req.set('host', this._host); } From 1c8930d2b1e904af8f5c6a567c232d45ef881bc1 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 18 Sep 2022 18:58:34 +0800 Subject: [PATCH 3/7] refactor(TestAgent): removed the host param when creating `Test` object `Test` constructor has already removed the host param in de056d2c904ae19f3bc3ac2b3015f88f06915bab --- lib/agent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/agent.js b/lib/agent.js index 1b4aa27..c501642 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -39,7 +39,7 @@ 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); + const req = new Test(this.app, method.toUpperCase(), url); if (this._host) { req.set('host', this._host); From 8bf4c1401d132a27633043810f859d23743ea1eb Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Tue, 20 Sep 2022 15:58:05 +0800 Subject: [PATCH 4/7] test: 100% test coverage --- test/supertest.js | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/test/supertest.js b/test/supertest.js index 7561d1f..d108b63 100644 --- a/test/supertest.js +++ b/test/supertest.js @@ -1269,7 +1269,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 +1285,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 +1296,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(); From c1c4402ab87019fc0d62ab2f094f419419598869 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Tue, 20 Sep 2022 16:31:31 +0800 Subject: [PATCH 5/7] refactor(test): do not have both `s` and `server`, renamed to `server` for consistency --- test/supertest.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/supertest.js b/test/supertest.js index d108b63..a0b82c7 100644 --- a/test/supertest.js +++ b/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); @@ -93,12 +93,13 @@ 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 () { + server = app.listen(4001, function () { request('http://localhost:4001') .get('/') .end(function (err, res) { From 8847310ea86cebe415e92378eb8f3851d8e4057d Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Tue, 20 Sep 2022 16:36:58 +0800 Subject: [PATCH 6/7] refactor(test): do not hardcode any ports, use ephemeral ports --- test/supertest.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/supertest.js b/test/supertest.js index a0b82c7..a25ce95 100644 --- a/test/supertest.js +++ b/test/supertest.js @@ -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) { @@ -99,8 +99,9 @@ describe('request(app)', function () { res.send('hey'); }); - server = 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); From da1e842171112e08f5044838cf3de4fac2ab7239 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Tue, 4 Oct 2022 02:59:57 +0800 Subject: [PATCH 7/7] ci: added ci --- .github/workflows/node.js.yml | 40 +++++++++++++++++++++++++++++++++++ ci/remove-deps-4-old-node.js | 22 +++++++++++++++++++ package.json | 2 +- 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/node.js.yml create mode 100644 ci/remove-deps-4-old-node.js diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml new file mode 100644 index 0000000..b7bf26c --- /dev/null +++ b/.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 diff --git a/ci/remove-deps-4-old-node.js b/ci/remove-deps-4-old-node.js new file mode 100644 index 0000000..ca1368b --- /dev/null +++ b/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) +); diff --git a/package.json b/package.json index 49a2fda..426ad89 100644 --- a/package.json +++ b/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" },