better hooks handling by submitting where test.failed occur (#4782)

* better hooks handling by submitting where test.failed occur * reverted autologin plugin changes * small fix to screenshot failures * fixed screenshot tests * fixed PW tests --------- Co-authored-by: DavertMik <[email protected]>
codeceptjs · Jan 27, 2025 · ba64d2c · ba64d2c
1 parent 2534959
commit ba64d2c
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 46 deletions.
diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js
@@ -523,12 +523,17 @@ class Playwright extends Helper {
         this.currentRunningTest.artifacts.har = fileName
         contextOptions.recordHar = this.options.recordHar
       }
+
+      // load pre-saved cookies
+      if (test?.opts?.cookies) contextOptions.storageState = { cookies: test.opts.cookies }
+
       if (this.storageState) contextOptions.storageState = this.storageState
       if (this.options.userAgent) contextOptions.userAgent = this.options.userAgent
       if (this.options.locale) contextOptions.locale = this.options.locale
       if (this.options.colorScheme) contextOptions.colorScheme = this.options.colorScheme
       this.contextOptions = contextOptions
       if (!this.browserContext || !restartsSession()) {
+        this.debugSection('New Session', JSON.stringify(this.contextOptions))
         this.browserContext = await this.browser.newContext(this.contextOptions) // Adding the HTTPSError ignore in the context so that we can ignore those errors
       }
     }
