feat: enhance test error UI (#24266)

packages/driver/cypress/e2e/commands/sessions/sessions.cy.js

@@ -585,7 +585,6 @@ describe('cy.session', { retries: 0 }, () => {
         })
 
         expect(logs[6].get()).to.deep.contain({
-          showError: true,
           group: validateSessionGroup.id,
         })
 
@@ -679,7 +678,6 @@ describe('cy.session', { retries: 0 }, () => {
           })
 
           expect(logs[6].get()).to.deep.contain({
-            showError: true,
             group: validateSessionGroup.id,
           })
 

packages/driver/src/cy/commands/sessions/index.ts

@@ -236,8 +236,8 @@ export default function (Commands, Cypress, cy) {
 
               // show validation error and allow sessions workflow to recreate the session
               if (restoreSession) {
+                err.isRecovered = true
                 Cypress.log({
-                  showError: true,
                   type: 'system',
                   name: 'session',
                 })

packages/driver/src/cypress/error_utils.ts

@@ -9,7 +9,7 @@ import $stackUtils, { StackAndCodeFrameIndex } from './stack_utils'
 import $utils from './utils'
 import type { HandlerType } from './runner'
 
-const ERROR_PROPS = ['message', 'type', 'name', 'stack', 'parsedStack', 'fileName', 'lineNumber', 'columnNumber', 'host', 'uncaught', 'actual', 'expected', 'showDiff', 'isPending', 'docsUrl', 'codeFrame'] as const
+const ERROR_PROPS = ['message', 'type', 'name', 'stack', 'parsedStack', 'fileName', 'lineNumber', 'columnNumber', 'host', 'uncaught', 'actual', 'expected', 'showDiff', 'isPending', 'isRecovered', 'docsUrl', 'codeFrame'] as const
 const ERR_PREPARED_FOR_SERIALIZATION = Symbol('ERR_PREPARED_FOR_SERIALIZATION')
 
 const crossOriginScriptRe = /^script error/i

packages/driver/src/cypress/log.ts

@@ -15,7 +15,7 @@ import type { StateFunc } from './state'
 const groupsOrTableRe = /^(groups|table)$/
 const parentOrChildRe = /parent|child|system/
 const SNAPSHOT_PROPS = 'id snapshots $el url coords highlightAttr scrollBy viewportWidth viewportHeight'.split(' ')
-const DISPLAY_PROPS = 'id alias aliasType callCount displayName end err event functionName groupLevel hookId instrument isStubbed group message method name numElements showError numResponses referencesAlias renderProps sessionInfo state testId timeout type url visible wallClockStartedAt testCurrentRetry'.split(' ')
+const DISPLAY_PROPS = 'id alias aliasType callCount displayName end err event functionName groupLevel hookId instrument isStubbed group message method name numElements numResponses referencesAlias renderProps sessionInfo state testId timeout type url visible wallClockStartedAt testCurrentRetry'.split(' ')
 const BLACKLIST_PROPS = 'snapshots'.split(' ')
 
 let counter = 0
@@ -230,16 +230,16 @@ const defaults = function (state: StateFunc, config, obj) {
 }
 
 export class Log {
-  cy: any
+  createSnapshot: Function
   state: StateFunc
   config: any
   fireChangeEvent: ((log) => (void | undefined))
   obj: any
 
   private attributes: Record<string, any> = {}
 
-  constructor (cy, state, config, fireChangeEvent, obj) {
-    this.cy = cy
+  constructor (createSnapshot, state, config, fireChangeEvent, obj) {
+    this.createSnapshot = createSnapshot
     this.state = state
     this.config = config
     // only fire the log:state:changed event as fast as every 4ms
@@ -391,7 +391,7 @@ export class Log {
       }
     }
 
-    const snapshot = this.cy.createSnapshot(name, this.get('$el'))
+    const snapshot = this.createSnapshot(name, this.get('$el'))
 
     this.addSnapshot(snapshot, options)
 
@@ -608,7 +608,7 @@ class LogManager {
         $errUtils.throwErrByPath('log.invalid_argument', { args: { arg: options } })
       }
 
-      const log = new Log(cy, state, config, this.fireChangeEvent, options)
+      const log = new Log(cy.createSnapshot, state, config, this.fireChangeEvent, options)
 
       log.set(options)
 

packages/reporter/cypress/e2e/commands.cy.ts

@@ -926,24 +926,30 @@ describe('commands', { viewportHeight: 1000 }, () => {
 
   context('test error', () => {
     // this is a unique error permutation currently only observed in cy.session() where an error
-    // message should be presented during the session validation of a saves/restored session and the
-    // session command will attempt to recreate a valid session.
-    it('renders error instead of command', () => {
-      // font-family: $font-system;
+    // message should be presented if the session validation fails for a restored session because the
+    // session command recover and attempt to recreate a valid session.
+    it('renders recovered error for command', () => {
       cy.fixture('command_error').then((_commandErr) => {
+        const groupId = addCommand(runner, {
+          name: 'session',
+          message: 'mock restore',
+          state: 'passed',
+          type: 'system',
+        })
+
         addCommand(runner, {
-          id: 10,
-          number: 7,
+          type: 'system',
           name: 'validate',
-          displayMessage: 'mock session validation',
           state: 'failed',
-          showError: true,
-          err: _commandErr,
-          type: 'parent',
+          err: {
+            ..._commandErr,
+            isRecovered: true,
+          },
+          groupLevel: 1,
+          group: groupId,
         })
 
         addCommand(runner, {
-          id: 11,
           name: 'recreate session',
           message: 'mock recreate session cmd',
           state: 'success',
@@ -955,6 +961,58 @@ describe('commands', { viewportHeight: 1000 }, () => {
       cy.contains('recreate session')
       cy.percySnapshot()
     })
+
+    it('renders recovered error for nested group command', () => {
+      cy.fixture('command_error').then((_commandErr) => {
+        const groupId = addCommand(runner, {
+          name: 'session',
+          message: 'mock restore',
+          state: 'passed',
+          type: 'system',
+        })
+
+        const nested = addCommand(runner, {
+          type: 'system',
+          name: 'validate',
+          state: 'failed',
+          group: groupId,
+        })
+
+        addCommand(runner, {
+          number: 8,
+          name: 'get',
+          message: 'does_not_exist',
+          state: 'failed',
+          err: {
+            ..._commandErr,
+            isRecovered: true,
+          },
+          type: 'parent',
+          groupLevel: 2,
+          group: nested,
+        })
+
+        addCommand(runner, {
+          id: 12,
+          name: 'recreate session',
+          message: 'mock recreate session cmd',
+          state: 'success',
+          type: 'parent',
+        })
+      })
+
+      cy.contains('.command', 'validate').as('validate')
+      .find('.command-expander')
+      .should('have.class', 'command-expander-is-open')
+
+      cy.get('@validate').within(() => {
+        cy.contains('CommandError')
+      })
+
+      cy.contains('recreate session')
+
+      cy.percySnapshot()
+    })
   })
 
   context('studio commands', () => {

packages/reporter/cypress/e2e/test_errors.cy.ts

@@ -227,12 +227,13 @@ describe('test errors', () => {
 
     // NOTE: still needs to be implemented
     it.skip('renders and escapes markdown with leading/trailing whitespace', () => {
+      setError(commandErr)
       cy.get('.runnable-err-message')
 
       // https://github.com/cypress-io/cypress/issues/1360
       // renders ** buzz ** as <strong> buzz </strong>
-      .contains('code', 'foo')
-      .and('not.contain', '`foo`')
+      .contains('strong', 'buzz')
+      .and('not.contain', '** buzz **')
     })
   })
 

packages/reporter/cypress/e2e/unit/err_model.cy.ts

@@ -69,6 +69,12 @@ describe('Err model', () => {
       expect(err.stack).to.equal('the stack (path/to/file.js 45:203)')
     })
 
+    it('updates isRecovered if specified', () => {
+      expect(err.isRecovered).to.be.false
+      err.update({ isRecovered: true })
+      expect(err.isRecovered).to.be.true
+    })
+
     it('does nothing if props is undefined', () => {
       err.update()
       expect(err.name).to.equal('BadError')

packages/reporter/cypress/e2e/unit/events.cy.ts

@@ -306,22 +306,19 @@ describe('events', () => {
       expect(runner.emit).to.have.been.calledWith('runner:console:log', 'command id')
     })
 
-    it('emits runner:console:error with test id on show:error', () => {
-      const test = { err: { isCommandErr: false } }
+    it('emits runner:console:error with error on show:error', () => {
+      const errorDetails = { err: { stack: 'hi' } }
 
-      runnablesStore.testById.returns(test)
-      events.emit('show:error', test)
+      events.emit('show:error', errorDetails)
       expect(runner.emit).to.have.been.calledWith('runner:console:error', {
-        err: test.err,
+        err: errorDetails.err,
         testId: undefined,
         logId: undefined,
       })
     })
 
-    it('emits runner:console:error with test id and command id on show:error when it is a command error and there is a matching command', () => {
-      const test = { err: { isCommandErr: true }, commandMatchingErr: () => {
-        return { id: 'matching command id', testId: 'test' }
-      } }
+    it('emits runner:console:error with error, test id and command id on show:error ', () => {
+      const test = { err: { isCommandErr: true }, commandId: 'matching command id', testId: 'test' }
 
       runnablesStore.testById.returns(test)
       events.emit('show:error', test)
@@ -332,20 +329,6 @@ describe('events', () => {
       })
     })
 
-    it('emits runner:console:error with test id on show:error when it is a command error but there not a matching command', () => {
-      const test = { err: { isCommandErr: true }, commandMatchingErr: () => {
-        return null
-      } }
-
-      runnablesStore.testById.returns(test)
-      events.emit('show:error', test)
-      expect(runner.emit).to.have.been.calledWith('runner:console:error', {
-        err: test.err,
-        testId: undefined,
-        logId: undefined,
-      })
-    })
-
     it('emits runner:show:snapshot on show:snapshot', () => {
       events.emit('show:snapshot', 'command id')
       expect(runner.emit).to.have.been.calledWith('runner:show:snapshot', 'command id')

packages/reporter/src/attempts/attempt-model.ts

@@ -17,7 +17,7 @@ export default class Attempt {
   @observable agents: Agent[] = []
   @observable sessions: Record<string, Session> = {}
   @observable commands: Command[] = []
-  @observable err = new Err({})
+  @observable err?: Err = undefined
   @observable hooks: Hook[] = []
   // TODO: make this an enum with states: 'QUEUED, ACTIVE, INACTIVE'
   @observable isActive: boolean | null = null
@@ -49,7 +49,10 @@ export default class Attempt {
     this.id = props.currentRetry || 0
     this.test = test
     this._state = props.state
-    this.err.update(props.err)
+
+    if (props.err) {
+      this.err = new Err(props.err)
+    }
 
     this.invocationDetails = props.invocationDetails
 
@@ -78,6 +81,16 @@ export default class Attempt {
     return this._state || (this.isActive ? 'active' : 'processing')
   }
 
+  @computed get error () {
+    const command = this.err?.isCommandErr ? this.commandMatchingErr() : undefined
+
+    return {
+      err: this.err,
+      testId: command?.testId,
+      commandId: command?.id,
+    }
+  }
+
   @computed get isLast () {
     return this.id === this.test.lastAttempt.id
   }
@@ -137,9 +150,14 @@ export default class Attempt {
     }
   }
 
-  commandMatchingErr () {
+  commandMatchingErr (): Command | undefined {
+    if (!this.err) {
+      return undefined
+    }
+
     return _(this.hooks)
     .map((hook) => {
+      // @ts-ignore
       return hook.commandMatchingErr(this.err)
     })
     .compact()
@@ -155,7 +173,13 @@ export default class Attempt {
       this._state = props.state
     }
 
-    this.err.update(props.err)
+    if (props.err) {
+      if (this.err) {
+        this.err.update(props.err)
+      } else {
+        this.err = new Err(props.err)
+      }
+    }
 
     if (props.failedFromHookId) {
       const hook = _.find(this.hooks, { hookId: props.failedFromHookId })

packages/reporter/src/attempts/attempts.tsx

@@ -55,10 +55,12 @@ function renderAttemptContent (model: AttemptModel, studioActive: boolean) {
       <div ref='commands' className='runnable-commands-region'>
         {model.hasCommands ? <Hooks model={model} /> : <NoCommands />}
       </div>
-      <div className='attempt-error-region'>
-        <TestError model={model} />
-        {studioActive && model.state === 'failed' && <StudioError />}
-      </div>
+      {model.state === 'failed' && (
+        <div className='attempt-error-region'>
+          <TestError {...model.error} />
+          {studioActive && <StudioError />}
+        </div>
+      )}
     </div>
   )
 }