From 10404945b99e53e0acfef37f7d19baaea669e34e Mon Sep 17 00:00:00 2001
From: KobeN <7845001+kobenguyent@users.noreply.github.com>
Date: Sat, 8 Jun 2024 06:56:51 +0200
Subject: [PATCH] fix: screenshot error in beforeSuite/AfterSuite (#4385)

---
 lib/plugin/screenshotOnFail.js            |  4 ++++
 lib/plugin/stepByStepReport.js            | 12 +++++++-----
 test/unit/plugin/screenshotOnFail_test.js | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/lib/plugin/screenshotOnFail.js b/lib/plugin/screenshotOnFail.js
index 58ef1948a..501ce2777 100644
--- a/lib/plugin/screenshotOnFail.js
+++ b/lib/plugin/screenshotOnFail.js
@@ -73,6 +73,10 @@ module.exports = function (config) {
   }
 
   event.dispatcher.on(event.test.failed, (test) => {
+    if (test.ctx?._runnable.title.includes('hook: ')) {
+      output.plugin('screenshotOnFail', 'BeforeSuite/AfterSuite do not have any access to the browser, hence it could not take screenshot.');
+      return;
+    }
     recorder.add('screenshot of failed test', async () => {
       let fileName = clearString(test.title);
       const dataType = 'image/png';
diff --git a/lib/plugin/stepByStepReport.js b/lib/plugin/stepByStepReport.js
index 52fad61e5..9f22dc576 100644
--- a/lib/plugin/stepByStepReport.js
+++ b/lib/plugin/stepByStepReport.js
@@ -99,12 +99,12 @@ module.exports = function (config) {
     currentTest = test;
   });
 
-  event.dispatcher.on(event.step.failed, persistStep);
-
-  event.dispatcher.on(event.step.after, (step) => {
+  event.dispatcher.on(event.step.failed, (step) => {
     recorder.add('screenshot of failed test', async () => persistStep(step), true);
   });
 
+  event.dispatcher.on(event.step.after, persistStep);
+
   event.dispatcher.on(event.test.passed, (test) => {
     if (!config.deleteSuccessful) return persist(test);
     // cleanup
@@ -112,8 +112,10 @@ module.exports = function (config) {
   });
 
   event.dispatcher.on(event.test.failed, (test, err) => {
-    // BeforeSuite/AfterSuite don't have any access to the browser, hence it could not take screenshot.
-    if (test.ctx._runnable.title.includes('hook: BeforeSuite')) return;
+    if (test.ctx._runnable.title.includes('hook: ')) {
+      output.plugin('stepByStepReport', 'BeforeSuite/AfterSuite do not have any access to the browser, hence it could not take screenshot.');
+      return;
+    }
     persist(test, err);
   });
 
diff --git a/test/unit/plugin/screenshotOnFail_test.js b/test/unit/plugin/screenshotOnFail_test.js
index b2f24a627..fc6b7295f 100644
--- a/test/unit/plugin/screenshotOnFail_test.js
+++ b/test/unit/plugin/screenshotOnFail_test.js
@@ -64,5 +64,19 @@ describe('screenshotOnFail', () => {
     const regexpFileName = /test1_[0-9]{10}.failed.png/;
     expect(fileName.match(regexpFileName).length).is.equal(1);
   });
+
+  it('should not save screenshot in BeforeSuite', async () => {
+    screenshotOnFail({ uniqueScreenshotNames: true });
+    event.dispatcher.emit(event.test.failed, { title: 'test1', ctx: { _runnable: { title: 'hook: BeforeSuite' } } });
+    await recorder.promise();
+    expect(!screenshotSaved.called).is.ok;
+  });
+
+  it('should not save screenshot in AfterSuite', async () => {
+    screenshotOnFail({ uniqueScreenshotNames: true });
+    event.dispatcher.emit(event.test.failed, { title: 'test1', ctx: { _runnable: { title: 'hook: AfterSuite' } } });
+    await recorder.promise();
+    expect(!screenshotSaved.called).is.ok;
+  });
   // TODO: write more tests for different options
 });