@@ -938,7 +943,8 @@ class Playwright extends Helper {
       throw new Error('Cannot open pages inside an Electron container')
     }
     if (!/^\w+\:(\/\/|.+)/.test(url)) {
-      url = this.options.url + (url.startsWith('/') ? url : `/${url}`)
+      url = this.options.url + (!this.options.url.endsWith('/') && url.startsWith('/') ? url : `/${url}`)
+      this.debug(`Changed URL to base url + relative path: ${url}`)
     }
 
     if (this.options.basicAuth && this.isAuthenticated !== true) {

diff --git a/lib/listener/store.js b/lib/listener/store.js
@@ -2,11 +2,19 @@ const event = require('../event')
 const store = require('../store')
 
 module.exports = function () {
+  event.dispatcher.on(event.suite.before, suite => {
+    store.currentSuite = suite
+  })
+
+  event.dispatcher.on(event.suite.after, () => {
+    store.currentSuite = null
+  })
+
   event.dispatcher.on(event.test.before, test => {
     store.currentTest = test
   })
 
-  event.dispatcher.on(event.test.finished, test => {
+  event.dispatcher.on(event.test.finished, () => {
     store.currentTest = null
   })
 }
diff --git a/lib/mocha/asyncWrapper.js b/lib/mocha/asyncWrapper.js
@@ -19,10 +19,10 @@ const injectHook = function (inject, suite) {
   return recorder.promise()
 }
 
-function suiteTestFailedHookError(suite, err) {
+function suiteTestFailedHookError(suite, err, hookName) {
   suite.eachTest(test => {
     test.err = err
-    event.emit(event.test.failed, test, err)
+    event.emit(event.test.failed, test, err, ucfirst(hookName))
   })
 }
 
@@ -120,7 +120,7 @@ module.exports.injected = function (fn, suite, hookName) {
     const errHandler = err => {
       recorder.session.start('teardown')
       recorder.cleanAsyncErr()
-      if (hookName == 'before' || hookName == 'beforeSuite') suiteTestFailedHookError(suite, err)
+      if (hookName == 'before' || hookName == 'beforeSuite') suiteTestFailedHookError(suite, err, hookName)
       if (hookName === 'after') event.emit(event.test.after, suite)
       if (hookName === 'afterSuite') event.emit(event.suite.after, suite)
       recorder.add(() => doneFn(err))

diff --git a/lib/mocha/cli.js b/lib/mocha/cli.js
@@ -198,17 +198,25 @@ class Cli extends Base {
         // add new line before the message
         err.message = '\n  ' + err.message
 
+        // explicitly show file with error
+        if (test.file) {
+          log += `\n${output.styles.basic(figures.circle)} ${output.styles.section('File:')} ${output.styles.basic(test.file)}\n`
+        }
+
         const steps = test.steps || (test.ctx && test.ctx.test.steps)
 
         if (steps && steps.length) {
           let scenarioTrace = ''
-          steps.reverse().forEach(step => {
-            const hasFailed = step.status === 'failed'
-            let line = `${hasFailed ? output.styles.bold(figures.cross) : figures.tick} ${step.toCode()} ${step.line()}`
-            if (hasFailed) line = output.styles.bold(line)
-            scenarioTrace += `\n${line}`
-          })
-          log += `${output.styles.basic(figures.circle)} ${output.styles.section('Scenario Steps')}:${scenarioTrace}\n`
+          steps
+            .reverse()
+            .slice(0, 10)
+            .forEach(step => {
+              const hasFailed = step.status === 'failed'
+              let line = `${hasFailed ? output.styles.bold(figures.cross) : figures.tick} ${step.toCode()} ${step.line()}`
+              if (hasFailed) line = output.styles.bold(line)
+              scenarioTrace += `\n${line}`
+            })
+          log += `\n${output.styles.basic(figures.circle)} ${output.styles.section('Scenario Steps')}:${scenarioTrace}\n`
         }
 
         // display artifacts in debug mode

diff --git a/lib/plugin/analyze.js b/lib/plugin/analyze.js
@@ -12,8 +12,8 @@ const { ansiRegExp, base64EncodeFile, markdownToAnsi } = require('../utils')
 const MAX_DATA_LENGTH = 5000
 
 const defaultConfig = {
-  clusterize: 2,
-  analyze: 3,
+  clusterize: 5,
+  analyze: 2,
   vision: false,
   categories: [
     'Browser connection error / browser crash',
@@ -64,17 +64,18 @@ const defaultConfig = {
         If you identify that all tests in the group have the same tag, add this tag to the group report, otherwise ignore TAG section.
         If you identify that all tests in the group have the same suite, add this suite to the group report, otherwise ignore SUITE section.
         Pick different emojis for each group.
-        Do not include group into report if it has only one test in affected tests section.
+        Order groups by the number of tests in the group.
+        If group has one test, skip that group.
 
         Provide list of groups in following format:
 
         _______________________________
 
-        ## Group <group_number>
+        ## Group <group_number> <emoji>
 
+        * SUMMARY <summary_of_errors>
         * CATEGORY <category_of_failure>
         * ERROR <error_message_1>, <error_message_2>, ...
-        * SUMMARY <summary_of_errors>
         * STEP <step_of_failure> (use CodeceptJS format I.click(), I.see(), etc; if all failures happend on the same step)
         * SUITE <suite_title>, <suite_title> (if SUITE is present, and if all tests in the group have the same suite or suites)
         * TAG <tag> (if TAG is present, and if all tests in the group have the same tag)
@@ -126,14 +127,16 @@ const defaultConfig = {
         Do not get to details, be concise.
         If there is failed step, just write it in STEPS section.
         If you have suggestions for the test, write them in SUMMARY section.
+        Do not be too technical in SUMMARY section.
         Inside SUMMARY write exact values, if you have suggestions, explain which information you used to suggest.
         Be concise, each section should not take more than one sentence.
 
         Response format:
 
+        * SUMMARY <explanation_of_failure>
+        * ERROR <error_message_1>, <error_message_2>, ...
         * CATEGORY <category_of_failure>
         * STEPS <step_of_failure>
-        * SUMMARY <explanation_of_failure>
 
         Do not add any other sections or explanations. Only CATEGORY, SUMMARY, STEPS.
         ${config.vision ? 'Also a screenshot of the page is attached to the prompt.' : ''}
@@ -153,11 +156,6 @@ const defaultConfig = {
         })
       }
 
-      messages.push({
-        role: 'assistant',
-        content: `## `,
-      })
-
       return messages
     },
   },

diff --git a/lib/plugin/pageInfo.js b/lib/plugin/pageInfo.js
@@ -62,10 +62,7 @@ module.exports = function (config = {}) {
     })
     recorder.add('HTML snapshot failed test', async () => {
       try {
-        const currentOutputLevel = output.level()
-        output.level(0)
         const html = await helper.grabHTMLFrom('body')
-        output.level(currentOutputLevel)
 
         if (!html) return
 

diff --git a/lib/plugin/screenshotOnFail.js b/lib/plugin/screenshotOnFail.js
@@ -64,19 +64,20 @@ module.exports = function (config) {
   }
 
   if (Codeceptjs.container.mocha()) {
-    options.reportDir = Codeceptjs.container.mocha().options.reporterOptions && Codeceptjs.container.mocha().options.reporterOptions.reportDir
+    options.reportDir = Codeceptjs.container.mocha()?.options?.reporterOptions && Codeceptjs.container.mocha()?.options?.reporterOptions?.reportDir
   }
 
   if (options.disableScreenshots) {
     // old version of disabling screenshots
     return
   }
 
-  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.')
+  event.dispatcher.on(event.test.failed, (test, _err, hookName) => {
+    if (hookName === 'BeforeSuite' || hookName === 'AfterSuite') {
+      // no browser here
       return
     }
+
     recorder.add(
       'screenshot of failed test',
       async () => {
@@ -139,12 +140,8 @@ module.exports = function (config) {
   })
 
   function _getUUID(test) {
-    if (test.uuid) {
-      return test.uuid
-    }
-
-    if (test.ctx && test.ctx.test.uuid) {
-      return test.ctx.test.uuid
+    if (test.uid) {
+      return test.uid
     }
 
     return Math.floor(new Date().getTime() / 1000)

diff --git a/lib/plugin/stepByStepReport.js b/lib/plugin/stepByStepReport.js
@@ -121,12 +121,13 @@ module.exports = function (config) {
     deleteDir(dir)
   })
 
-  event.dispatcher.on(event.test.failed, (test, err) => {
-    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.')
+  event.dispatcher.on(event.test.failed, (test, _err, hookName) => {
+    if (hookName === 'BeforeSuite' || hookName === 'AfterSuite') {
+      // no browser here
       return
     }
-    persist(test, err)
+
+    persist(test)
   })
 
   event.dispatcher.on(event.all.result, () => {

diff --git a/lib/step.js b/lib/step.js
@@ -14,7 +14,13 @@ const Step = require('./step/helper')
  */
 const MetaStep = require('./step/meta')
 
+/**
+ * Step used to execute a single function
+ */
+const FuncStep = require('./step/func')
+
 module.exports = Step
 module.exports.MetaStep = MetaStep
 module.exports.BaseStep = BaseStep
 module.exports.StepConfig = StepConfig
+module.exports.FuncStep = FuncStep
diff --git a/lib/store.js b/lib/store.js
@@ -15,6 +15,8 @@ const store = {
   currentTest: null,
   /** @type {any} */
   currentStep: null,
+  /** @type {CodeceptJS.Suite | null} */
+  currentSuite: null,
 }
 
 module.exports = store
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "codeceptjs",
-  "version": "3.7.0-beta.1",
+  "version": "3.7.0-beta.8",
   "description": "Supercharged End 2 End Testing Framework for NodeJS",
   "keywords": [
     "acceptance",

diff --git a/test/unit/plugin/screenshotOnFail_test.js b/test/unit/plugin/screenshotOnFail_test.js
@@ -49,14 +49,17 @@ describe('screenshotOnFail', () => {
 
   it('should create screenshot with unique name', async () => {
     screenshotOnFail({ uniqueScreenshotNames: true })
-    event.dispatcher.emit(event.test.failed, { title: 'test1', uuid: 1 })
+
+    const test = { title: 'test1', uid: 1 }
+    event.dispatcher.emit(event.test.failed, test)
     await recorder.promise()
     expect(screenshotSaved.called).is.ok
-    expect('test1_1.failed.png').is.equal(screenshotSaved.getCall(0).args[0])
+    expect(`test1_${test.uid}.failed.png`).is.equal(screenshotSaved.getCall(0).args[0])
   })
 
-  it('should create screenshot with unique name when uuid is null', async () => {
+  it('should create screenshot with unique name when uid is null', async () => {
     screenshotOnFail({ uniqueScreenshotNames: true })
+
     event.dispatcher.emit(event.test.failed, { title: 'test1' })
     await recorder.promise()
     expect(screenshotSaved.called).is.ok
@@ -67,14 +70,14 @@ describe('screenshotOnFail', () => {
 
   it('should not save screenshot in BeforeSuite', async () => {
     screenshotOnFail({ uniqueScreenshotNames: true })
-    event.dispatcher.emit(event.test.failed, { title: 'test1', ctx: { _runnable: { title: 'hook: BeforeSuite' } } })
+    event.dispatcher.emit(event.test.failed, { title: 'test1' }, null, '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' } } })
+    event.dispatcher.emit(event.test.failed, { title: 'test1' }, null, 'AfterSuite')
     await recorder.promise()
     expect(!screenshotSaved.called).is.ok
   })