fix: do not ignore assertView errors in broken session rejection

gemini-testing · Oct 18, 2023 · b65fe38 · b65fe38
1 parent 05ec28e
commit b65fe38
Show file tree

Hide file tree

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