Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 1 deletion packages/cli/src/commands/pw-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
Events,
MAX_SCHEDULING_DELAY_SECONDS,
SequenceId,
CheckFailure,
} from '../services/abstract-check-runner.js'
import { TestResultsShortLinks } from '../rest/test-sessions.js'
import commonMessages from '../messages/common-messages.js'
Expand Down Expand Up @@ -361,13 +362,14 @@ export default class PwTestCommand extends AuthCommand {
}, testResultId, links))
})

runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string) => {
runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string, failure?: CheckFailure) => {
reporters.forEach(r => r.onCheckEnd(sequenceId, {
...check,
logicalId: check.logicalId,
sourceFile: check.getSourceFile(),
hasFailures: true,
runError: message,
timedOut: failure?.timedOut ?? false,
}))
if (message.includes('No tests found')) {
this.style.shortInfo('Check your Playwright configuration to ensure it targets your test files.')
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { parseProject } from '../services/project-parser.js'
import {
Events,
SequenceId,
CheckFailure,
DEFAULT_CHECK_RUN_TIMEOUT_SECONDS,
MAX_SCHEDULING_DELAY_SECONDS,
} from '../services/abstract-check-runner.js'
Expand Down Expand Up @@ -433,13 +434,14 @@ export default class Test extends AuthCommand {
}, testResultId, links))
})

runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string) => {
runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string, failure?: CheckFailure) => {
reporters.forEach(r => r.onCheckEnd(sequenceId, {
...check,
logicalId: check.logicalId,
sourceFile: check.getSourceFile(),
hasFailures: true,
runError: message,
timedOut: failure?.timedOut ?? false,
}))
process.exitCode = 1
})
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Events,
PrivateRunLocation,
SequenceId,
CheckFailure,
DEFAULT_CHECK_RUN_TIMEOUT_SECONDS,
MAX_SCHEDULING_DELAY_SECONDS,
} from '../services/abstract-check-runner.js'
Expand Down Expand Up @@ -210,11 +211,12 @@ export default class Trigger extends AuthCommand {
}
reporters.forEach(r => r.onCheckEnd(sequenceId, result, testResultId, links))
})
runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string) => {
runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string, failure?: CheckFailure) => {
reporters.forEach(r => r.onCheckEnd(sequenceId, {
...check,
hasFailures: true,
runError: message,
timedOut: failure?.timedOut ?? false,
}))
process.exitCode = 1
})
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/reporters/__tests__/abstract-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,28 @@ describe('AbstractListReporter', () => {
expect(countOccurrences(output, `Open session: ${TEST_SESSION_URL}`)).toBe(1)
})

it('counts and titles a check the CLI stopped waiting for as timed out, not failed', () => {
withTty(process.stdout, false, () => {
const reporter = new ListReporter(PUBLIC_RUN_LOCATION, false)
const slow = { name: 'Slow Check', getSourceFile: () => SOURCE_FILE }
const broken = { name: 'Broken Check', getSourceFile: () => SOURCE_FILE }
reporter.onBegin([
{ check: slow, sequenceId: 'seq-slow' },
{ check: broken, sequenceId: 'seq-broken' },
])
reporter.onCheckEnd('seq-slow', {
...makePassingResult(), name: 'Slow Check', hasFailures: true, runError: 'Reached timeout', timedOut: true,
})
reporter.onCheckEnd('seq-broken', { ...makePassingResult(), name: 'Broken Check', hasFailures: true })
printLnMock.mockClear()
reporter._printSummary()

const summary = outputOf(printLnMock)
expect(summary).toContain('⏱ Slow Check')
expect(summary).toContain('1 failed, 1 timed out, 2 total')
})
})

describe('after the run stopped at the max failures limit', () => {
function makeStoppedReporter (
reporter = new ListReporter(PUBLIC_RUN_LOCATION, false),
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/reporters/__tests__/dot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,20 @@ describe('DotReporter', () => {
expect(output).toContain('My Playwright Check')
})

it('marks and summarizes a check the CLI stopped waiting for as timed out', () => {
const { reporter } = makeReporterWithOneCheck()

reporter.onCheckEnd(SEQUENCE_ID, makeResult({ hasFailures: true, runError: RUN_ERROR, timedOut: true }))
reporter.onEnd()

expect(marks()).toBe('T')
const output = printedOutput()
expect(output).toContain('1 timed out, 1 total')
expect(output).not.toContain('failed')
expect(output).toContain('⏱ My Playwright Check')
expect(output).toContain(RUN_ERROR)
})

it('should not print execution error details for a passing check', () => {
const { reporter } = makeReporterWithOneCheck()

Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/reporters/__tests__/github-md-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,28 @@ describe('GithubMdBuilder', () => {
expect(markdown).toContain('## Execution Errors')
expect(markdown).toContain(runError)
})
test('labels a check the CLI stopped waiting for as timed out, not failed', () => {
const row = (name: string, timedOut: boolean) => [`seq-${name}`, {
result: { name, sourceFile: 'folder/timeout.check.ts', hasFailures: true, runError: 'Error', timedOut },
titleString: name,
numRetries: 1,
}] as const
const map: checkFilesMap = new Map([
['folder/timeout.check.ts', new Map([
row('Slow check', true),
row('Crashed check', false),
])],
])
const markdown = new GithubMdBuilder({
testSessionId,
numChecks: 2,
runLocation,
checkFilesMap: map,
}).render()
expect(markdown).toContain('|⏱️ Timed out (1 retry)|Slow check|')
expect(markdown).toContain('|❌ Fail (1 retry)|Crashed check|')
expect(markdown).toContain('## Execution Errors')
})
test('shows the retry count in the status cell of a check that needed retries', () => {
const row = (name: string, hasFailures: boolean, numRetries: number) => [`seq-${name}`, {
result: { name, sourceFile: 'folder/retry.check.ts', hasFailures, checkType: 'API', responseTime: 10 },
Expand Down
29 changes: 29 additions & 0 deletions packages/cli/src/reporters/__tests__/json-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,35 @@ describe('JsonBuilder', () => {
})
})

test('reports a check the CLI stopped waiting for as timed out', () => {
const runError = 'Reached timeout of 600 seconds waiting for check result.'
const map: checkFilesMap = new Map([
['folder/timeout.check.ts', new Map([
['seq-timeout', {
result: { name: 'Slow check', sourceFile: 'folder/timeout.check.ts', hasFailures: true, runError, timedOut: true },
titleString: 'Slow check',
numRetries: 0,
}],
['seq-crashed', {
result: { name: 'Crashed check', sourceFile: 'folder/timeout.check.ts', hasFailures: true, runError: 'Error' },
titleString: 'Crashed check',
numRetries: 0,
}],
])],
])
const summary = JSON.parse(new JsonBuilder({
testSessionId: undefined,
numChecks: 2,
runLocation,
checkFilesMap: map,
}).render())

expect(summary.checks.map((c: any) => [c.name, c.result, c.runError])).toEqual([
['Slow check', 'Timed out', runError],
['Crashed check', 'Fail', 'Error'],
])
})

test('omits stoppedEarly when the run finished', () => {
const checkFilesMap = generateMapAndTestResultIds({ includeTestResultIds: false })
const summary = JSON.parse(new JsonBuilder({
Expand Down
15 changes: 15 additions & 0 deletions packages/cli/src/reporters/__tests__/util.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,21 @@ describe('resultToCheckStatus()', () => {
expect(resultToCheckStatus({ isCancelled: true, hasFailures: true }))
.toBe(CheckStatus.CANCELLED)
})
it('returns timed out for a check the CLI stopped waiting for', () => {
expect(resultToCheckStatus({ hasFailures: true, runError: 'Reached timeout', timedOut: true }))
.toBe(CheckStatus.TIMED_OUT)
})
it('returns failed for a run error that is not a timeout', () => {
expect(resultToCheckStatus({ hasFailures: true, runError: 'runner crashed', timedOut: false }))
.toBe(CheckStatus.FAILED)
})
})

describe('formatCheckTitle() with TIMED_OUT status', () => {
it('should use the ⏱ symbol for a timed-out check title', () => {
const result = stripAnsi(formatCheckTitle(CheckStatus.TIMED_OUT, simpleCheckFixture))
expect(result).toContain('⏱')
})
})

describe('formatCheckTitle() with CANCELLED status', () => {
Expand Down
14 changes: 11 additions & 3 deletions packages/cli/src/reporters/abstract-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export default abstract class AbstractListReporter implements Reporter {
} = {}) {
const write = opts.stream === 'stderr' ? printLnErr : printLn
const counts = {
numFailed: 0, numPassed: 0, numDegraded: 0, numFlaky: 0,
numFailed: 0, numTimedOut: 0, numPassed: 0, numDegraded: 0, numFlaky: 0,
numRunning: 0, numRetrying: 0, scheduling: 0, numCancelled: 0, numNotFinished: 0,
}
const status = []
Expand All @@ -249,6 +249,8 @@ export default abstract class AbstractListReporter implements Reporter {
counts.numRetrying++
} else if (!result) {
counts.numRunning++
} else if (checkStatus === CheckStatus.TIMED_OUT) {
counts.numTimedOut++
} else if (result.hasFailures) {
counts.numFailed++
} else if (result.isDegraded) {
Expand Down Expand Up @@ -302,6 +304,7 @@ export default abstract class AbstractListReporter implements Reporter {
counts.numRunning ? chalk.bold.magenta(`${counts.numRunning} running`) : undefined,
counts.numRetrying ? chalk.bold(`${counts.numRetrying} retrying`) : undefined,
counts.numFailed ? chalk.bold.red(`${counts.numFailed} failed`) : undefined,
counts.numTimedOut ? chalk.bold.red(`${counts.numTimedOut} timed out`) : undefined,
counts.numDegraded ? chalk.bold.yellow(`${counts.numDegraded} degraded`) : undefined,
counts.numPassed ? chalk.bold.green(`${counts.numPassed} passed`) : undefined,
counts.numFlaky ? chalk.bold.yellow(`${counts.numFlaky} flaky`) : undefined,
Expand Down Expand Up @@ -331,14 +334,18 @@ export default abstract class AbstractListReporter implements Reporter {

/** `lineEnded`: the reporter already ended its last line, so no empty line opens the summary. */
_printBriefSummary ({ lineEnded = false }: { lineEnded?: boolean } = {}) {
const counts = { numFailed: 0, numDegraded: 0, numPassed: 0, numPending: 0, numCancelled: 0, numNotFinished: 0 }
const counts = {
numFailed: 0, numTimedOut: 0, numDegraded: 0, numPassed: 0, numPending: 0, numCancelled: 0, numNotFinished: 0,
}
const status = []
for (const [, checkMap] of this.checkFilesMap!.entries()) {
for (const [, { result, checkStatus }] of checkMap.entries()) {
if (checkStatus === CheckStatus.NOT_FINISHED) {
counts.numNotFinished++
} else if (!result) {
counts.numPending++
} else if (checkStatus === CheckStatus.TIMED_OUT) {
counts.numTimedOut++
} else if (result.hasFailures) {
counts.numFailed++
} else if (result.isDegraded) {
Expand All @@ -355,6 +362,7 @@ export default abstract class AbstractListReporter implements Reporter {
}
status.push([
counts.numFailed ? chalk.bold.red(`${counts.numFailed} failed`) : undefined,
counts.numTimedOut ? chalk.bold.red(`${counts.numTimedOut} timed out`) : undefined,
counts.numDegraded ? chalk.bold.yellow(`${counts.numDegraded} degraded`) : undefined,
counts.numPassed ? chalk.bold.green(`${counts.numPassed} passed`) : undefined,
counts.numCancelled ? chalk.bold.grey(`${counts.numCancelled} cancelled`) : undefined,
Expand All @@ -379,7 +387,7 @@ export default abstract class AbstractListReporter implements Reporter {
for (const [, checkMap] of this.checkFilesMap!.entries()) {
for (const [, { result }] of checkMap.entries()) {
if (result?.runError) {
printLn(formatCheckTitle(CheckStatus.FAILED, result))
printLn(formatCheckTitle(resultToCheckStatus(result), result))
printLn(indentString(formatCheckResult(result), 4), 2, 1)
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/reporters/dot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export default class DotReporter extends AbstractListReporter {
onCheckEnd (sequenceId: SequenceId, checkResult: any) {
super.onCheckEnd(sequenceId, checkResult)
this.#printedMarks = true
if (checkResult.hasFailures) {
if (checkResult.timedOut) {
printLnErr(`${chalk.red('T')}`, 0)
} else if (checkResult.hasFailures) {
printLnErr(`${chalk.red('F')}`, 0)
} else if (checkResult.isDegraded) {
printLnErr(`${chalk.yellow('D')}`, 0)
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/reporters/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ export class GithubMdBuilder {
continue
}
const checkStatus = resultToCheckStatus(result)
const statusCell = checkStatus === CheckStatus.FAILED ? '❌ Fail' : checkStatus === CheckStatus.DEGRADED ? '⚠️ Degraded' : '✅ Pass'
const statusCell = checkStatus === CheckStatus.TIMED_OUT
? '⏱️ Timed out'
: checkStatus === CheckStatus.FAILED ? '❌ Fail' : checkStatus === CheckStatus.DEGRADED ? '⚠️ Degraded' : '✅ Pass'
const tableRow: Array<string> = [
`${statusCell}${retries}`,
`${result.name}`,
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/reporters/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ export class JsonBuilder {
}
const checkStatus = resultToCheckStatus(result)
const check: any = {
result: checkStatus === CheckStatus.FAILED ? 'Fail' : checkStatus === CheckStatus.DEGRADED ? 'Degraded' : 'Pass',
result: checkStatus === CheckStatus.TIMED_OUT
? 'Timed out'
: checkStatus === CheckStatus.FAILED ? 'Fail' : checkStatus === CheckStatus.DEGRADED ? 'Degraded' : 'Pass',
name: result.name,
checkType: result.checkType,
durationMilliseconds: result.responseTime ?? null,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/reporters/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default class ListReporter extends AbstractListReporter {
printLn(indentString(formatCheckResult(checkResult), 4), 2, 1)
} else {
if (checkResult.hasFailures) {
printLn(formatCheckTitle(CheckStatus.FAILED, checkResult))
printLn(formatCheckTitle(resultToCheckStatus(checkResult), checkResult))
printLn(indentString(formatCheckResult(checkResult), 4), 2, 1)
}
if (checkResult.isDegraded) {
Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/reporters/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export enum CheckStatus {
CANCELLED,
// The run stopped early (max failures) before this check reported a result.
NOT_FINISHED,
// The CLI stopped waiting for a result; the check may still have passed in Checkly.
TIMED_OUT,
}

export function formatDuration (ms: number): string {
Expand Down Expand Up @@ -70,6 +72,9 @@ export function formatCheckTitle (
} else if (status === CheckStatus.NOT_FINISHED) {
statusString = '…'
format = chalk.bold.dim
} else if (status === CheckStatus.TIMED_OUT) {
statusString = '⏱'
format = chalk.bold.red
} else {
statusString = '-'
format = chalk.bold.dim
Expand Down Expand Up @@ -776,6 +781,9 @@ export function resultToCheckStatus (checkResult: any): CheckStatus {
if (checkResult.isCancelled) {
return CheckStatus.CANCELLED
}
if (checkResult.timedOut) {
return CheckStatus.TIMED_OUT
}
return checkResult.hasFailures
? CheckStatus.FAILED
: checkResult.isDegraded
Expand Down
17 changes: 17 additions & 0 deletions packages/cli/src/services/__tests__/abstract-check-runner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,23 @@ describe('AbstractCheckRunner — per-check timeouts vs scheduling delay', () =>
;(runner as any).disableAllTimeouts()
})

it('flags a timeout as timed out, unlike other run errors', async () => {
const runner = new StubCheckRunner('acc-1', TIMEOUT_SECONDS, false)
runner.checks = new Map([['seq-1', { check: {} }], ['seq-2', { check: {} }]])
const failures: Array<{ sequenceId: string, failure?: { timedOut?: boolean } }> = []
runner.on(Events.CHECK_FAILED, (sequenceId, _check, _message, failure) => failures.push({ sequenceId, failure }))
;(runner as any).setAllTimeouts()

await (runner as any).processMessage('seq-2', 'error', 'runner crashed')
await vi.advanceTimersByTimeAsync((TIMEOUT_SECONDS + MAX_SCHEDULING_DELAY_SECONDS) * 1000)

expect(failures).toEqual([
{ sequenceId: 'seq-2', failure: undefined },
{ sequenceId: 'seq-1', failure: { timedOut: true } },
])
;(runner as any).disableAllTimeouts()
})

it('a final result before the timeout produces no failure', async () => {
const { runner, failures } = makeArmedRunner()
const successes: string[] = []
Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/services/abstract-check-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ export type RunLocation = PublicRunLocation | PrivateRunLocation
export type CheckRunId = string
export type SequenceId = string

// Optional details emitted with CHECK_FAILED alongside the error message.
export type CheckFailure = {
// The CLI stopped waiting for a result; the check may still have passed in Checkly.
timedOut?: boolean
}

export const DEFAULT_CHECK_RUN_TIMEOUT_SECONDS = 600
export const DEFAULT_PLAYWRIGHT_CHECK_RUN_TIMEOUT_SECONDS = 1200

Expand Down Expand Up @@ -493,7 +499,7 @@ export default abstract class AbstractCheckRunner extends EventEmitter {
} else if (!started || this.timeout === DEFAULT_CHECK_RUN_TIMEOUT_SECONDS) {
errorMessage += ' Checkly may be experiencing problems. Please check https://is.checkly.online or reach out to support@checklyhq.com.'
}
this.emit(Events.CHECK_FAILED, sequenceId, check, errorMessage)
this.emit(Events.CHECK_FAILED, sequenceId, check, errorMessage, { timedOut: true })
this.emit(Events.CHECK_FINISHED, check)
}, checkTimeout * 1000))
}
Expand Down
Loading