Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 39 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
ae25def
report correct number of failures if browser crashes in the middle of…
cacieprins Sep 5, 2023
8d67daa
report failure correctly when browser crashes during test
cacieprins Sep 6, 2023
494aa67
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 8, 2023
5b46d7e
refactor crash handling
cacieprins Sep 11, 2023
f3a2bee
correct exit option check, clean up debug
cacieprins Sep 11, 2023
2de2a38
exit on success if exit option !== false
cacieprins Sep 11, 2023
ddd5c86
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 11, 2023
44d71f1
use default stats when reporter stats are unavailable
cacieprins Sep 12, 2023
983bab3
fix error messaging
cacieprins Sep 13, 2023
93f7e8a
move reporter types to an intermediate .ts file, to be combined with …
cacieprins Sep 13, 2023
93e7d46
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 13, 2023
a117dbf
debug tab close test in ci
cacieprins Sep 14, 2023
c9c5ebb
move debug env from pkg to ci yml
cacieprins Sep 14, 2023
ef74379
set debug env in spec
cacieprins Sep 14, 2023
e259a69
fix pckg
cacieprins Sep 14, 2023
0896f7f
adds some logging to cri-client
cacieprins Sep 14, 2023
b8be7fd
remove event emit logging from project-base
cacieprins Sep 14, 2023
8ddeb41
revert snapshot for tab close system test
cacieprins Sep 14, 2023
2bb5df4
fixes console output for no exit on success
cacieprins Sep 15, 2023
ec7d362
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 15, 2023
f6cf2cd
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 18, 2023
e69f9bd
changelog
cacieprins Sep 18, 2023
bc44078
changelog wsp
cacieprins Sep 18, 2023
9dd282e
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 19, 2023
47f03d4
cleanup
cacieprins Sep 19, 2023
c40c982
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 19, 2023
dd4f1f7
clean up tests
cacieprins Sep 19, 2023
aace680
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 20, 2023
0c09b1d
refactor to more straightforward control flow
cacieprins Sep 20, 2023
29b0020
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 20, 2023
8da0a05
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 21, 2023
f7e145f
rm export for unused type
cacieprins Sep 21, 2023
65cbc13
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 21, 2023
c688ca6
correct tab close snapshot for ci
cacieprins Sep 21, 2023
7a802ec
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Sep 22, 2023
a043e40
new system test for mid-test config crash
cacieprins Oct 4, 2023
8ffb34c
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Oct 5, 2023
9bf8025
update snapshots
cacieprins Oct 5, 2023
edbad0e
Merge branch 'develop' into cacie/fix/report-results-on-crash
cacieprins Oct 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ _Released 09/19/2023 (PENDING)_

- Introduces new layout for Runs page providing additional run information. Addresses [#27203](https://github.com/cypress-io/cypress/issues/27203).

**Bugfixes:**

- 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.2.0

_Released 09/12/2023_
Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/modes/results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ interface ScreenshotInformation {
width: pixels
}

interface RunResult {
export interface RunResult {
error: string | null
hooks: HookInformation[]
reporter: string
Expand Down
93 changes: 40 additions & 53 deletions packages/server/lib/modes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


type SetScreenshotMetadata = (data: TakeScreenshotProps) => void
type ScreenshotMetadata = ReturnType<typeof screenshotMetadata>
Expand All @@ -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}/`, '')
Expand Down Expand Up @@ -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)
})
})
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
8 changes: 6 additions & 2 deletions packages/server/lib/project-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ export class ProjectBase extends EE {
},

onMocha: async (event, runnable) => {
debug('onMocha', event)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this debug too spammy?

// bail if we dont have a
// reporter instance
if (!reporterInstance) {
Expand All @@ -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(),
Expand Down
48 changes: 48 additions & 0 deletions packages/server/lib/types/reporter.ts
Original file line number Diff line number Diff line change
@@ -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[]
}
115 changes: 115 additions & 0 deletions packages/server/lib/util/graceful_crash_handling.ts
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading