fix: report results correctly when the browser crashes mid-test (#27786)

* report correct number of failures if browser crashes in the middle of a test suite

* report failure correctly when browser crashes during test

* refactor crash handling

* correct exit option check, clean up debug

* exit on success if exit option !== false

* use default stats when reporter stats are unavailable

* fix error messaging

* move reporter types to an intermediate .ts file, to be combined with reporter.js when migrated

* debug tab close test in ci

* move debug env from pkg to ci yml

* set debug env in spec

* fix pckg

* adds some logging to cri-client

* remove event emit logging from project-base

* revert snapshot for tab close system test

* fixes console output for no exit on success

* changelog

* changelog wsp

* cleanup

* clean up tests

* refactor to more straightforward control flow

* rm export for unused type

* correct tab close snapshot for ci

* new system test for mid-test config crash

* update snapshots

Author: cacieprins

cli/CHANGELOG.md

@@ -8,6 +8,7 @@ _Released 10/03/2023 (PENDING)_
 - Fixed an issue where requests were correlated in the wrong order in the proxy. This could cause an issue where the wrong request is used for `cy.intercept` or assets (e.g. stylesheets or images) may not properly be available in Test Replay. Addressed in [#27892](https://github.com/cypress-io/cypress/pull/27892).
 - Fixed an issue where a crashed Chrome renderer can cause the Test Replay recorder to hang. Addressed in [#27909](https://github.com/cypress-io/cypress/pull/27909).
 - Fixed an issue where multiple responses yielded from calls to `cy.wait()` would sometimes be out of order. Fixes [#27337](https://github.com/cypress-io/cypress/issues/27337).
+- Enables test replay for executed specs in runs that have a spec that causes a browser crash. Addressed in [#27786](https://github.com/cypress-io/cypress/pull/27786)
 
 ## 13.3.0
 

packages/server/lib/modes/run.ts

@@ -21,12 +21,13 @@ import random from '../util/random'
 import system from '../util/system'
 import chromePolicyCheck from '../util/chrome_policy_check'
 import type { SpecWithRelativeRoot, SpecFile, TestingType, OpenProjectLaunchOpts, FoundBrowser, BrowserVideoController, VideoRecording, ProcessOptions } from '@packages/types'
-import type { Cfg } from '../project-base'
+import type { Cfg, ProjectBase } from '../project-base'
 import type { Browser } from '../browsers/types'
 import * as printResults from '../util/print-run'
 import type { ProtocolManager } from '../cloud/protocol'
 import { telemetry } from '@packages/telemetry'
 import { CypressRunResult, createPublicBrowser, createPublicConfig, createPublicRunResults, createPublicSpec, createPublicSpecResults } from './results'
+import { EarlyExitTerminator } from '../util/graceful_crash_handling'
 
 type SetScreenshotMetadata = (data: TakeScreenshotProps) => void
 type ScreenshotMetadata = ReturnType<typeof screenshotMetadata>
@@ -36,18 +37,13 @@ type BeforeSpecRun = any
 type AfterSpecRun = any
 type Project = NonNullable<ReturnType<typeof openProject['getProject']>>
 
-let exitEarly = (err) => {
-  debug('set early exit error: %s', err.stack)
-
-  earlyExitErr = err
-}
-let earlyExitErr: Error
 let currentSetScreenshotMetadata: SetScreenshotMetadata
 
 const debug = Debug('cypress:server:run')
-
 const DELAY_TO_LET_VIDEO_FINISH_MS = 1000
 
+let earlyExitTerminator = new EarlyExitTerminator()
+
 const relativeSpecPattern = (projectRoot, pattern) => {
   if (typeof pattern === 'string') {
     return pattern.replace(`${projectRoot}/`, '')
@@ -411,53 +407,30 @@ function launchBrowser (options: { browser: Browser, spec: SpecWithRelativeRoot,
   return openProject.launch(browser, spec, browserOpts)
 }
 
-function listenForProjectEnd (project, exit): Bluebird<any> {
+async function listenForProjectEnd (project: ProjectBase, exit: boolean): Promise<any> {
   if (globalThis.CY_TEST_MOCK?.listenForProjectEnd) return Bluebird.resolve(globalThis.CY_TEST_MOCK.listenForProjectEnd)
 
-  return new Bluebird((resolve, reject) => {
-    if (exit === false) {
-      resolve = () => {
+  // if exit is false, we need to intercept the resolution of tests - whether
+  // an early exit with intermediate results, or a full run.
+  return new Promise((resolve, reject) => {
+    Promise.race([
+      new Promise((res) => {
+        project.once('end', (results) => {
+          debug('project ended with results %O', results)
+          res(results)
+        })
+      }),
+      earlyExitTerminator.waitForEarlyExit(project, exit),
+    ]).then((results) => {
+      if (exit === false) {
+        // eslint-disable-next-line no-console
         console.log('not exiting due to options.exit being false')
+      } else {
+        resolve(results)
       }
-    }
-
-    const onEarlyExit = function (err) {
-      if (err.isFatalApiErr) {
-        return reject(err)
-      }
-
-      console.log('')
-      errors.log(err)
-
-      const obj = {
-        error: errors.stripAnsi(err.message),
-        stats: {
-          failures: 1,
-          tests: 0,
-          passes: 0,
-          pending: 0,
-          suites: 0,
-          skipped: 0,
-          wallClockDuration: 0,
-          wallClockStartedAt: new Date().toJSON(),
-          wallClockEndedAt: new Date().toJSON(),
-        },
-      }
-
-      return resolve(obj)
-    }
-
-    project.once('end', (results) => resolve(results))
-
-    // if we already received a reason to exit early, go ahead and do it
-    if (earlyExitErr) {
-      return onEarlyExit(earlyExitErr)
-    }
-
-    // otherwise override exitEarly so we exit as soon as there is a reason
-    exitEarly = (err) => {
-      onEarlyExit(err)
-    }
+    }).catch((err) => {
+      reject(err)
+    })
   })
 }
 
@@ -730,6 +703,13 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens
     results.video = null
   }
 
+  // the early exit terminator persists between specs,
+  // so if this spec crashed, the next one will report as
+  // a crash too unless it is reset. Would like to not rely
+  // on closure, but threading through fn props via options is also not
+  // great.
+  earlyExitTerminator = new EarlyExitTerminator()
+
   return results
 }
 
@@ -1005,7 +985,11 @@ async function ready (options: ReadyOptions) {
   // this needs to be a closure over `exitEarly` and not a reference
   // because `exitEarly` gets overwritten in `listenForProjectEnd`
   // TODO: refactor this so we don't need to extend options
-  const onError = options.onError = (err) => exitEarly(err)
+
+  const onError = options.onError = (err) => {
+    debug('onError')
+    earlyExitTerminator.exitEarly(err)
+  }
 
   // alias and coerce to null
   let specPatternFromCli = options.spec || null
@@ -1140,6 +1124,7 @@ async function ready (options: ReadyOptions) {
 }
 
 export async function run (options, loading: Promise<void>) {
+  debug('run start')
   // Check if running as electron process
   if (require('../util/electron-app').isRunningAsElectronProcess({ debug })) {
     const app = require('electron').app
@@ -1161,6 +1146,8 @@ export async function run (options, loading: Promise<void>) {
   try {
     return ready(options)
   } catch (e) {
-    return exitEarly(e)
+    debug('caught outer error', e)
+
+    return earlyExitTerminator.exitEarly(e)
   }
 }

packages/server/lib/project-base.ts

@@ -376,7 +376,6 @@ export class ProjectBase extends EE {
       },
 
       onMocha: async (event, runnable) => {
-        debug('onMocha', event)
         // bail if we dont have a
         // reporter instance
         if (!reporterInstance) {
@@ -385,7 +384,12 @@ export class ProjectBase extends EE {
 
         reporterInstance.emit(event, runnable)
 
-        if (event === 'end') {
+        if (event === 'test:before:run') {
+          this.emit('test:before:run', {
+            runnable,
+            previousResults: reporterInstance?.results() || {},
+          })
+        } else if (event === 'end') {
           const [stats = {}] = await Promise.all([
             (reporterInstance != null ? reporterInstance.end() : undefined),
             this.server.end(),

packages/server/lib/types/reporter.ts

@@ -0,0 +1,48 @@
+interface ReporterTestAttempt {
+  state: 'skipped' | 'failed' | 'passed'
+  error: any
+  timings: any
+  failedFromHookId: any
+  wallClockStartedAt: Date
+  wallClockDuration: number
+  videoTimestamp: any
+}
+interface ReporterTest {
+  testId: string
+  title: string[]
+  state: 'skipped' | 'passed' | 'failed'
+  body: string
+  displayError: any
+  attempts: ReporterTestAttempt[]
+}
+
+export interface BaseReporterResults {
+  error?: string
+  stats: {
+    failures: number
+    tests: number
+    passes: number
+    pending: number
+    suites: number
+    skipped: number
+    wallClockDuration: number
+    wallClockStartedAt: string
+    wallClockEndedAt: string
+  }
+}
+
+export interface ReporterResults extends BaseReporterResults {
+  reporter: string
+  reporterStats: {
+    suites: number
+    tests: number
+    passes: number
+    pending: number
+    failures: number
+    start: string
+    end: string
+    duration: number
+  }
+  hooks: any[]
+  tests: ReporterTest[]
+}

packages/server/lib/util/graceful_crash_handling.ts

@@ -0,0 +1,115 @@
+import type { ProjectBase } from '../project-base'
+import type { BaseReporterResults, ReporterResults } from '../types/reporter'
+import * as errors from '../errors'
+import Debug from 'debug'
+import pDefer, { DeferredPromise } from 'p-defer'
+
+const debug = Debug('cypress:util:crash_handling')
+
+const patchRunResultsAfterCrash = (error: Error, reporterResults: ReporterResults, mostRecentRunnable: any): ReporterResults => {
+  const endTime: number = reporterResults?.stats?.wallClockEndedAt ? Date.parse(reporterResults?.stats?.wallClockEndedAt) : new Date().getTime()
+  const wallClockDuration = reporterResults?.stats?.wallClockStartedAt ?
+    endTime - Date.parse(reporterResults.stats.wallClockStartedAt) : 0
+  const endTimeStamp = new Date(endTime).toJSON()
+
+  // in crash situations, the most recent report will not have the triggering test
+  // so the results are manually patched, which produces the expected exit=1 and
+  // terminal output indicating the failed test
+  return {
+    ...reporterResults,
+    stats: {
+      ...reporterResults?.stats,
+      wallClockEndedAt: endTimeStamp,
+      wallClockDuration,
+      failures: (reporterResults?.stats?.failures ?? 0) + 1,
+      skipped: (reporterResults?.stats?.skipped ?? 1) - 1,
+    },
+    reporterStats: {
+      ...reporterResults?.reporterStats,
+      tests: (reporterResults?.reporterStats?.tests ?? 0) + 1, // crashed test does not increment this value
+      end: reporterResults?.reporterStats?.end || endTimeStamp,
+      duration: wallClockDuration,
+      failures: (reporterResults?.reporterStats?.failures ?? 0) + 1,
+    },
+    tests: (reporterResults?.tests || []).map((test) => {
+      if (test.testId === mostRecentRunnable.id) {
+        return {
+          ...test,
+          state: 'failed',
+          attempts: [
+            ...test.attempts.slice(0, -1),
+            {
+              ...test.attempts[test.attempts.length - 1],
+              state: 'failed',
+            },
+          ],
+        }
+      }
+
+      return test
+    }),
+    error: errors.stripAnsi(error.message),
+  }
+}
+
+const defaultStats = (error: Error): BaseReporterResults => {
+  return {
+    error: errors.stripAnsi(error.message),
+    stats: {
+      failures: 1,
+      tests: 0,
+      passes: 0,
+      pending: 0,
+      suites: 0,
+      skipped: 0,
+      wallClockDuration: 0,
+      wallClockStartedAt: new Date().toJSON(),
+      wallClockEndedAt: new Date().toJSON(),
+    },
+  }
+}
+
+export class EarlyExitTerminator {
+  private terminator: DeferredPromise<BaseReporterResults>
+
+  private pendingRunnable: any
+  private intermediateStats: ReporterResults | undefined
+
+  constructor () {
+    this.terminator = pDefer<BaseReporterResults>()
+  }
+
+  waitForEarlyExit (project: ProjectBase, exit?: boolean) {
+    debug('waiting for early exit')
+
+    project.on('test:before:run', ({
+      runnable,
+      previousResults,
+    }) => {
+      debug('preparing to run test, previous stats reported as %O', previousResults)
+
+      this.intermediateStats = previousResults
+      this.pendingRunnable = runnable
+    })
+
+    return this.terminator.promise
+  }
+
+  exitEarly (error) {
+    if (error.isFatalApiErr) {
+      this.terminator.reject(error)
+
+      return
+    }
+
+    // eslint-disable-next-line no-console
+    console.log('')
+    errors.log(error)
+
+    const runResults: BaseReporterResults = (this.intermediateStats && this.pendingRunnable) ?
+      patchRunResultsAfterCrash(error, this.intermediateStats, this.pendingRunnable) :
+      defaultStats(error)
+
+    this.terminator.resolve(runResults)
+  }
+}