chore: address comments from code review

packages/driver/cypress/e2e/util/mocha_custom_methods.cy.js

@@ -21,7 +21,7 @@ describe('mocha custom methods', () => {
     describe('strategy w/ experimentalBurnIn=false', () => {
       it('should never attempt to retry a test that passes on the first try, regardless of strategy', function () {
         const undefinedStrategyTest = createMockTest()
-        const noExperimentalRetries = calculateTestStatus.call(undefinedStrategyTest)
+        const noExperimentalRetries = calculateTestStatus(undefinedStrategyTest)
 
         expect(noExperimentalRetries.outerStatus).to.equal('passed')
         expect(noExperimentalRetries.attempts).to.equal(1)
@@ -30,7 +30,7 @@ describe('mocha custom methods', () => {
         expect(undefinedStrategyTest.final).to.be.true
 
         const detectFlakeAndPassOnThresholdStrategyTest = createMockTest()
-        const detectFlakeAndPassOnThreshold = calculateTestStatus.call(detectFlakeAndPassOnThresholdStrategyTest, 'detect-flake-and-pass-on-threshold', {
+        const detectFlakeAndPassOnThreshold = calculateTestStatus(detectFlakeAndPassOnThresholdStrategyTest, 'detect-flake-and-pass-on-threshold', {
           maxRetries: 8,
           passesRequired: 5,
         })
@@ -42,7 +42,7 @@ describe('mocha custom methods', () => {
         expect(detectFlakeAndPassOnThresholdStrategyTest.final).to.be.true
 
         const detectFlakeButAlwaysFailStrategyTest = createMockTest()
-        const detectFlakeButAlwaysFail = calculateTestStatus.call(detectFlakeButAlwaysFailStrategyTest, 'detect-flake-but-always-fail', {
+        const detectFlakeButAlwaysFail = calculateTestStatus(detectFlakeButAlwaysFailStrategyTest, 'detect-flake-but-always-fail', {
           maxRetries: 8,
           stopIfAnyPassed: false,
         })
@@ -57,15 +57,15 @@ describe('mocha custom methods', () => {
       describe('undefined (GA implementation/original)', () => {
         it('passed: keeps signaling to retry until test passes', function () {
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1)
+          const attempt1 = calculateTestStatus(mockTest1)
 
           expect(attempt1.outerStatus).to.be.undefined
           expect(attempt1.attempts).to.equal(1)
           expect(attempt1.shouldAttemptsContinue).to.be.true
           expect(attempt1.strategy).to.be.undefined
 
           const mockTest2 = createMockTest('passed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2)
+          const attempt2 = calculateTestStatus(mockTest2)
 
           expect(attempt2.outerStatus).to.equal('passed')
           expect(attempt2.attempts).to.equal(2)
@@ -77,23 +77,23 @@ describe('mocha custom methods', () => {
         // this happens inside ./driver/src/cypress/runner.ts
         it('failed: keeps signaling to retry until retry limit is reached', function () {
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1)
+          const attempt1 = calculateTestStatus(mockTest1)
 
           expect(attempt1.outerStatus).to.be.undefined
           expect(attempt1.attempts).to.equal(1)
           expect(attempt1.shouldAttemptsContinue).to.be.true
           expect(attempt1.strategy).to.be.undefined
 
           const mockTest2 = createMockTest('failed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2)
+          const attempt2 = calculateTestStatus(mockTest2)
 
           expect(attempt2.outerStatus).to.be.undefined
           expect(attempt2.attempts).to.equal(2)
           expect(attempt2.shouldAttemptsContinue).to.be.true
           expect(attempt2.strategy).to.be.undefined
 
           const mockTest3 = createMockTest('failed', [mockTest1, mockTest2])
-          const attempt3 = calculateTestStatus.call(mockTest3)
+          const attempt3 = calculateTestStatus(mockTest3)
 
           expect(attempt3.outerStatus).to.equal('failed')
           expect(attempt3.attempts).to.equal(3)
@@ -106,7 +106,7 @@ describe('mocha custom methods', () => {
         it('passed: no longer signals to retry test after passesRequired threshold is reached', function () {
           totalRetries = 5
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1, 'detect-flake-and-pass-on-threshold', {
+          const attempt1 = calculateTestStatus(mockTest1, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -118,7 +118,7 @@ describe('mocha custom methods', () => {
           expect(mockTest1.final).to.be.false
 
           const mockTest2 = createMockTest('failed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2, 'detect-flake-and-pass-on-threshold', {
+          const attempt2 = calculateTestStatus(mockTest2, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -130,7 +130,7 @@ describe('mocha custom methods', () => {
           expect(mockTest2.final).to.be.false
 
           const mockTest3 = createMockTest('passed', [mockTest1, mockTest2])
-          const attempt3 = calculateTestStatus.call(mockTest3, 'detect-flake-and-pass-on-threshold', {
+          const attempt3 = calculateTestStatus(mockTest3, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -142,7 +142,7 @@ describe('mocha custom methods', () => {
           expect(mockTest3.final).to.be.false
 
           const mockTest4 = createMockTest('passed', [mockTest1, mockTest2, mockTest3])
-          const attempt4 = calculateTestStatus.call(mockTest4, 'detect-flake-and-pass-on-threshold', {
+          const attempt4 = calculateTestStatus(mockTest4, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -157,7 +157,7 @@ describe('mocha custom methods', () => {
         it('failed: no longer signals to retry test if the passesRequired is impossible to meet', function () {
           totalRetries = 4
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1, 'detect-flake-and-pass-on-threshold', {
+          const attempt1 = calculateTestStatus(mockTest1, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -169,7 +169,7 @@ describe('mocha custom methods', () => {
           expect(mockTest1.final).to.be.false
 
           const mockTest2 = createMockTest('failed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2, 'detect-flake-and-pass-on-threshold', {
+          const attempt2 = calculateTestStatus(mockTest2, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -181,7 +181,7 @@ describe('mocha custom methods', () => {
           expect(mockTest2.final).to.be.false
 
           const mockTest3 = createMockTest('failed', [mockTest1, mockTest2])
-          const attempt3 = calculateTestStatus.call(mockTest3, 'detect-flake-and-pass-on-threshold', {
+          const attempt3 = calculateTestStatus(mockTest3, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -193,7 +193,7 @@ describe('mocha custom methods', () => {
           expect(mockTest3.final).to.be.false
 
           const mockTest4 = createMockTest('failed', [mockTest1, mockTest2, mockTest3])
-          const attempt4 = calculateTestStatus.call(mockTest4, 'detect-flake-and-pass-on-threshold', {
+          const attempt4 = calculateTestStatus(mockTest4, 'detect-flake-and-pass-on-threshold', {
             maxRetries: totalRetries,
             passesRequired: 2,
           })
@@ -210,7 +210,7 @@ describe('mocha custom methods', () => {
         it('failed: no longer signals to retry after retries are exhausted', function () {
           totalRetries = 3
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1, 'detect-flake-but-always-fail', {
+          const attempt1 = calculateTestStatus(mockTest1, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: false,
           })
@@ -222,7 +222,7 @@ describe('mocha custom methods', () => {
           expect(mockTest1.final).to.be.false
 
           const mockTest2 = createMockTest('failed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2, 'detect-flake-but-always-fail', {
+          const attempt2 = calculateTestStatus(mockTest2, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: false,
           })
@@ -234,7 +234,7 @@ describe('mocha custom methods', () => {
           expect(mockTest2.final).to.be.false
 
           const mockTest3 = createMockTest('passed', [mockTest1, mockTest2])
-          const attempt3 = calculateTestStatus.call(mockTest3, 'detect-flake-but-always-fail', {
+          const attempt3 = calculateTestStatus(mockTest3, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: false,
           })
@@ -246,7 +246,7 @@ describe('mocha custom methods', () => {
           expect(mockTest3.final).to.be.false
 
           const mockTest4 = createMockTest('passed', [mockTest1, mockTest2, mockTest3])
-          const attempt4 = calculateTestStatus.call(mockTest4, 'detect-flake-but-always-fail', {
+          const attempt4 = calculateTestStatus(mockTest4, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: false,
           })
@@ -263,7 +263,7 @@ describe('mocha custom methods', () => {
         it('failed: short circuits after a retry has a passed test', function () {
           totalRetries = 3
           const mockTest1 = createMockTest('failed')
-          const attempt1 = calculateTestStatus.call(mockTest1, 'detect-flake-but-always-fail', {
+          const attempt1 = calculateTestStatus(mockTest1, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: true,
           })
@@ -275,7 +275,7 @@ describe('mocha custom methods', () => {
           expect(mockTest1.final).to.be.false
 
           const mockTest2 = createMockTest('passed', [mockTest1])
-          const attempt2 = calculateTestStatus.call(mockTest2, 'detect-flake-but-always-fail', {
+          const attempt2 = calculateTestStatus(mockTest2, 'detect-flake-but-always-fail', {
             maxRetries: totalRetries,
             stopIfAnyPassed: true,
           })

packages/driver/src/cypress/mocha.ts

@@ -38,26 +38,29 @@ delete (window as any).Mocha
 export const SKIPPED_DUE_TO_BROWSER_MESSAGE = ' (skipped due to browser)'
 
 // NOTE: 'calculateTestStatus' is marked as an individual function to make functionality easier to test.
-export const calculateTestStatus = function (strategy?: 'detect-flake-and-pass-on-threshold' | 'detect-flake-but-always-fail', options?: any) {
-  const totalAttemptsAlreadyExecuted = this.currentRetry() + 1
+export function calculateTestStatus (test: Mocha.Test, strategy?: 'detect-flake-and-pass-on-threshold' | 'detect-flake-but-always-fail', options?: any) {
+  // @ts-expect-error
+  const totalAttemptsAlreadyExecuted = test.currentRetry() + 1
   let shouldAttemptsContinue: boolean = true
   let outerTestStatus: 'passed' | 'failed' | undefined = undefined
 
-  const passedTests = _.filter(this.prevAttempts, (o) => o.state === 'passed')
-  const failedTests = _.filter(this.prevAttempts, (o) => o.state === 'failed')
+  // @ts-expect-error
+  const passedTests = _.filter(test.prevAttempts, (o) => o.state === 'passed')
+  // @ts-expect-error
+  const failedTests = _.filter(test.prevAttempts, (o) => o.state === 'failed')
 
   // Additionally, if the current test attempt passed/failed, add it to the attempt list
-  if (this.state === 'passed') {
-    passedTests.push(this)
-  } else if (this.state === 'failed') {
-    failedTests.push(this)
+  if (test.state === 'passed') {
+    passedTests.push(test)
+  } else if (test.state === 'failed') {
+    failedTests.push(test)
   }
 
   // If there is AT LEAST one failed test attempt, we know we need to apply retry logic.
   // Otherwise, the test might be burning in (not implemented yet) OR the test passed on the first attempt,
   // meaning retry logic does NOT need to be applied.
   if (failedTests.length > 0) {
-    const maxAttempts = this.retries() + 1
+    const maxAttempts = test.retries() + 1
     const remainingAttempts = maxAttempts - totalAttemptsAlreadyExecuted
     const passingAttempts = passedTests.length
 
@@ -78,7 +81,8 @@ export const calculateTestStatus = function (strategy?: 'detect-flake-and-pass-o
     // Do we have the required amount of passes? If yes, we no longer need to keep running the test.
     if (strategy !== 'detect-flake-but-always-fail' && passingAttempts >= passesRequired) {
       outerTestStatus = 'passed'
-      this.final = true
+      // @ts-expect-error
+      test.final = true
       shouldAttemptsContinue = false
     } else if (totalAttemptsAlreadyExecuted < maxAttempts &&
       (
@@ -94,22 +98,26 @@ export const calculateTestStatus = function (strategy?: 'detect-flake-and-pass-o
         // retry the test.
         (strategy === 'detect-flake-but-always-fail' && (!stopIfAnyPassed || stopIfAnyPassed && passingAttempts === 0))
       )) {
-      this.final = false
+      // @ts-expect-error
+      test.final = false
       shouldAttemptsContinue = true
     } else {
       // Otherwise, we should stop retrying the test.
       outerTestStatus = 'failed'
-      this.final = true
+      // @ts-expect-error
+      test.final = true
       // If an outerStatus is 'failed', but the last test attempt was 'passed', we need to force the status so mocha doesn't flag the test attempt as failed.
       // This is a common use case with 'detect-flake-but-always-fail', where we want to display the last attempt as 'passed' but fail the test.
-      this.forceState = this.state === 'passed' ? this.state : undefined
+      // @ts-expect-error
+      test.forceState = test.state === 'passed' ? test.state : undefined
       shouldAttemptsContinue = false
     }
   } else {
     // retry logic did not need to be applied and the test passed.
     outerTestStatus = 'passed'
     shouldAttemptsContinue = false
-    this.final = true
+    // @ts-expect-error
+    test.final = true
   }
 
   return {
@@ -428,7 +436,7 @@ function createCalculateTestStatus (Cypress: Cypress.Cypress) {
     let retriesConfig = Cypress.config('retries')
 
     // @ts-expect-error
-    return calculateTestStatus.call(this, retriesConfig?.experimentalStrategy, retriesConfig?.experimentalOptions)
+    return calculateTestStatus(this, retriesConfig?.experimentalStrategy, retriesConfig?.experimentalOptions)
   }
 }
 

packages/driver/src/cypress/runner.ts

@@ -482,16 +482,16 @@ const overrideRunnerHook = (Cypress, _runner, getTestById, getTest, setTest, get
           test.final = true
           if (test.state === 'passed') {
             if (test?._cypressTestStatusInfo?.outerStatus === 'failed') {
-            // We call _runner.fail here instead of in mocha because we need to make sure none of the hooks mutate the current test state, which might trigger
-            // another attempt. This affects our server reporter by reporting the test final status multiple times and incorrect attempt statuses.
-            // We can be sure here that the test is settled and we can fail it appropriately if the condition is met.
+              // We call _runner.fail here instead of in mocha because we need to make sure none of the hooks mutate the current test state, which might trigger
+              // another attempt. This affects our server reporter by reporting the test final status multiple times and incorrect attempt statuses.
+              // We can be sure here that the test is settled and we can fail it appropriately if the condition is met.
 
               // In this case, since the last attempt of the test does not contain an error, we need to look one up from a previous attempt
               // and fail the last attempt with this error to appropriate the correct runner lifecycle hooks. However, we still want the
               // last attempt to be marked as 'passed'. This is where 'forceState' comes into play (see 'calculateTestStatus' in ./driver/src/cypress/mocha.ts)
 
               // If there are other hooks (such as multiple afterEach hooks) that MIGHT impact the end conditions of the test, we only want to fail this ONCE!
-              let lastTestWithErr = (test.prevAttempts || []).find((t) => t.state === 'failed')
+              const lastTestWithErr = (test.prevAttempts || []).find((t) => t.state === 'failed')
               // TODO: figure out serialization with this looked up error as it isn't printed to the console reporter properly.
               const err = lastTestWithErr?.err