Skip to content

Commit 3e318d7

Browse files
DudaGodDudaGod
authored
Merge pull request #826 from gemini-testing/HERMIONE-1328.fix_x_req_id_hermione7
fix: correctly generate test x request id for each test in one browser (backport to v7)
2 parents 5eab513 + 373be4a commit 3e318d7

File tree

15 files changed

+57
-48
lines changed

15 files changed

+57
-48
lines changed

‎src/browser/browser.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ module.exports = class Browser {
2828
constructor(config, opts) {
2929
this.id = opts.id;
3030
this.version = opts.version;
31-
this.testXReqId = opts.testXReqId;
3231

3332
this._config = config.forBrowser(this.id);
3433
this._debug = config.system.debug;
3534
this._session = null;
3635
this._callstackHistory = null;
3736
this._state = {
37+
...opts.state,
3838
isBroken: false,
3939
};
4040
}
@@ -93,7 +93,7 @@ module.exports = class Browser {
9393

9494
if (!req.headers["X-Request-ID"]) {
9595
req.headers["X-Request-ID"] = `${
96-
this.testXReqId
96+
this.state.testXReqId
9797
}${X_REQUEST_ID_DELIMITER}${crypto.randomUUID()}`;
9898
}
9999

‎src/browser/existing-browser.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ module.exports = class ExistingBrowser extends Browser {
6565

6666
quit() {
6767
this._meta = this._initMeta();
68-
this._state = { isBroken: false };
6968
}
7069

7170
async prepareScreenshot(selectors, opts = {}) {
@@ -165,7 +164,7 @@ module.exports = class ExistingBrowser extends Browser {
165164
return {
166165
pid: process.pid,
167166
browserVersion: this.version,
168-
testXReqId: this.testXReqId,
167+
testXReqId: this.state.testXReqId,
169168
...this._config.meta,
170169
};
171170
}

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ module.exports = class RegularTestRunner extends Runner {
6565
sessionCaps: this._browser.capabilities,
6666
sessionOpts: this._browser.publicAPI.options,
6767
file: this._test.file,
68-
testXReqId: this._browser.testXReqId,
68+
state: this._browser.state,
6969
});
7070
}
7171

@@ -82,7 +82,15 @@ module.exports = class RegularTestRunner extends Runner {
8282

8383
async _getBrowser() {
8484
try {
85-
this._browser = await this._browserAgent.getBrowser({ testXReqId: crypto.randomUUID() });
85+
const state = { testXReqId: crypto.randomUUID() };
86+
87+
this._browser = await this._browserAgent.getBrowser({ state });
88+
89+
// use correct state for cached browsers
90+
if (this._browser.state.testXReqId !== state.testXReqId) {
91+
this._browser.applyState(state);
92+
}
93+
8694
this._test.sessionId = this._browser.sessionId;
8795

8896
return this._browser;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ module.exports = class BrowserAgent {
1212
this._pool = pool;
1313
}
1414

15-
getBrowser({ sessionId, sessionCaps, sessionOpts, testXReqId }) {
15+
getBrowser({ sessionId, sessionCaps, sessionOpts, state }) {
1616
return this._pool.getBrowser({
1717
browserId: this.browserId,
1818
browserVersion: this.browserVersion,
1919
sessionId,
2020
sessionCaps,
2121
sessionOpts,
22-
testXReqId,
22+
state,
2323
});
2424
}
2525

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ module.exports = class BrowserPool {
1616
this._calibrator = new Calibrator();
1717
}
1818

19-
async getBrowser({ browserId, browserVersion, sessionId, sessionCaps, sessionOpts, testXReqId }) {
19+
async getBrowser({ browserId, browserVersion, sessionId, sessionCaps, sessionOpts, state }) {
2020
const browser = Browser.create(this._config, {
2121
id: browserId,
2222
version: browserVersion,
23-
testXReqId,
23+
state,
2424
emitter: this._emitter,
2525
});
2626

‎src/worker/runner/index.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ module.exports = class Runner extends AsyncEmitter {
2727
]);
2828
}
2929

30-
async runTest(fullTitle, { browserId, browserVersion, file, sessionId, sessionCaps, sessionOpts, testXReqId }) {
30+
async runTest(fullTitle, { browserId, browserVersion, file, sessionId, sessionCaps, sessionOpts, state }) {
3131
const tests = await this._testParser.parse({ file, browserId });
3232
const test = tests.find(t => t.fullTitle() === fullTitle);
3333
const browserAgent = BrowserAgent.create({ id: browserId, version: browserVersion, pool: this._browserPool });
3434
const runner = TestRunner.create(test, this._config.forBrowser(browserId), browserAgent);
3535

36-
return runner.run({ sessionId, sessionCaps, sessionOpts, testXReqId });
36+
return runner.run({ sessionId, sessionCaps, sessionOpts, state });
3737
}
3838
};

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ module.exports = class TestRunner {
2222
this._browserAgent = browserAgent;
2323
}
2424

25-
async run({ sessionId, sessionCaps, sessionOpts, testXReqId }) {
25+
async run({ sessionId, sessionCaps, sessionOpts, state }) {
2626
const test = this._test;
2727
const hermioneCtx = test.hermioneCtx || {};
2828

2929
let browser;
3030

3131
try {
32-
browser = await this._browserAgent.getBrowser({ sessionId, sessionCaps, sessionOpts, testXReqId });
32+
browser = await this._browserAgent.getBrowser({ sessionId, sessionCaps, sessionOpts, state });
3333
} catch (e) {
3434
throw Object.assign(e, { hermioneCtx });
3535
}

‎test/src/browser/existing-browser.js‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ describe("ExistingBrowser", () => {
8282
});
8383

8484
it('should extend meta-info with "testXReqId" field', () => {
85-
const browser = mkBrowser_({}, { testXReqId: "12345" });
85+
const browser = mkBrowser_({}, { state: { testXReqId: "12345" } });
8686

8787
assert.propertyVal(browser.meta, "testXReqId", "12345");
8888
});
@@ -177,10 +177,10 @@ describe("ExistingBrowser", () => {
177177

178178
it('should add "X-Request-ID" header', async () => {
179179
crypto.randomUUID.returns("67890");
180-
const testXReqId = "12345";
180+
const state = { testXReqId: "12345" };
181181
const request = { headers: {} };
182182

183-
await initBrowser_(mkBrowser_({}, { testXReqId }));
183+
await initBrowser_(mkBrowser_({}, { state }));
184184

185185
const { transformRequest } = webdriverio.attach.lastCall.args[0];
186186
transformRequest(request);
@@ -1060,15 +1060,6 @@ describe("ExistingBrowser", () => {
10601060
});
10611061

10621062
describe("quit", () => {
1063-
it("should overwrite state field", async () => {
1064-
const browser = await initBrowser_();
1065-
const state = browser.state;
1066-
1067-
browser.quit();
1068-
1069-
assert.notEqual(state, browser.state);
1070-
});
1071-
10721063
it("should keep process id in meta", async () => {
10731064
const browser = await initBrowser_();
10741065
const pid = browser.meta.pid;

‎test/src/browser/new-browser.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,10 +225,10 @@ describe("NewBrowser", () => {
225225

226226
it('should add "X-Request-ID" header', async () => {
227227
crypto.randomUUID.returns("67890");
228-
const testXReqId = "12345";
228+
const state = { testXReqId: "12345" };
229229
const request = { headers: {} };
230230

231-
await mkBrowser_({}, { testXReqId }).init();
231+
await mkBrowser_({}, { state }).init();
232232

233233
const { transformRequest } = webdriverio.remote.lastCall.args[0];
234234
transformRequest(request);

‎test/src/browser/utils.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,11 @@ function createBrowserConfig_(opts = {}) {
4848
};
4949
}
5050

51-
exports.mkNewBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", testXReqId: "" }) => {
51+
exports.mkNewBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", state: {} }) => {
5252
return NewBrowser.create(createBrowserConfig_(configOpts), opts);
5353
};
5454

55-
exports.mkExistingBrowser_ = (
56-
configOpts,
57-
opts = { id: "browser", version: "1.0", testXReqId: "", emitter: "emitter" },
58-
) => {
55+
exports.mkExistingBrowser_ = (configOpts, opts = { id: "browser", version: "1.0", state: {}, emitter: "emitter" }) => {
5956
return ExistingBrowser.create(createBrowserConfig_(configOpts), opts);
6057
};
6158

0 commit comments

Comments
 (0)