Merge pull request #825 from gemini-testing/HERMIONE-1328.fix_x_req_id

fix: correctly generate test x request id for each test in one browser

src/browser/browser.js

@@ -28,13 +28,13 @@ module.exports = class Browser {
     constructor(config, opts) {
         this.id = opts.id;
         this.version = opts.version;
-        this.testXReqId = opts.testXReqId;
 
         this._config = config.forBrowser(this.id);
         this._debug = config.system.debug;
         this._session = null;
         this._callstackHistory = null;
         this._state = {
+            ...opts.state,
             isBroken: false,
         };
     }
@@ -93,7 +93,7 @@ module.exports = class Browser {
 
                     if (!req.headers["X-Request-ID"]) {
                         req.headers["X-Request-ID"] = `${
-                            this.testXReqId
+                            this.state.testXReqId
                         }${X_REQUEST_ID_DELIMITER}${crypto.randomUUID()}`;
                     }
 

src/browser/existing-browser.js

@@ -69,7 +69,6 @@ module.exports = class ExistingBrowser extends Browser {
 
     quit() {
         this._meta = this._initMeta();
-        this._state = { isBroken: false };
     }
 
     async prepareScreenshot(selectors, opts = {}) {
@@ -169,7 +168,7 @@ module.exports = class ExistingBrowser extends Browser {
         return {
             pid: process.pid,
             browserVersion: this.version,
-            testXReqId: this.testXReqId,
+            testXReqId: this.state.testXReqId,
             ...this._config.meta,
         };
     }

src/runner/test-runner/regular-test-runner.js

@@ -65,7 +65,7 @@ module.exports = class RegularTestRunner extends Runner {
             sessionCaps: this._browser.capabilities,
             sessionOpts: this._browser.publicAPI.options,
             file: this._test.file,
-            testXReqId: this._browser.testXReqId,
+            state: this._browser.state,
         });
     }
 
@@ -82,7 +82,15 @@ module.exports = class RegularTestRunner extends Runner {
 
     async _getBrowser() {
         try {
-            this._browser = await this._browserAgent.getBrowser({ testXReqId: crypto.randomUUID() });
+            const state = { testXReqId: crypto.randomUUID() };
+
+            this._browser = await this._browserAgent.getBrowser({ state });
+
+            // use correct state for cached browsers
+            if (this._browser.state.testXReqId !== state.testXReqId) {
+                this._browser.applyState(state);
+            }
+
             this._test.sessionId = this._browser.sessionId;
 
             return this._browser;

src/worker/hermione.ts

@@ -11,7 +11,7 @@ export interface WorkerRunTestOpts {
     sessionId: string;
     sessionCaps: WdioBrowser["capabilities"];
     sessionOpts: WdioBrowser["options"];
-    testXReqId: string;
+    state: Record<string, unknown>;
 }
 
 export interface AssertViewResultsSuccess {

src/worker/runner/browser-agent.js

@@ -12,14 +12,14 @@ module.exports = class BrowserAgent {
         this._pool = pool;
     }
 
-    getBrowser({ sessionId, sessionCaps, sessionOpts, testXReqId }) {
+    getBrowser({ sessionId, sessionCaps, sessionOpts, state }) {
         return this._pool.getBrowser({
             browserId: this.browserId,
             browserVersion: this.browserVersion,
             sessionId,
             sessionCaps,
             sessionOpts,
-            testXReqId,
+            state,
         });
     }
 

src/worker/runner/browser-pool.js

@@ -16,11 +16,11 @@ module.exports = class BrowserPool {
         this._calibrator = new Calibrator();
     }
 
-    async getBrowser({ browserId, browserVersion, sessionId, sessionCaps, sessionOpts, testXReqId }) {
+    async getBrowser({ browserId, browserVersion, sessionId, sessionCaps, sessionOpts, state }) {
         const browser = Browser.create(this._config, {
             id: browserId,
             version: browserVersion,
-            testXReqId,
+            state,
             emitter: this._emitter,
         });
 

src/worker/runner/index.js

@@ -27,12 +27,12 @@ module.exports = class Runner extends AsyncEmitter {
         ]);
     }
 
-    async runTest(fullTitle, { browserId, browserVersion, file, sessionId, sessionCaps, sessionOpts, testXReqId }) {
+    async runTest(fullTitle, { browserId, browserVersion, file, sessionId, sessionCaps, sessionOpts, state }) {
         const tests = await this._testParser.parse({ file, browserId });
         const test = tests.find(t => t.fullTitle() === fullTitle);
         const browserAgent = BrowserAgent.create({ id: browserId, version: browserVersion, pool: this._browserPool });
         const runner = TestRunner.create(test, this._config.forBrowser(browserId), browserAgent);
 
-        return runner.run({ sessionId, sessionCaps, sessionOpts, testXReqId });
+        return runner.run({ sessionId, sessionCaps, sessionOpts, state });
     }
 };

src/worker/runner/test-runner/index.js

@@ -22,14 +22,14 @@ module.exports = class TestRunner {
         this._browserAgent = browserAgent;
     }
 
-    async run({ sessionId, sessionCaps, sessionOpts, testXReqId }) {
+    async run({ sessionId, sessionCaps, sessionOpts, state }) {
         const test = this._test;
         const hermioneCtx = test.hermioneCtx || {};
 
         let browser;
 
         try {
-            browser = await this._browserAgent.getBrowser({ sessionId, sessionCaps, sessionOpts, testXReqId });
+            browser = await this._browserAgent.getBrowser({ sessionId, sessionCaps, sessionOpts, state });
         } catch (e) {
             throw Object.assign(e, { hermioneCtx });
         }

test/src/browser/existing-browser.js

@@ -82,7 +82,7 @@ describe("ExistingBrowser", () => {
             });
 
             it('should extend meta-info with "testXReqId" field', () => {
-                const browser = mkBrowser_({}, { testXReqId: "12345" });
+                const browser = mkBrowser_({}, { state: { testXReqId: "12345" } });
 
                 assert.propertyVal(browser.meta, "testXReqId", "12345");
             });
@@ -179,10 +179,10 @@ describe("ExistingBrowser", () => {
 
             it('should add "X-Request-ID" header', async () => {
                 crypto.randomUUID.returns("67890");
-                const testXReqId = "12345";
+                const state = { testXReqId: "12345" };
                 const request = { headers: {} };
 
-                await initBrowser_(mkBrowser_({}, { testXReqId }));
+                await initBrowser_(mkBrowser_({}, { state }));
 
                 const { transformRequest } = webdriverio.attach.lastCall.args[0];
                 transformRequest(request);
@@ -1073,15 +1073,6 @@ describe("ExistingBrowser", () => {
     });
 
     describe("quit", () => {
-        it("should overwrite state field", async () => {
-            const browser = await initBrowser_();
-            const state = browser.state;
-
-            browser.quit();
-
-            assert.notEqual(state, browser.state);
-        });
-
         it("should keep process id in meta", async () => {
             const browser = await initBrowser_();
             const pid = browser.meta.pid;

test/src/browser/new-browser.js

@@ -224,10 +224,10 @@ describe("NewBrowser", () => {
 
             it('should add "X-Request-ID" header', async () => {
                 crypto.randomUUID.returns("67890");
-                const testXReqId = "12345";
+                const state = { testXReqId: "12345" };
                 const request = { headers: {} };
 
-                await mkBrowser_({}, { testXReqId }).init();
+                await mkBrowser_({}, { state }).init();
 
                 const { transformRequest } = webdriverio.remote.lastCall.args[0];
                 transformRequest(request);

test/src/browser/utils.js

@@ -49,14 +49,11 @@ function createBrowserConfig_(opts = {}) {
     };
 }
 
-exports.mkNewBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", testXReqId: "" }) => {
+exports.mkNewBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", state: {} }) => {
     return NewBrowser.create(createBrowserConfig_(configOpts), opts);
 };
 
-exports.mkExistingBrowser_ = (
-    configOpts,
-    opts = { id: "browser", version: "1.0", testXReqId: "", emitter: "emitter" },
-) => {
+exports.mkExistingBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", state: {}, emitter: "emitter" }) => {
     return ExistingBrowser.create(createBrowserConfig_(configOpts), opts);
 };
 

test/src/runner/test-runner/regular-test-runner.js

@@ -89,9 +89,9 @@ describe("runner/test-runner/regular-test-runner", () => {
 
     describe("run", () => {
         it("should get browser before running test", async () => {
-            const testXReqId = "12345";
-            crypto.randomUUID.returns(testXReqId);
-            BrowserAgent.prototype.getBrowser.withArgs({ testXReqId }).resolves(
+            const state = { testXReqId: "12345" };
+            crypto.randomUUID.returns(state.testXReqId);
+            BrowserAgent.prototype.getBrowser.withArgs({ state }).resolves(
                 stubBrowser_({
                     id: "bro",
                     version: "1.0",
@@ -100,7 +100,7 @@ describe("runner/test-runner/regular-test-runner", () => {
                     publicAPI: {
                         options: { foo: "bar" },
                     },
-                    testXReqId,
+                    state,
                 }),
             );
             const workers = mkWorkers_();
@@ -116,11 +116,25 @@ describe("runner/test-runner/regular-test-runner", () => {
                     sessionId: "100500",
                     sessionCaps: { browserName: "bro" },
                     sessionOpts: { foo: "bar" },
-                    testXReqId,
+                    state,
                 }),
             );
         });
 
+        it("should modify state if 'testXReqId' is not actual", async () => {
+            const state = { testXReqId: "12345" };
+            crypto.randomUUID.returns(state.testXReqId);
+            const browser = stubBrowser_({
+                state: { testXReqId: "67890" },
+            });
+            BrowserAgent.prototype.getBrowser.withArgs({ state }).resolves(browser);
+            const workers = mkWorkers_();
+
+            await run_({ workers });
+
+            assert.calledWith(browser.applyState.firstCall, state);
+        });
+
         it("should run test in workers", async () => {
             const test = new Test({
                 file: "foo/bar",
@@ -517,7 +531,7 @@ describe("runner/test-runner/regular-test-runner", () => {
                     },
                 });
 
-                assert.calledOnceWith(browser.applyState, { foo: "bar" });
+                assert.calledWith(browser.applyState, { foo: "bar" });
                 assert.callOrder(browser.applyState, BrowserAgent.prototype.freeBrowser);
             });
 

test/src/worker/runner/browser-agent.js

@@ -17,7 +17,7 @@ describe("worker/browser-agent", () => {
                     sessionId: "100-500",
                     sessionCaps: "some-caps",
                     sessionOpts: "some-opts",
-                    testXReqId: "12345",
+                    state: {},
                 })
                 .returns({ some: "browser" });
             const browserAgent = BrowserAgent.create({ id: "bro-id", version: null, pool: browserPool });
@@ -26,7 +26,7 @@ describe("worker/browser-agent", () => {
                 sessionId: "100-500",
                 sessionCaps: "some-caps",
                 sessionOpts: "some-opts",
-                testXReqId: "12345",
+                state: {},
             });
 
             assert.deepEqual(browser, { some: "browser" });
@@ -40,7 +40,7 @@ describe("worker/browser-agent", () => {
                     sessionId: "100-500",
                     sessionCaps: "some-caps",
                     sessionOpts: "some-opts",
-                    testXReqId: "12345",
+                    state: {},
                 })
                 .returns({ some: "browser" });
             const browserAgent = BrowserAgent.create({ id: "bro-id", version: "10.1", pool: browserPool });
@@ -49,7 +49,7 @@ describe("worker/browser-agent", () => {
                 sessionId: "100-500",
                 sessionCaps: "some-caps",
                 sessionOpts: "some-opts",
-                testXReqId: "12345",
+                state: {},
             });
 
             assert.deepEqual(browser, { some: "browser" });

test/src/worker/runner/browser-pool.js

@@ -54,12 +54,12 @@ describe("worker/browser-pool", () => {
             const browserPool = createPool({ config, emitter });
             Browser.create.returns(stubBrowser({ browserId: "bro-id" }));
 
-            await browserPool.getBrowser({ browserId: "bro-id", browserVersion: "1.0", testXReqId: "12345" });
+            await browserPool.getBrowser({ browserId: "bro-id", browserVersion: "1.0", state: {} });
 
             assert.calledOnceWith(Browser.create, config, {
                 id: "bro-id",
                 version: "1.0",
-                testXReqId: "12345",
+                state: {},
                 emitter,
             });
         });

test/src/worker/runner/index.js

@@ -130,14 +130,14 @@ describe("worker/runner", () => {
                 sessionId: "100500",
                 sessionCaps: "some-caps",
                 sessionOpts: "some-opts",
-                testXReqId: "12345",
+                state: {},
             });
 
             assert.calledOnceWith(TestRunner.prototype.run, {
                 sessionId: "100500",
                 sessionCaps: "some-caps",
                 sessionOpts: "some-opts",
-                testXReqId: "12345",
+                state: {},
             });
         });
     });

test/src/worker/runner/test-runner/index.js

@@ -110,7 +110,7 @@ describe("worker/runner/test-runner", () => {
                 sessionId: "100500",
                 sessionCaps: "some-caps",
                 sessionOpts: "some-opts",
-                testXReqId: "12345",
+                state: {},
             };
 
             await runner.run(opts);