diff --git a/packages/cli/src/commands/pw-test.ts b/packages/cli/src/commands/pw-test.ts index 36481dee..b0514ecb 100644 --- a/packages/cli/src/commands/pw-test.ts +++ b/packages/cli/src/commands/pw-test.ts @@ -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' @@ -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.') diff --git a/packages/cli/src/commands/test.ts b/packages/cli/src/commands/test.ts index efbd69e6..e6847ab9 100644 --- a/packages/cli/src/commands/test.ts +++ b/packages/cli/src/commands/test.ts @@ -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' @@ -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 }) diff --git a/packages/cli/src/commands/trigger.ts b/packages/cli/src/commands/trigger.ts index f811f44e..7e7b7bed 100644 --- a/packages/cli/src/commands/trigger.ts +++ b/packages/cli/src/commands/trigger.ts @@ -12,6 +12,7 @@ import { Events, PrivateRunLocation, SequenceId, + CheckFailure, DEFAULT_CHECK_RUN_TIMEOUT_SECONDS, MAX_SCHEDULING_DELAY_SECONDS, } from '../services/abstract-check-runner.js' @@ -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 }) diff --git a/packages/cli/src/reporters/__tests__/abstract-list.spec.ts b/packages/cli/src/reporters/__tests__/abstract-list.spec.ts index e67cb5b3..85881b2d 100644 --- a/packages/cli/src/reporters/__tests__/abstract-list.spec.ts +++ b/packages/cli/src/reporters/__tests__/abstract-list.spec.ts @@ -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), diff --git a/packages/cli/src/reporters/__tests__/dot.spec.ts b/packages/cli/src/reporters/__tests__/dot.spec.ts index a62bccca..df0f1e58 100644 --- a/packages/cli/src/reporters/__tests__/dot.spec.ts +++ b/packages/cli/src/reporters/__tests__/dot.spec.ts @@ -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() diff --git a/packages/cli/src/reporters/__tests__/github-md-builder.spec.ts b/packages/cli/src/reporters/__tests__/github-md-builder.spec.ts index 4af4533e..fdf113cf 100644 --- a/packages/cli/src/reporters/__tests__/github-md-builder.spec.ts +++ b/packages/cli/src/reporters/__tests__/github-md-builder.spec.ts @@ -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 }, diff --git a/packages/cli/src/reporters/__tests__/json-builder.spec.ts b/packages/cli/src/reporters/__tests__/json-builder.spec.ts index 2fa2f779..258199e4 100644 --- a/packages/cli/src/reporters/__tests__/json-builder.spec.ts +++ b/packages/cli/src/reporters/__tests__/json-builder.spec.ts @@ -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({ diff --git a/packages/cli/src/reporters/__tests__/util.spec.ts b/packages/cli/src/reporters/__tests__/util.spec.ts index 89091d38..27e823f8 100644 --- a/packages/cli/src/reporters/__tests__/util.spec.ts +++ b/packages/cli/src/reporters/__tests__/util.spec.ts @@ -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', () => { diff --git a/packages/cli/src/reporters/abstract-list.ts b/packages/cli/src/reporters/abstract-list.ts index 993fd89d..ba7b0fc8 100644 --- a/packages/cli/src/reporters/abstract-list.ts +++ b/packages/cli/src/reporters/abstract-list.ts @@ -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 = [] @@ -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) { @@ -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, @@ -331,7 +334,9 @@ 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()) { @@ -339,6 +344,8 @@ export default abstract class AbstractListReporter implements Reporter { 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) { @@ -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, @@ -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) } } diff --git a/packages/cli/src/reporters/dot.ts b/packages/cli/src/reporters/dot.ts index bd42ca90..6c034f3f 100644 --- a/packages/cli/src/reporters/dot.ts +++ b/packages/cli/src/reporters/dot.ts @@ -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) diff --git a/packages/cli/src/reporters/github.ts b/packages/cli/src/reporters/github.ts index b7f59d7e..10d5fea4 100644 --- a/packages/cli/src/reporters/github.ts +++ b/packages/cli/src/reporters/github.ts @@ -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 = [ `${statusCell}${retries}`, `${result.name}`, diff --git a/packages/cli/src/reporters/json.ts b/packages/cli/src/reporters/json.ts index e4fc9205..ddcbc7b4 100644 --- a/packages/cli/src/reporters/json.ts +++ b/packages/cli/src/reporters/json.ts @@ -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, diff --git a/packages/cli/src/reporters/list.ts b/packages/cli/src/reporters/list.ts index 0aadba21..d048ee8f 100644 --- a/packages/cli/src/reporters/list.ts +++ b/packages/cli/src/reporters/list.ts @@ -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) { diff --git a/packages/cli/src/reporters/util.ts b/packages/cli/src/reporters/util.ts index 3c9d346b..552c7b15 100644 --- a/packages/cli/src/reporters/util.ts +++ b/packages/cli/src/reporters/util.ts @@ -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 { @@ -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 @@ -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 diff --git a/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts b/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts index 795d158e..74e5a2cc 100644 --- a/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts +++ b/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts @@ -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[] = [] diff --git a/packages/cli/src/services/abstract-check-runner.ts b/packages/cli/src/services/abstract-check-runner.ts index 3e7f1948..9789306a 100644 --- a/packages/cli/src/services/abstract-check-runner.ts +++ b/packages/cli/src/services/abstract-check-runner.ts @@ -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 @@ -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)) }