From b65fe3878c43984b95e9dd63e981acf0c993ea81 Mon Sep 17 00:00:00 2001 From: sipayrt <sipayrt@yandex.ru> Date: Tue, 17 Oct 2023 18:30:32 +0300 Subject: [PATCH] fix: do not ignore assertView errors in broken session rejection --- src/browser/existing-browser.js | 4 ++ src/worker/runner/test-runner/index.js | 7 ++++ test/src/browser/existing-browser.js | 12 ++++++ test/src/worker/runner/test-runner/index.js | 42 ++++++++++++++++++--- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/browser/existing-browser.js b/src/browser/existing-browser.js index c1d11da21..ae76367b8 100644 --- a/src/browser/existing-browser.js +++ b/src/browser/existing-browser.js @@ -58,6 +58,10 @@ module.exports = class ExistingBrowser extends Browser { } markAsBroken() { + if (this.state.isBroken) { + return; + } + this.applyState({ isBroken: true }); this._stubCommands(); diff --git a/src/worker/runner/test-runner/index.js b/src/worker/runner/test-runner/index.js index 28a5f77c4..ace64318b 100644 --- a/src/worker/runner/test-runner/index.js +++ b/src/worker/runner/test-runner/index.js @@ -86,6 +86,13 @@ module.exports = class TestRunner { } } + // we need to check session twice: + // 1. before afterEach hook to prevent work with broken sessions + // 2. after collecting all assertView errors (including afterEach section) + if (!browser.state.isBroken && isSessionBroken(error, this._config)) { + browser.markAsBroken(); + } + hermioneCtx.assertViewResults = assertViewResults ? assertViewResults.toRawObject() : []; const { meta } = browser; const commandsHistory = callstackHistory ? callstackHistory.release() : []; diff --git a/test/src/browser/existing-browser.js b/test/src/browser/existing-browser.js index 92bb3950c..c9a71cfa3 100644 --- a/test/src/browser/existing-browser.js +++ b/test/src/browser/existing-browser.js @@ -881,6 +881,18 @@ describe("ExistingBrowser", () => { const result = await session.foo(); assert.isUndefined(result); }); + + it("should not mark session as broken twice", async () => { + session.commandList = ["foo"]; + session.foo = () => "foo"; + const browser = await initBrowser_(); + + browser.markAsBroken(); + session.overwriteCommand.resetHistory(); + browser.markAsBroken(); + + assert.notCalled(session.overwriteCommand); + }); }); describe("quit", () => { diff --git a/test/src/worker/runner/test-runner/index.js b/test/src/worker/runner/test-runner/index.js index 0db8f3e47..29967a3ff 100644 --- a/test/src/worker/runner/test-runner/index.js +++ b/test/src/worker/runner/test-runner/index.js @@ -44,6 +44,7 @@ describe("worker/runner/test-runner", () => { const publicAPI = _.defaults(prototype, { $: sandbox.stub().named("$").resolves(mkElement_()), execute: sandbox.stub().named("execute").resolves({ x: 0, y: 0 }), + assertView: sandbox.stub().named("assertView").resolves(), }); config = _.defaults(config, { resetCursor: true }); @@ -52,8 +53,14 @@ describe("worker/runner/test-runner", () => { publicAPI, config, meta: {}, - state: {}, - markAsBroken: sandbox.stub(), + state: { + isBroken: false, + }, + markAsBroken: sandbox.stub().callsFake(() => { + this.state.isBroken = true; + + return sandbox.stub(); + }), }; }; @@ -548,16 +555,41 @@ describe("worker/runner/test-runner", () => { }); describe('in "afterEach" hook', () => { - it("should not mark even if session is broken", async () => { + it("should mark if session is broken", async () => { const config = makeConfigStub({ system: { patternsOnReject: ["FOO_BAR"] } }); - const runner = mkRunner_({ config }); + const test = mkTest_({ fn: sinon.stub().resolves() }); + const runner = mkRunner_({ config, test }); const browser = mkBrowser_(); BrowserAgent.prototype.getBrowser.resolves(browser); + HookRunner.prototype.hasAfterEachHooks.returns(true); HookRunner.prototype.runAfterEachHooks.rejects(new Error("FOO_BAR")); await run_({ runner }).catch(() => {}); - assert.notCalled(browser.markAsBroken); + assert.calledOnce(browser.markAsBroken); + }); + }); + + describe("with assertView errors", () => { + it("should mark if test fails with screenshot error", async () => { + const config = makeConfigStub({ system: { patternsOnReject: ["image comparison failed"] } }); + const runner = mkRunner_({ config }); + const browser = mkBrowser_(); + BrowserAgent.prototype.getBrowser.resolves(browser); + + const assertViewResults = AssertViewResults.create([new Error("image error")]); + + ExecutionThread.create.callsFake(({ hermioneCtx }) => { + ExecutionThread.prototype.run.callsFake(() => { + hermioneCtx.assertViewResults = assertViewResults; + }); + + return Object.create(ExecutionThread.prototype); + }); + + await run_({ runner }).catch(() => {}); + + assert.calledOnce(browser.markAsBroken); }); }); });