From a3448ceedd5a46a1da967f4a74153101c9ee5208 Mon Sep 17 00:00:00 2001
From: DudaGod <dudkevich@yandex-team.ru>
Date: Tue, 30 Jan 2024 12:17:37 +0100
Subject: [PATCH] fix: do not lost "testXReqId" when using "parallelLimit"

---
 src/browser-pool/limited-pool.js              | 11 +++----
 src/runner/test-runner/regular-test-runner.js |  2 +-
 test/src/browser-pool/limited-pool.js         | 30 +++++++++++++++----
 3 files changed, 32 insertions(+), 11 deletions(-)

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" });
+            });
         });
     });