From 443056bb7f4b29b2694516d041e4c0403b9669e3 Mon Sep 17 00:00:00 2001 From: DudaGod Date: Thu, 11 Jan 2024 16:08:53 +0300 Subject: [PATCH] fix: correctly generate test x request id for each test in one browser --- src/browser/browser.js | 4 +-- src/browser/existing-browser.js | 3 +-- src/runner/test-runner/regular-test-runner.js | 12 +++++++-- src/worker/hermione.ts | 2 +- src/worker/runner/browser-agent.js | 4 +-- src/worker/runner/browser-pool.js | 4 +-- src/worker/runner/index.js | 4 +-- src/worker/runner/test-runner/index.js | 4 +-- test/src/browser/existing-browser.js | 15 +++-------- test/src/browser/new-browser.js | 4 +-- test/src/browser/utils.js | 7 ++--- .../runner/test-runner/regular-test-runner.js | 26 ++++++++++++++----- test/src/worker/runner/browser-agent.js | 8 +++--- test/src/worker/runner/browser-pool.js | 4 +-- test/src/worker/runner/index.js | 4 +-- test/src/worker/runner/test-runner/index.js | 2 +- 16 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/browser/browser.js b/src/browser/browser.js index 0f79298fb..f278e60d1 100644 --- a/src/browser/browser.js +++ b/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()}`; } diff --git a/src/browser/existing-browser.js b/src/browser/existing-browser.js index e33098059..0a389c29c 100644 --- a/src/browser/existing-browser.js +++ b/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, }; } diff --git a/src/runner/test-runner/regular-test-runner.js b/src/runner/test-runner/regular-test-runner.js index f551498ec..3bdab6e22 100644 --- a/src/runner/test-runner/regular-test-runner.js +++ b/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; diff --git a/src/worker/hermione.ts b/src/worker/hermione.ts index cd7ddb111..9daf4c301 100644 --- a/src/worker/hermione.ts +++ b/src/worker/hermione.ts @@ -11,7 +11,7 @@ export interface WorkerRunTestOpts { sessionId: string; sessionCaps: WdioBrowser["capabilities"]; sessionOpts: WdioBrowser["options"]; - testXReqId: string; + state: Record; } export interface AssertViewResultsSuccess { diff --git a/src/worker/runner/browser-agent.js b/src/worker/runner/browser-agent.js index 43bf7d0e7..3b51e76df 100644 --- a/src/worker/runner/browser-agent.js +++ b/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, }); } diff --git a/src/worker/runner/browser-pool.js b/src/worker/runner/browser-pool.js index c81ab468c..6565ae744 100644 --- a/src/worker/runner/browser-pool.js +++ b/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, }); diff --git a/src/worker/runner/index.js b/src/worker/runner/index.js index e67f06041..39af6525a 100644 --- a/src/worker/runner/index.js +++ b/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 }); } }; diff --git a/src/worker/runner/test-runner/index.js b/src/worker/runner/test-runner/index.js index 15e148580..67791e82c 100644 --- a/src/worker/runner/test-runner/index.js +++ b/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 }); } diff --git a/test/src/browser/existing-browser.js b/test/src/browser/existing-browser.js index a71b7836d..63b493509 100644 --- a/test/src/browser/existing-browser.js +++ b/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; diff --git a/test/src/browser/new-browser.js b/test/src/browser/new-browser.js index 47ae5e96a..0f714a5e5 100644 --- a/test/src/browser/new-browser.js +++ b/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); diff --git a/test/src/browser/utils.js b/test/src/browser/utils.js index a9d506aab..21747e385 100644 --- a/test/src/browser/utils.js +++ b/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); }; diff --git a/test/src/runner/test-runner/regular-test-runner.js b/test/src/runner/test-runner/regular-test-runner.js index 7b3a0e2da..0420ee1d4 100644 --- a/test/src/runner/test-runner/regular-test-runner.js +++ b/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); }); diff --git a/test/src/worker/runner/browser-agent.js b/test/src/worker/runner/browser-agent.js index 486d7d74b..9be24b218 100644 --- a/test/src/worker/runner/browser-agent.js +++ b/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" }); diff --git a/test/src/worker/runner/browser-pool.js b/test/src/worker/runner/browser-pool.js index d7b029ad3..9eabdf34c 100644 --- a/test/src/worker/runner/browser-pool.js +++ b/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, }); }); diff --git a/test/src/worker/runner/index.js b/test/src/worker/runner/index.js index c8025c3ba..f13e7a8fc 100644 --- a/test/src/worker/runner/index.js +++ b/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: {}, }); }); }); diff --git a/test/src/worker/runner/test-runner/index.js b/test/src/worker/runner/test-runner/index.js index b322c0037..c008ade09 100644 --- a/test/src/worker/runner/test-runner/index.js +++ b/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);