Skip to content

Commit

Permalink
feat: implement experimentalRetries strategies 'detect-flake-and-pass…
Browse files Browse the repository at this point in the history
…-on-threshold' and 'detect-flake-but-always-fail'. This should not be a breaking change, though it does modify mocha and the test object even when the experiment is not configured. This is to exercise the system and make sure things still work as expected even when we go GA. Test updates will follow in following commits.
  • Loading branch information
AtofStryker committed Sep 27, 2023
1 parent 386ff3d commit 8a4526a
Show file tree
Hide file tree
Showing 11 changed files with 508 additions and 38 deletions.
7 changes: 5 additions & 2 deletions packages/app/src/runner/event-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ let crossOriginOnMessageRef = ({ data, source }: MessageEvent<{
return undefined
}
let crossOriginLogs: {[key: string]: Cypress.Log} = {}
let hasMochaRunEnded: boolean = false

interface AddGlobalListenerOptions {
element: AutomationElementId
Expand Down Expand Up @@ -564,12 +565,14 @@ export class EventManager {
})

Cypress.on('run:start', async () => {
hasMochaRunEnded = false
if (Cypress.config('experimentalMemoryManagement') && Cypress.isBrowser({ family: 'chromium' })) {
await Cypress.backend('start:memory:profiling', Cypress.config('spec'))
}
})

Cypress.on('run:end', async () => {
hasMochaRunEnded = true
if (Cypress.config('experimentalMemoryManagement') && Cypress.isBrowser({ family: 'chromium' })) {
await Cypress.backend('end:memory:profiling')
}
Expand Down Expand Up @@ -720,8 +723,8 @@ export class EventManager {
Cypress.primaryOriginCommunicator.on('after:screenshot', handleAfterScreenshot)

Cypress.primaryOriginCommunicator.on('log:added', (attrs) => {
// If the test is over and the user enters interactive snapshot mode, do not add cross origin logs to the test runner.
if (Cypress.state('test')?.final) return
// If the mocha run is over and the user enters interactive snapshot mode, do not add cross origin logs to the test runner.
if (hasMochaRunEnded) return

// Create a new local log representation of the cross origin log.
// It will be attached to the current command.
Expand Down
86 changes: 86 additions & 0 deletions packages/driver/patches/mocha+7.0.1.dev.patch
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,89 @@ index 0b43004..588e195 100644
runner.checkLeaks = options.checkLeaks === true;
runner.fullStackTrace = options.fullTrace;
runner.asyncOnly = options.asyncOnly;
diff --git a/node_modules/mocha/lib/runner.js b/node_modules/mocha/lib/runner.js
index ceb1a24..d433661 100644
--- a/node_modules/mocha/lib/runner.js
+++ b/node_modules/mocha/lib/runner.js
@@ -677,9 +677,45 @@ Runner.prototype.runTests = function(suite, fn) {
}
self.emit(constants.EVENT_TEST_END, test);
return self.hookUp(HOOK_TYPE_AFTER_EACH, next);
- } else if (err) {
+ }
+ else if (err || test.hasAttemptPassed) {
+ if(test.hasAttemptPassed){
+ // Currently, to get passing attempts to rerun in mocha,
+ // we signal to mocha that we MIGHT need to retry a passed test attempt.
+ // If the test is run and there are no errors present, we assume a
+ // passed test attempt(set in ./driver/src/cypress/runner.ts)
+ test.state = STATE_PASSED
+ } else {
+ // Otherwise, we can assume the test attempt failed as 'err' would have to be present here.
+ test.state = STATE_FAILED
+ }
+
+ // Evaluate if the test should continue based on 'calculateTestStatus'.
+ // This is a custom method added by Cypress in ./driver/src/cypress/mocha.ts
+ var testStatusInfo = test.calculateTestStatus()
+
+ if(!testStatusInfo.shouldAttemptsContinue){
+ // If the test has met the exit condition, we need to grab the metadata from
+ // 'calculateTestStatus' in order to display and interpret the test outerStatus correctly.
+ test._cypressTestStatusInfo = testStatusInfo
+
+ if(testStatusInfo.attempts > 1) {
+ // If the test has been run AT LEAST twice (i.e. we are retrying), and the exit condition is met,
+ // modify mocha '_retries' to be the max retries made in order to possibly short circuit a suite
+ // if a hook has failed on every attempt (which we may not know at this stage of the test run).
+
+ // We will need the original retries to 'reset' the possible retries
+ // if the test attempt passes and fits the exit condition, BUT an 'afterEach' hook fails.
+ // In this case, we need to know how many retries we can reapply to satisfy the config.
+ test._maxRetries = test._retries
+ test._retries = test._currentRetry
+ }
+ }
+
var retry = test.currentRetry();
- if (retry < test.retries()) {
+
+ // requeue the test if we have retries and haven't satisfied our retry configuration.
+ if (retry < test.retries() && testStatusInfo.shouldAttemptsContinue) {
var clonedTest = test.clone();
clonedTest.currentRetry(retry + 1);
tests.unshift(clonedTest);
@@ -689,8 +725,26 @@ Runner.prototype.runTests = function(suite, fn) {
// Early return + hook trigger so that it doesn't
// increment the count wrong
return self.hookUp(HOOK_TYPE_AFTER_EACH, next);
- } else {
- self.fail(test, err);
+ } else if(testStatusInfo.outerStatus === STATE_FAILED) {
+ // However, if we have fit the exit condition and the outerStatus of the test attempt is marked as 'failed'.
+ if(test.state == STATE_PASSED){
+ // We might need to check the state of the last test attempt. In this case, if the strategy is "detect-flake-but-always-fail",
+ // has an outerStatus of 'failed', but the last test attempt passed, we still want to call the 'fail' hooks on the test, but keep
+ // the test attempt marked as passed.
+
+ // 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).
+ var lastTestWithErr = (test.prevAttempts || []).find(t => t.state === STATE_FAILED)
+ // TODO: figure out serialization with this looked up error as it isn't printed to the console reporter properly.
+ // Additionally, the OR statement here should be unreachable, but is intended for a fallback in case something is missed (especially under development).
+ err = lastTestWithErr?.err || new Error({ message: `expected test to have state "${testStatusInfo.outerStatus}" but was state "${test.state}".`})
+ }
+ self.fail(test, err)
+ } else if (testStatusInfo?.outerStatus === STATE_PASSED){
+ // There is no case where a test can 'pass' and the last test attempt be a failure,
+ // meaning we can assume a 'passed' outerStatus has a final passed test attempt.
+ self.emit(constants.EVENT_TEST_PASS, test);
}
self.emit(constants.EVENT_TEST_END, test);
return self.hookUp(HOOK_TYPE_AFTER_EACH, next);
@@ -1029,3 +1083,4 @@ Runner.constants = constants;
* @external EventEmitter
* @see {@link https://nodejs.org/api/events.html#events_class_eventemitter}
*/
+
11 changes: 10 additions & 1 deletion packages/driver/src/cypress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,16 @@ class $Cypress {
}

if (_.isObject(testRetries)) {
return testRetries[this.config('isInteractive') ? 'openMode' : 'runMode']
const retriesAsNumberOrBoolean = testRetries[this.config('isInteractive') ? 'openMode' : 'runMode']

// If experimentalRetries are configured, a experimentalStrategy is present, and the retries configured is a boolean
// then we need to set the mocha '_retries' to 'maxRetries' present in the 'experimentalOptions' configuration.
if (testRetries['experimentalStrategy'] && _.isBoolean(retriesAsNumberOrBoolean) && retriesAsNumberOrBoolean) {
return testRetries['experimentalOptions'].maxRetries
}

// Otherwise, this is a number and falls back to default
return retriesAsNumberOrBoolean
}

return null
Expand Down
103 changes: 103 additions & 0 deletions packages/driver/src/cypress/mocha.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,89 @@ 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
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')

// 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 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 remainingAttempts = maxAttempts - totalAttemptsAlreadyExecuted
const passingAttempts = passedTests.length

// Below variables are used for when strategy is "detect-flake-and-pass-on-threshold" or no strategy is defined
let passesRequired = strategy !== 'detect-flake-but-always-fail' ?
(options?.passesRequired || 1) :
null

const neededPassingAttemptsLeft = strategy !== 'detect-flake-but-always-fail' ?
passesRequired - passingAttempts :
null

// Below variables are used for when strategy is only "detect-flake-but-always-fail"
let stopIfAnyPassed = strategy === 'detect-flake-but-always-fail' ?
(options?.stopIfAnyPassed || false) :
null

// 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
shouldAttemptsContinue = false
} else if (totalAttemptsAlreadyExecuted < maxAttempts &&
(
// For strategy "detect-flake-and-pass-on-threshold" or no strategy (current GA retries):
// If we haven't met our max attempt limit AND we have enough remaining attempts that can satisfy the passing requirement.
// retry the test.
// @ts-expect-error
(strategy !== 'detect-flake-but-always-fail' && remainingAttempts >= neededPassingAttemptsLeft) ||
// For strategy "detect-flake-but-always-fail":
// If we haven't met our max attempt limit AND
// stopIfAnyPassed is false OR
// stopIfAnyPassed is true and no tests have passed yet.
// retry the test.
(strategy === 'detect-flake-but-always-fail' && (!stopIfAnyPassed || stopIfAnyPassed && passingAttempts === 0))
)) {
this.final = false
shouldAttemptsContinue = true
} else {
// Otherwise, we should stop retrying the test.
outerTestStatus = 'failed'
this.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
shouldAttemptsContinue = false
}
} else {
// retry logic did not need to be applied and the test passed.
outerTestStatus = 'passed'
shouldAttemptsContinue = false
this.final = true
}

return {
strategy,
shouldAttemptsContinue,
attempts: totalAttemptsAlreadyExecuted,
outerStatus: outerTestStatus,
}
}

type MochaArgs = [string, Function | undefined]
function createRunnable (ctx, fnType: 'Test' | 'Suite', mochaArgs: MochaArgs, runnableFn: Function, testCallback: Function | string = '', _testConfig?: Record<string, any>) {
const runnable = runnableFn.apply(ctx, mochaArgs)
Expand Down Expand Up @@ -221,6 +304,10 @@ const restoreTestClone = () => {
Test.prototype.clone = testClone
}

const removeCalculateTestStatus = () => {
delete Test.prototype.calculateTestStatus
}

const restoreRunnerRunTests = () => {
Runner.prototype.runTests = runnerRunTests
}
Expand Down Expand Up @@ -326,11 +413,25 @@ function patchTestClone () {
ret._testConfig = this._testConfig
ret.id = this.id
ret.order = this.order
ret._currentRetry = this._currentRetry

return ret
}
}

function createCalculateTestStatus (Cypress: Cypress.Cypress) {
// Adds a method to the test object called 'calculateTestStatus'
// which is used inside our mocha patch (./driver/patches/mocha+7.0.1.dev.patch)
// in order to calculate test retries. This prototype functions as a light abstraction around
// 'calculateTestStatus', which makes the function easier to unit-test
Test.prototype.calculateTestStatus = function () {
let retriesConfig = Cypress.config('retries')

// @ts-expect-error
return calculateTestStatus.call(this, retriesConfig?.experimentalStrategy, retriesConfig?.experimentalOptions)
}
}

function patchRunnerRunTests () {
Runner.prototype.runTests = function () {
const suite = arguments[0]
Expand Down Expand Up @@ -495,6 +596,7 @@ const restore = () => {
restoreHookRetries()
restoreRunnerRunTests()
restoreTestClone()
removeCalculateTestStatus()
restoreSuiteAddTest()
restoreSuiteAddSuite()
restoreSuiteHooks()
Expand All @@ -509,6 +611,7 @@ const override = (specWindow, Cypress, config) => {
patchHookRetries()
patchRunnerRunTests()
patchTestClone()
createCalculateTestStatus(Cypress)
patchSuiteAddTest(specWindow, config)
patchSuiteAddSuite(specWindow, config)
patchSuiteHooks(specWindow, config)
Expand Down
Loading

0 comments on commit 8a4526a

Please sign in to comment.