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 27 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.
Jump to
Jump to file
Failed to load files.
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
70 changes: 16 additions & 54 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 { endAfterError, exitEarly } from '../util/graceful_crash_handling'

type SetScreenshotMetadata = (data: TakeScreenshotProps) => void
type ScreenshotMetadata = ReturnType<typeof screenshotMetadata>
Expand All @@ -36,16 +37,9 @@ 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

const relativeSpecPattern = (projectRoot, pattern) => {
Expand Down Expand Up @@ -411,54 +405,22 @@ function launchBrowser (options: { browser: Browser, spec: SpecWithRelativeRoot,
return openProject.launch(browser, spec, browserOpts)
}

function listenForProjectEnd (project, exit): Bluebird<any> {
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 = () => {
console.log('not exiting due to options.exit being false')
}
}

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)
}
})
return Promise.race([
new Promise((resolve) => {
project.once('end', (results) => {
debug('project ended with results %O', results)
if (exit === false) {
console.log('not exiting due to options.exit being false')
} else {
resolve(results)
}
})
}),
endAfterError(project, exit),
])
}

async function waitForBrowserToConnect (options: { project: Project, socketId: string, onError: (err: Error) => void, spec: SpecWithRelativeRoot, isFirstSpecInBrowser: boolean, testingType: string, experimentalSingleTabRunMode: boolean, browser: Browser, screenshots: ScreenshotMetadata[], projectRoot: string, shouldLaunchNewTab: boolean, webSecurity: boolean, videoRecording?: VideoRecording, protocolManager?: ProtocolManager }) {
Expand Down
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[]
}
132 changes: 132 additions & 0 deletions packages/server/lib/util/graceful_crash_handling.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import type { ProjectBase } from '../project-base'
import type { BaseReporterResults, ReporterResults } from '../types/reporter'
import * as errors from '../errors'
import Debug from 'debug'

const debug = Debug('cypress:util:crash_handling')

let earlyExitError: Error

let earlyExit = (err: Error) => {
debug('set early exit error: %s', err.stack)

earlyExitError = err
}

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 const endAfterError = (project: ProjectBase, exit: boolean): Promise<any> => {
let pendingRunnable: any
let intermediateStats: ReporterResults

project.on('test:before:run', ({
runnable,
previousResults,
}) => {
debug('preparing to run test, previous stats reported as %O', previousResults)

intermediateStats = previousResults
pendingRunnable = runnable
})

return new Promise((resolve, reject) => {
const patchedResolve = exit === false ? () => {
// eslint-disable-next-line no-console
console.log('not exiting due to options.exit being false')
} : resolve

const handleEarlyExit = (error: Error & { isFatalApiErr?: boolean }) => {
if (error.isFatalApiErr) {
debug('handling fatal api error', error)
reject(error)
} else {
debug('patching results and resolving')
// eslint-disable-next-line no-console
console.log('')
errors.log(error)
const results = (intermediateStats && pendingRunnable) ?
patchRunResultsAfterCrash(error, intermediateStats, pendingRunnable) :
defaultStats(error)

debug('resolving with patched results %O', results)
patchedResolve(results)
}
}

earlyExit = (error) => {
debug('handling early exit with error', error)
handleEarlyExit(error)
}

if (earlyExitError) {
handleEarlyExit(earlyExitError)
}
})
}

export const exitEarly = (error) => {
cacieprins marked this conversation as resolved.
Show resolved Hide resolved
debug('exit early called', error)

return earlyExit(error)
}