diff --git a/src/browser-pool/limited-pool.js b/src/browser-pool/limited-pool.js index 51f50b3a5..88368b012 100644 --- a/src/browser-pool/limited-pool.js +++ b/src/browser-pool/limited-pool.js @@ -49,7 +49,8 @@ module.exports = class LimitedPool extends Pool { --this._requests; const nextRequest = this._lookAtNextRequest(); - const compositeIdForNextRequest = nextRequest && buildCompositeBrowserId(nextRequest.id, nextRequest.version); + const compositeIdForNextRequest = + nextRequest && buildCompositeBrowserId(nextRequest.id, nextRequest.opts.version); const hasFreeSlots = this._launched < this._limit; const shouldFreeUnusedResource = this._isSpecificBrowserLimiter && this._launched > this._requests; const force = opts.force || shouldFreeUnusedResource; @@ -83,10 +84,9 @@ module.exports = class LimitedPool extends Pool { this.log("queuing the request"); const queue = opts.highPriority ? this._highPriorityRequestQueue : this._requestQueue; - const { version } = opts; return new Promise((resolve, reject) => { - queue.push({ id, version, resolve, reject }); + queue.push({ id, opts, resolve, reject }); }); } @@ -111,11 +111,12 @@ module.exports = class LimitedPool extends Pool { const queued = this._highPriorityRequestQueue.shift() || this._requestQueue.shift(); if (queued) { - const compositeId = buildCompositeBrowserId(queued.id, queued.version); + const compositeId = buildCompositeBrowserId(queued.id, queued.opts.version); this.log(`has queued requests for ${compositeId}`); this.log(`remaining queue length: ${this._requestQueue.length}`); - this._newBrowser(queued.id, { version: queued.version }).then(queued.resolve, queued.reject); + + this._newBrowser(queued.id, queued.opts).then(queued.resolve, queued.reject); } else { this._launched--; } diff --git a/src/runner/test-runner/regular-test-runner.js b/src/runner/test-runner/regular-test-runner.js index b4e7ed593..71f04e6b8 100644 --- a/src/runner/test-runner/regular-test-runner.js +++ b/src/runner/test-runner/regular-test-runner.js @@ -86,7 +86,7 @@ module.exports = class RegularTestRunner extends Runner { this._browser = await this._browserAgent.getBrowser({ state }); - // use correct state for cached browsers + // TODO: move logic to caching pool (in order to use correct state for cached browsers) if (this._browser.state.testXReqId !== state.testXReqId) { this._browser.applyState(state); } diff --git a/test/src/browser-pool/limited-pool.js b/test/src/browser-pool/limited-pool.js index bc0092f99..0782cfcd8 100644 --- a/test/src/browser-pool/limited-pool.js +++ b/test/src/browser-pool/limited-pool.js @@ -32,13 +32,33 @@ describe("browser-pool/limited-pool", () => { assert.equal(bro, browser); }); - it("should pass opts to underlying pool", async () => { - const browser = stubBrowser("bro"); - underlyingPool.getBrowser.returns(Promise.resolve(browser)); + describe("should pass opts to underlying pool from", () => { + it("first requested browser", async () => { + const browser = stubBrowser("bro"); + underlyingPool.getBrowser.returns(Promise.resolve(browser)); - await makePool_().getBrowser("bro", { some: "opt" }); + await makePool_().getBrowser("bro", { some: "opt" }); - assert.calledOnceWith(underlyingPool.getBrowser, "bro", { some: "opt" }); + assert.calledOnceWith(underlyingPool.getBrowser, "bro", { some: "opt" }); + }); + + it("queued browser", async () => { + const browser1 = stubBrowser("bro"); + const browser2 = stubBrowser("bro"); + underlyingPool.getBrowser + .onFirstCall() + .returns(Promise.resolve(browser1)) + .onSecondCall() + .returns(Promise.resolve(browser2)); + + const pool = await makePool_({ limit: 1 }); + await pool.getBrowser("bro", { some: "opt1" }); + // should be called without await + Promise.delay(100).then(() => pool.freeBrowser(browser1)); + await pool.getBrowser("bro", { another: "opt2" }); + + assert.calledWith(underlyingPool.getBrowser.secondCall, "bro", { another: "opt2" }); + }); }); });