diff --git a/README.md b/README.md index 73e175f2..544cde3f 100644 --- a/README.md +++ b/README.md @@ -319,6 +319,27 @@ function hasPreviousAndNextKeys(res) { Perform the request and invoke `fn(err, res)`. +### .concurrently(n, build) + +Run `n` requests concurrently against a single shared server — useful for +testing race conditions and idempotency. `build` is called `n` times with a +request instance bound to that server and the request index, and the returned +promise resolves with the responses in build order: + +```js +const [res1, res2] = await request(app) + .concurrently(2, (r, i) => r.post('/payments').send({ amount: 10 })); + +// exactly one of the two should have won +[res1.status, res2.status].sort().should.eql([200, 409]); +``` + +Requests built by hand and awaited together also share the server: + +```js +await Promise.all([request(server).get('/a'), request(server).get('/b')]); +``` + ## Cookies Here is an example of using the `set` and `not` cookie assertions: diff --git a/index.js b/index.js index 7575a39d..7b8d47f1 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,7 @@ * Module dependencies. */ const methods = require('methods'); +const http = require('http'); let http2; try { http2 = require('http2'); // eslint-disable-line global-require @@ -48,6 +49,46 @@ module.exports = function(app, options = {}) { // Support previous use of del obj.del = obj.delete; + /** + * Run `n` requests concurrently against a single shared server and resolve + * with the responses in build order. `build` is called with a request + * instance bound to that server and the request index. + * + * const responses = await request(app) + * .concurrently(2, (r, i) => r.post('/payments').send(body)); + * + * @param {Number} n number of concurrent requests, >= 2 + * @param {Function} build (req, index) => Test + * @return {Promise} + * @api public + */ + obj.concurrently = function(n, build) { + if (!Number.isInteger(n) || n < 2) { + throw new TypeError( + '.concurrently(n, build) expects n to be an integer >= 2, got ' + n + ); + } + if (typeof build !== 'function') { + throw new TypeError( + '.concurrently(n, build) expects build to be a function, got ' + typeof build + ); + } + + // A function app would get one ephemeral server per request; wrap it once + // so all n requests genuinely race against the same server. + let target = app; + if (typeof app === 'function') { + target = options.http2 ? http2.createServer(app) : http.createServer(app); + } + const shared = module.exports(target, options); + + const tests = []; + for (let i = 0; i < n; i += 1) { + tests.push(build(shared, i)); + } + return global.Promise.all(tests); + }; + return obj; }; diff --git a/lib/test.js b/lib/test.js index f8300881..fd1eb663 100644 --- a/lib/test.js +++ b/lib/test.js @@ -58,12 +58,17 @@ class Test extends Request { * @api private */ serverAddress(app, path) { - const addr = app.address(); - - if (!addr) this._server = app.listen(0); - // } else { - // this._server = app; - // } + if (!app.address()) { + app.listen(0); + // The counter marks the server as supertest-started: only those servers + // are closed, and only after the last in-flight request finishes, so + // concurrent requests can share one server. + app._supertestInflight = 0; // eslint-disable-line no-param-reassign + } + if (app._supertestInflight !== undefined) { + app._supertestInflight += 1; // eslint-disable-line no-param-reassign + this._server = app; + } const port = app.address().port; const protocol = app instanceof Server ? 'https' : 'http'; return protocol + '://127.0.0.1:' + port + path; @@ -138,7 +143,17 @@ class Test extends Request { this.assert(err, res, fn); }; - if (server && server._handle) { + // Release this test's slot exactly once even if end() runs twice, + // e.g. .expect(status, fn) followed by await. + if (server && !this._serverReleased) { + this._serverReleased = true; + server._supertestInflight -= 1; + } + + if (server && server._supertestInflight === 0 && server._handle) { + // Unmark before closing so a server the user re-listens on later is + // not adopted (and closed) by supertest again. + delete server._supertestInflight; // Handle server closing with error handling for already closed servers return server.close((closeError) => { // Ignore ERR_SERVER_NOT_RUNNING errors as the server is already closed diff --git a/test/supertest.js b/test/supertest.js index 87acf58d..44bd5ba8 100644 --- a/test/supertest.js +++ b/test/supertest.js @@ -1,5 +1,6 @@ 'use strict'; +const http = require('http'); const https = require('https'); let http2; try { @@ -1458,3 +1459,106 @@ describeHttp2('http2', function() { }); }); }); + +describe('concurrent requests', function () { + describe('request(server) sharing one ephemeral server', function () { + it('should serve requests racing on the same server (#726)', function () { + const server = http.createServer(function (req, res) { + if (req.url === '/slow') { + setTimeout(function () { + res.end('slow'); + }, 50); + return; + } + res.end('fast'); + }); + + const fast = request(server).get('/fast'); + const slow = request(server).get('/slow'); + + return global.Promise.all([fast, slow]).then(function (responses) { + responses[0].text.should.equal('fast'); + responses[1].text.should.equal('slow'); + // the last in-flight request closed the server + server.listening.should.be.false(); + }); + }); + }); + + describe('.concurrently(n, build)', function () { + it('should run n requests genuinely in parallel against one server', function () { + const waiting = []; + const app = function (req, res) { + waiting.push(res); + // Respond only once every request has arrived: this deadlocks (and + // times out the test) unless the requests are truly concurrent. + if (waiting.length === 2) { + waiting.forEach(function (pending) { + pending.end('raced'); + }); + } + }; + + return request(app) + .concurrently(2, function (r, i) { + return r.get('/race').set('x-index', String(i)); + }) + .then(function (responses) { + responses.length.should.equal(2); + responses[0].text.should.equal('raced'); + responses[1].text.should.equal('raced'); + }); + }); + + it('should support per-request assertions', function () { + const app = function (req, res) { + res.end(req.url); + }; + + return request(app) + .concurrently(2, function (r, i) { + return r.get('/' + i).expect(200).expect('/' + i); + }) + .then(function (responses) { + responses[0].text.should.equal('/0'); + responses[1].text.should.equal('/1'); + }); + }); + + it('should leave a server the user started running', function () { + const server = http.createServer(function (req, res) { + res.end('ok'); + }); + + return new global.Promise(function (resolve) { + server.listen(0, resolve); + }) + .then(function () { + return request(server).concurrently(2, function (r) { + return r.get('/'); + }); + }) + .then(function (responses) { + responses.length.should.equal(2); + server.listening.should.be.true(); + return new global.Promise(function (resolve) { + server.close(resolve); + }); + }); + }); + + it('should reject n lower than 2', function () { + (function () { + request(function (req, res) { res.end(); }).concurrently(1, function (r) { + return r.get('/'); + }); + }).should.throw('.concurrently(n, build) expects n to be an integer >= 2, got 1'); + }); + + it('should reject a non-function build', function () { + (function () { + request(function (req, res) { res.end(); }).concurrently(2, 'nope'); + }).should.throw('.concurrently(n, build) expects build to be a function, got string'); + }); + }); +});