From 75708f68b4e4e8317695cbd1167400ffa2f21c96 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        | 13 +++++++
 test/src/worker/runner/test-runner/index.js | 38 +++++++++++++++++----
 4 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/src/browser/existing-browser.js b/src/browser/existing-browser.js
index c649b0842..33029d9c7 100644
--- a/src/browser/existing-browser.js
+++ b/src/browser/existing-browser.js
@@ -53,6 +53,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..f41f169f6 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 (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 ead6feba9..aca2bb78e 100644
--- a/test/src/browser/existing-browser.js
+++ b/test/src/browser/existing-browser.js
@@ -723,6 +723,19 @@ 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();
+            const callCount = session.overwriteCommand.callCount;
+            browser.markAsBroken();
+            const callCount2 = session.overwriteCommand.callCount;
+
+            assert.equal(callCount, callCount2);
+        });
     });
 
     describe("quit", () => {
diff --git a/test/src/worker/runner/test-runner/index.js b/test/src/worker/runner/test-runner/index.js
index 0db8f3e47..2cca56afb 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 });
 
@@ -491,7 +492,7 @@ describe("worker/runner/test-runner", () => {
 
                     await run_({ runner }).catch(() => {});
 
-                    assert.calledOnce(browser.markAsBroken);
+                    assert.calledTwice(browser.markAsBroken);
                 });
             });
 
@@ -517,7 +518,7 @@ describe("worker/runner/test-runner", () => {
 
                     await run_({ runner }).catch(() => {});
 
-                    assert.calledOnce(browser.markAsBroken);
+                    assert.calledTwice(browser.markAsBroken);
                 });
             });
 
@@ -543,21 +544,46 @@ describe("worker/runner/test-runner", () => {
 
                     await run_({ runner }).catch(() => {});
 
-                    assert.calledOnce(browser.markAsBroken);
+                    assert.calledTwice(browser.markAsBroken);
                 });
             });
 
             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);
                 });
             });
         });