fix: Do not screenshot or trigger the failed event when tests are skipped (#19331)

Author: ryanthemanuel

packages/driver/cypress/integration/cy/skipping_async_spec.js

@@ -0,0 +1,62 @@
+const { Screenshot } = Cypress
+
+let failedEventFired = false
+
+Cypress.on('fail', (error) => {
+  failedEventFired = true
+  throw new Error(error)
+})
+
+let screenshotTaken = false
+
+Screenshot.defaults({ onAfterScreenshot: () => {
+  screenshotTaken = true
+} })
+
+const pendingTests = []
+const passedTests = []
+
+Cypress.on('test:after:run', (test) => {
+  if (test.state === 'pending') {
+    return pendingTests.push(test)
+  }
+
+  if (test.state === 'passed') {
+    return passedTests.push(test)
+  }
+})
+
+beforeEach(() => {
+  // Set isInteractive to false to ensure that screenshots will be
+  // triggered in both run and open mode
+  Cypress.config('isInteractive', false)
+})
+
+describe('skipped test', () => {
+  it('does not fail', function () {
+    cy.then(() => {
+      this.skip()
+    }).then(() => {
+      expect(true).to.be.false
+    })
+  })
+
+  it('does not prevent subsequent tests from running', () => {
+    expect(true).to.be.true
+  })
+})
+
+describe('skipped test side effects', () => {
+  it('does not have a screenshot taken', () => {
+    expect(screenshotTaken).to.be.false
+  })
+
+  it('does not fire failed event', () => {
+    expect(failedEventFired).to.be.false
+  })
+
+  it('does still mark all tests with the correct state', () => {
+    expect(pendingTests).to.have.length(1)
+    expect(passedTests).to.have.length(3)
+  })
+})

packages/driver/cypress/integration/cy/skipping_sync_spec.js

@@ -0,0 +1,70 @@
+const { Screenshot } = Cypress
+
+let failedEventFired = false
+
+Cypress.on('fail', (error) => {
+  failedEventFired = true
+  throw new Error(error)
+})
+
+let screenshotTaken = false
+
+Screenshot.defaults({ onAfterScreenshot: () => {
+  screenshotTaken = true
+} })
+
+const pendingTests = []
+const passedTests = []
+
+Cypress.on('test:after:run', (test) => {
+  if (test.state === 'pending') {
+    return pendingTests.push(test)
+  }
+
+  if (test.state === 'passed') {
+    return passedTests.push(test)
+  }
+})
+
+beforeEach(() => {
+  // Set isInteractive to false to ensure that screenshots will be
+  // triggered in both run and open mode
+  Cypress.config('isInteractive', false)
+})
+
+describe('generally skipped test', () => {
+  before(function () {
+    this.skip()
+  })
+
+  it('does not fail', function () {
+    expect(true).to.be.false
+  })
+})
+
+describe('individually skipped tests', () => {
+  it('does not fail when using this.skip', function () {
+    this.skip()
+    expect(true).to.be.false
+  })
+
+  // NOTE: We are skipping this test in order to test skip functionality
+  it.skip('does not fail when using it.skip', () => {
+    expect(true).to.be.false
+  })
+})
+
+describe('skipped test side effects', () => {
+  it('does not have a screenshot taken', () => {
+    expect(screenshotTaken).to.be.false
+  })
+
+  it('does not fire failed event', () => {
+    expect(failedEventFired).to.be.false
+  })
+
+  it('does still mark all tests with the correct state', () => {
+    expect(pendingTests).to.have.length(3)
+    expect(passedTests).to.have.length(2)
+  })
+})

packages/driver/src/cypress/command_queue.ts

@@ -343,6 +343,14 @@ export class CommandQueue extends Queue<Command> {
     }
 
     const onError = (err: Error | string) => {
+      // If the runnable was marked as pending, this test was skipped
+      // go ahead and just return
+      const runnable = this.state('runnable')
+
+      if (runnable.isPending()) {
+        return
+      }
+
       if (this.state('onCommandFailed')) {
         return this.state('onCommandFailed')(err, this, next)
       }

packages/driver/src/cypress/cy.ts

@@ -942,6 +942,12 @@ export class $Cy implements ITimeouts, IStability, IAssertions, IRetries, IJQuer
         // else just return ret
         return ret
       } catch (err) {
+        // If the runnable was marked as pending, this test was skipped
+        // go ahead and just return
+        if (runnable.isPending()) {
+          return
+        }
+
         // if runnable.fn threw synchronously, then it didnt fail from
         // a cypress command, but we should still teardown and handle
         // the error