fix: do not ignore assertView errors in broken session rejection

gemini-testing · Oct 17, 2023 · 75708f6 · 75708f6
1 parent 83e33e0
commit 75708f6
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);
                 });
             });
         });