From 469a09bfbb88dfb5e73786188f7659889c8911e2 Mon Sep 17 00:00:00 2001 From: Chris Lample Date: Sun, 28 Jul 2024 15:17:03 +0200 Subject: [PATCH] feat: add support for test retries [sc-20570] (#952) --- packages/cli/e2e/__tests__/deploy.spec.ts | 6 +- .../fixtures/retry-project/checkly.config.ts | 22 +++++++ .../src/__checks__/group.check.ts | 26 ++++++++ packages/cli/e2e/__tests__/test.spec.ts | 13 ++++ packages/cli/e2e/run-checkly.ts | 4 +- packages/cli/src/commands/test.ts | 56 +++++++++++++---- packages/cli/src/commands/trigger.ts | 55 +++++++++++++---- .../__snapshots__/json-builder.spec.ts.snap | 18 ++++-- .../__tests__/fixtures/api-check-result.ts | 1 + .../fixtures/browser-check-result.ts | 1 + .../cli/src/reporters/__tests__/helpers.ts | 23 ++++++- packages/cli/src/reporters/abstract-list.ts | 47 +++++++++----- packages/cli/src/reporters/ci.ts | 16 +++-- packages/cli/src/reporters/dot.ts | 8 +-- packages/cli/src/reporters/github.ts | 4 +- packages/cli/src/reporters/json.ts | 7 ++- packages/cli/src/reporters/list.ts | 34 +++++++++-- packages/cli/src/reporters/reporter.ts | 9 +-- packages/cli/src/reporters/util.ts | 12 +++- packages/cli/src/rest/test-sessions.ts | 2 + .../cli/src/services/abstract-check-runner.ts | 61 +++++++++++-------- .../cli/src/services/checkly-config-loader.ts | 1 + packages/cli/src/services/test-runner.ts | 27 ++++---- packages/cli/src/services/trigger-runner.ts | 25 ++++---- 24 files changed, 355 insertions(+), 123 deletions(-) create mode 100644 packages/cli/e2e/__tests__/fixtures/retry-project/checkly.config.ts create mode 100644 packages/cli/e2e/__tests__/fixtures/retry-project/src/__checks__/group.check.ts diff --git a/packages/cli/e2e/__tests__/deploy.spec.ts b/packages/cli/e2e/__tests__/deploy.spec.ts index 8db6ed57..7e6a66ba 100644 --- a/packages/cli/e2e/__tests__/deploy.spec.ts +++ b/packages/cli/e2e/__tests__/deploy.spec.ts @@ -125,7 +125,11 @@ describe('deploy', () => { apiKey: config.get('apiKey'), accountId: config.get('accountId'), directory: path.join(__dirname, 'fixtures', 'deploy-project'), - env: { PROJECT_LOGICAL_ID: projectLogicalId, PRIVATE_LOCATION_SLUG_NAME: privateLocationSlugname }, + env: { + PROJECT_LOGICAL_ID: projectLogicalId, + PRIVATE_LOCATION_SLUG_NAME: privateLocationSlugname, + CHECKLY_CLI_VERSION: undefined, + }, }) expect(stderr).toBe('') // expect the version to be overriden with latest from NPM diff --git a/packages/cli/e2e/__tests__/fixtures/retry-project/checkly.config.ts b/packages/cli/e2e/__tests__/fixtures/retry-project/checkly.config.ts new file mode 100644 index 00000000..7c13f0da --- /dev/null +++ b/packages/cli/e2e/__tests__/fixtures/retry-project/checkly.config.ts @@ -0,0 +1,22 @@ +import { defineConfig } from 'checkly' + +const config = defineConfig({ + projectName: 'Test Project', + logicalId: 'test-project', + repoUrl: 'https://github.com/checkly/checkly-cli', + checks: { + locations: ['us-east-1', 'eu-west-1'], + tags: ['mac'], + runtimeId: '2023.09', + checkMatch: '**/*.check.ts', + browserChecks: { + // using .test.ts suffix (no .spec.ts) to avoid '@playwright/test not found error' when Jest transpile the spec.ts + testMatch: '**/__checks__/*.test.ts', + }, + }, + cli: { + runLocation: 'us-east-1', + }, +}) + +export default config diff --git a/packages/cli/e2e/__tests__/fixtures/retry-project/src/__checks__/group.check.ts b/packages/cli/e2e/__tests__/fixtures/retry-project/src/__checks__/group.check.ts new file mode 100644 index 00000000..d445abbb --- /dev/null +++ b/packages/cli/e2e/__tests__/fixtures/retry-project/src/__checks__/group.check.ts @@ -0,0 +1,26 @@ +/* eslint-disable no-new */ +import { CheckGroup, BrowserCheck } from 'checkly/constructs' + +const group = new CheckGroup('check-group-1', { + name: 'Group', + activated: true, + muted: false, + locations: ['us-east-1', 'eu-west-1'], + tags: ['mac', 'group'], + environmentVariables: [], + apiCheckDefaults: {}, + alertChannels: [], + browserChecks: { + // using .test.ts suffix (no .spec.ts) to avoid '@playwright/test not found error' when Jest transpile the spec.ts + testMatch: '**/*.test.ts', + }, +}) + +new BrowserCheck('group-browser-check-1', { + name: 'Check with group', + activated: false, + groupId: group.ref(), + code: { + content: 'throw new Error("Failing Check Result")', + }, +}) diff --git a/packages/cli/e2e/__tests__/test.spec.ts b/packages/cli/e2e/__tests__/test.spec.ts index 93b565b4..f0d2d5be 100644 --- a/packages/cli/e2e/__tests__/test.spec.ts +++ b/packages/cli/e2e/__tests__/test.spec.ts @@ -191,4 +191,17 @@ describe('test', () => { fs.rmSync(snapshotDir, { recursive: true }) } }) + + it('Should execute retries', async () => { + const result = await runChecklyCli({ + args: ['test', '--retries=3'], + apiKey: config.get('apiKey'), + accountId: config.get('accountId'), + directory: path.join(__dirname, 'fixtures', 'retry-project'), + timeout: 120000, // 2 minutes + }) + // The failing check result will have "Failing Check Result" in the output. + // We expect the check to be run 4 times. + expect(result.stdout.match(/Failing Check Result/g)).toHaveLength(4) + }) }) diff --git a/packages/cli/e2e/run-checkly.ts b/packages/cli/e2e/run-checkly.ts index b663050d..d65bc012 100644 --- a/packages/cli/e2e/run-checkly.ts +++ b/packages/cli/e2e/run-checkly.ts @@ -33,7 +33,9 @@ export function runChecklyCli (options: { CHECKLY_API_KEY: apiKey, CHECKLY_ACCOUNT_ID: accountId, CHECKLY_ENV: process.env.CHECKLY_ENV, - CHECKLY_CLI_VERSION: cliVersion, + // We need the CLI to report 4.8.0 or greater in order for the backend to use the new MQTT topic format. + // Once 4.8.0 has been released, we can remove the 4.8.0 fallback here. + CHECKLY_CLI_VERSION: cliVersion ?? '4.8.0', CHECKLY_E2E_PROMPTS_INJECTIONS: promptsInjection?.length ? JSON.stringify(promptsInjection) : undefined, ...env, }, diff --git a/packages/cli/src/commands/test.ts b/packages/cli/src/commands/test.ts index 9806ee62..280283e2 100644 --- a/packages/cli/src/commands/test.ts +++ b/packages/cli/src/commands/test.ts @@ -8,7 +8,7 @@ import { Events, RunLocation, PrivateRunLocation, - CheckRunId, + SequenceId, DEFAULT_CHECK_RUN_TIMEOUT_SECONDS, } from '../services/abstract-check-runner' import TestRunner from '../services/test-runner' @@ -16,7 +16,7 @@ import { loadChecklyConfig } from '../services/checkly-config-loader' import { filterByFileNamePattern, filterByCheckNamePattern, filterByTags } from '../services/test-filters' import type { Runtime } from '../rest/runtimes' import { AuthCommand } from './authCommand' -import { BrowserCheck, Check, HeartbeatCheck, MultiStepCheck, Project, Session } from '../constructs' +import { BrowserCheck, Check, HeartbeatCheck, MultiStepCheck, Project, RetryStrategyBuilder, Session } from '../constructs' import type { Region } from '..' import { splitConfigFilePath, getGitInformation, getCiInformation, getEnvs } from '../services/util' import { createReporters, ReporterType } from '../reporters/reporter' @@ -26,6 +26,7 @@ import { printLn, formatCheckTitle, CheckStatus } from '../reporters/util' import { uploadSnapshots } from '../services/snapshot-service' const DEFAULT_REGION = 'eu-central-1' +const MAX_RETRIES = 3 export default class Test extends AuthCommand { static coreCommand = true @@ -100,6 +101,9 @@ export default class Test extends AuthCommand { description: 'Update any snapshots using the actual result of this test run.', default: false, }), + retries: Flags.integer({ + description: `[default: 0, max: ${MAX_RETRIES}] How many times to retry a failing test run.`, + }), } static args = { @@ -132,6 +136,7 @@ export default class Test extends AuthCommand { record: shouldRecord, 'test-session-name': testSessionName, 'update-snapshots': updateSnapshots, + retries, } = flags const filePatterns = argv as string[] @@ -228,6 +233,7 @@ export default class Test extends AuthCommand { const reporters = createReporters(reporterTypes, location, verbose) const repoInfo = getGitInformation(project.repoUrl) const ciInfo = getCiInformation() + const testRetryStrategy = this.prepareTestRetryStrategy(retries, checklyConfig?.cli?.retries) const runner = new TestRunner( config.getAccountId(), @@ -241,34 +247,45 @@ export default class Test extends AuthCommand { ciInfo.environment, updateSnapshots, configDirectory, + testRetryStrategy, ) runner.on(Events.RUN_STARTED, - (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId: string) => + (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId: string) => reporters.forEach(r => r.onBegin(checks, testSessionId)), ) - runner.on(Events.CHECK_INPROGRESS, (check: any, checkRunId: CheckRunId) => { - reporters.forEach(r => r.onCheckInProgress(check, checkRunId)) + runner.on(Events.CHECK_INPROGRESS, (check: any, sequenceId: SequenceId) => { + reporters.forEach(r => r.onCheckInProgress(check, sequenceId)) }) runner.on(Events.MAX_SCHEDULING_DELAY_EXCEEDED, () => { reporters.forEach(r => r.onSchedulingDelayExceeded()) }) - runner.on(Events.CHECK_SUCCESSFUL, (checkRunId, check, result, links?: TestResultsShortLinks) => { - if (result.hasFailures) { - process.exitCode = 1 - } - - reporters.forEach(r => r.onCheckEnd(checkRunId, { + runner.on(Events.CHECK_ATTEMPT_RESULT, (sequenceId: SequenceId, check, result, links?: TestResultsShortLinks) => { + reporters.forEach(r => r.onCheckAttemptResult(sequenceId, { logicalId: check.logicalId, sourceFile: check.getSourceFile(), ...result, }, links)) }) - runner.on(Events.CHECK_FAILED, (checkRunId, check, message: string) => { - reporters.forEach(r => r.onCheckEnd(checkRunId, { + + runner.on(Events.CHECK_SUCCESSFUL, + (sequenceId: SequenceId, check, result, testResultId, links?: TestResultsShortLinks) => { + if (result.hasFailures) { + process.exitCode = 1 + } + + reporters.forEach(r => r.onCheckEnd(sequenceId, { + logicalId: check.logicalId, + sourceFile: check.getSourceFile(), + ...result, + }, testResultId, links)) + }) + + runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string) => { + reporters.forEach(r => r.onCheckEnd(sequenceId, { ...check, logicalId: check.logicalId, sourceFile: check.getSourceFile(), @@ -337,6 +354,19 @@ export default class Test extends AuthCommand { } } + prepareTestRetryStrategy (retries?: number, configRetries?: number) { + const numRetries = retries ?? configRetries ?? 0 + if (numRetries > MAX_RETRIES) { + printLn(`Defaulting to the maximum of ${MAX_RETRIES} retries.`) + } + return numRetries + ? RetryStrategyBuilder.fixedStrategy({ + maxRetries: Math.min(numRetries, MAX_RETRIES), + baseBackoffSeconds: 0, + }) + : null + } + private listChecks (checks: Array) { // Sort and print the checks in a way that's consistent with AbstractListReporter const sortedCheckFiles = [...new Set(checks.map((check) => check.getSourceFile()))].sort() diff --git a/packages/cli/src/commands/trigger.ts b/packages/cli/src/commands/trigger.ts index 9af0eb2c..0315e7e1 100644 --- a/packages/cli/src/commands/trigger.ts +++ b/packages/cli/src/commands/trigger.ts @@ -7,13 +7,21 @@ import { loadChecklyConfig } from '../services/checkly-config-loader' import { splitConfigFilePath, getEnvs, getGitInformation, getCiInformation } from '../services/util' import type { Region } from '..' import TriggerRunner, { NoMatchingChecksError } from '../services/trigger-runner' -import { RunLocation, Events, PrivateRunLocation, CheckRunId, DEFAULT_CHECK_RUN_TIMEOUT_SECONDS } from '../services/abstract-check-runner' +import { + RunLocation, + Events, + PrivateRunLocation, + SequenceId, + DEFAULT_CHECK_RUN_TIMEOUT_SECONDS, +} from '../services/abstract-check-runner' import config from '../services/config' import { createReporters, ReporterType } from '../reporters/reporter' +import { printLn } from '../reporters/util' import { TestResultsShortLinks } from '../rest/test-sessions' -import { Session } from '../constructs' +import { Session, RetryStrategyBuilder } from '../constructs' const DEFAULT_REGION = 'eu-central-1' +const MAX_RETRIES = 3 export default class Trigger extends AuthCommand { static coreCommand = true @@ -74,6 +82,9 @@ export default class Trigger extends AuthCommand { char: 'n', description: 'A name to use when storing results in Checkly with --record.', }), + retries: Flags.integer({ + description: `[default: 0, max: ${MAX_RETRIES}] How many times to retry a check run.`, + }), } async run (): Promise { @@ -90,6 +101,7 @@ export default class Trigger extends AuthCommand { env, 'env-file': envFile, 'test-session-name': testSessionName, + retries, } = flags const envVars = await getEnvs(envFile, env) const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) @@ -106,6 +118,7 @@ export default class Trigger extends AuthCommand { const verbose = this.prepareVerboseFlag(verboseFlag, checklyConfig?.cli?.verbose) const reporterTypes = this.prepareReportersTypes(reporterFlag as ReporterType, checklyConfig?.cli?.reporters) const reporters = createReporters(reporterTypes, location, verbose) + const testRetryStrategy = this.prepareTestRetryStrategy(retries, checklyConfig?.cli?.retries) const repoInfo = getGitInformation() const ciInfo = getCiInformation() @@ -121,22 +134,27 @@ export default class Trigger extends AuthCommand { repoInfo, ciInfo.environment, testSessionName, + testRetryStrategy, ) // TODO: This is essentially the same for `checkly test`. Maybe reuse code. runner.on(Events.RUN_STARTED, - (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId: string) => + (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId: string) => reporters.forEach(r => r.onBegin(checks, testSessionId))) - runner.on(Events.CHECK_INPROGRESS, (check: any, checkRunId: CheckRunId) => { - reporters.forEach(r => r.onCheckInProgress(check, checkRunId)) + runner.on(Events.CHECK_INPROGRESS, (check: any, sequenceId: SequenceId) => { + reporters.forEach(r => r.onCheckInProgress(check, sequenceId)) }) - runner.on(Events.CHECK_SUCCESSFUL, (checkRunId, _, result, links?: TestResultsShortLinks) => { - if (result.hasFailures) { - process.exitCode = 1 - } - reporters.forEach(r => r.onCheckEnd(checkRunId, result, links)) + runner.on(Events.CHECK_ATTEMPT_RESULT, (sequenceId: SequenceId, check, result, links?: TestResultsShortLinks) => { + reporters.forEach(r => r.onCheckAttemptResult(sequenceId, result, links)) }) - runner.on(Events.CHECK_FAILED, (checkRunId, check, message: string) => { - reporters.forEach(r => r.onCheckEnd(checkRunId, { + runner.on(Events.CHECK_SUCCESSFUL, + (sequenceId: SequenceId, _, result, testResultId, links?: TestResultsShortLinks) => { + if (result.hasFailures) { + process.exitCode = 1 + } + reporters.forEach(r => r.onCheckEnd(sequenceId, result, testResultId, links)) + }) + runner.on(Events.CHECK_FAILED, (sequenceId: SequenceId, check, message: string) => { + reporters.forEach(r => r.onCheckEnd(sequenceId, { ...check, hasFailures: true, runError: message, @@ -209,4 +227,17 @@ export default class Trigger extends AuthCommand { } return reporterFlag ? [reporterFlag] : cliReporters } + + prepareTestRetryStrategy (retries?: number, configRetries?: number) { + const numRetries = retries ?? configRetries ?? 0 + if (numRetries > MAX_RETRIES) { + printLn(`Defaulting to the maximum of ${MAX_RETRIES} retries.`) + } + return numRetries + ? RetryStrategyBuilder.fixedStrategy({ + maxRetries: Math.min(numRetries, MAX_RETRIES), + baseBackoffSeconds: 0, + }) + : null + } } diff --git a/packages/cli/src/reporters/__tests__/__snapshots__/json-builder.spec.ts.snap b/packages/cli/src/reporters/__tests__/__snapshots__/json-builder.spec.ts.snap index e163922b..14e38bac 100644 --- a/packages/cli/src/reporters/__tests__/__snapshots__/json-builder.spec.ts.snap +++ b/packages/cli/src/reporters/__tests__/__snapshots__/json-builder.spec.ts.snap @@ -13,7 +13,8 @@ exports[`JsonBuilder renders JSON markdown output with assets & links: json-with "durationMilliseconds": 6522, "filename": "src/__checks__/folder/browser.check.ts", "link": "https://app.checklyhq.com/test-sessions/0c4c64b3-79c5-44a6-ae07-b580ce73f328/results/702961fd-7e2c-45f0-97be-1aa9eabd4d82", - "runError": "Run error" + "runError": "Run error", + "retries": 0 }, { "result": "Pass", @@ -22,7 +23,8 @@ exports[`JsonBuilder renders JSON markdown output with assets & links: json-with "durationMilliseconds": 1234, "filename": "src/some-other-folder/api.check.ts", "link": "https://app.checklyhq.com/test-sessions/0c4c64b3-79c5-44a6-ae07-b580ce73f328/results/1c0be612-a5ec-432e-ac1c-837d2f70c010", - "runError": "Run error" + "runError": "Run error", + "retries": 0 } ] }" @@ -40,7 +42,8 @@ exports[`JsonBuilder renders basic JSON output with no assets & links: json-basi "durationMilliseconds": 6522, "filename": "src/__checks__/folder/browser.check.ts", "link": null, - "runError": null + "runError": null, + "retries": 0 }, { "result": "Pass", @@ -49,7 +52,8 @@ exports[`JsonBuilder renders basic JSON output with no assets & links: json-basi "durationMilliseconds": 1234, "filename": "src/some-other-folder/api.check.ts", "link": null, - "runError": null + "runError": null, + "retries": 0 } ] }" @@ -67,7 +71,8 @@ exports[`JsonBuilder renders basic JSON output with run errors: json-basic 1`] = "durationMilliseconds": 6522, "filename": "src/__checks__/folder/browser.check.ts", "link": null, - "runError": "Run error" + "runError": "Run error", + "retries": 0 }, { "result": "Pass", @@ -76,7 +81,8 @@ exports[`JsonBuilder renders basic JSON output with run errors: json-basic 1`] = "durationMilliseconds": 1234, "filename": "src/some-other-folder/api.check.ts", "link": null, - "runError": "Run error" + "runError": "Run error", + "retries": 0 } ] }" diff --git a/packages/cli/src/reporters/__tests__/fixtures/api-check-result.ts b/packages/cli/src/reporters/__tests__/fixtures/api-check-result.ts index eb98ad30..ce665b4b 100644 --- a/packages/cli/src/reporters/__tests__/fixtures/api-check-result.ts +++ b/packages/cli/src/reporters/__tests__/fixtures/api-check-result.ts @@ -5,6 +5,7 @@ export const apiCheckResult = { sourceInfo: { checkRunId: '4f20dfa7-8c66-4a15-8c43-5dc24f6206c6', checkRunSuiteId: '6390a87e-89c7-4295-b6f8-b23e87922ef3', + sequenceId: '72c5d10f-fc68-4361-a779-8543575336ae', ephemeral: true, }, checkRunId: '1c0be612-a5ec-432e-ac1c-837d2f70c010', diff --git a/packages/cli/src/reporters/__tests__/fixtures/browser-check-result.ts b/packages/cli/src/reporters/__tests__/fixtures/browser-check-result.ts index e559374f..6ce2102e 100644 --- a/packages/cli/src/reporters/__tests__/fixtures/browser-check-result.ts +++ b/packages/cli/src/reporters/__tests__/fixtures/browser-check-result.ts @@ -4,6 +4,7 @@ export const browserCheckResult = { sourceInfo: { checkRunId: 'ebefe140-29bf-4650-9e89-6e36a5c092ef', checkRunSuiteId: '2b938869-e38b-4599-b3b0-901aeea4bbef', + sequenceId: 'd2ec2faa-a7de-4dc1-8b56-2f4ee9248dc6', ephemeral: true, }, checkRunId: '702961fd-7e2c-45f0-97be-1aa9eabd4d82', diff --git a/packages/cli/src/reporters/__tests__/helpers.ts b/packages/cli/src/reporters/__tests__/helpers.ts index cb01d704..2f078c87 100644 --- a/packages/cli/src/reporters/__tests__/helpers.ts +++ b/packages/cli/src/reporters/__tests__/helpers.ts @@ -1,6 +1,7 @@ import { checkFilesMap } from '../abstract-list' import { browserCheckResult } from './fixtures/browser-check-result' import { apiCheckResult } from './fixtures/api-check-result' +import { SequenceId } from '../../services/abstract-check-runner' export function generateMapAndTestResultIds ({ includeTestResultIds = true, includeRunErrors = false }) { if (includeRunErrors) { @@ -9,18 +10,36 @@ export function generateMapAndTestResultIds ({ includeTestResultIds = true, incl } const checkFilesMapFixture: checkFilesMap = new Map([ ['folder/browser.check.ts', new Map([ - [browserCheckResult.checkRunId, { + [browserCheckResult.sourceInfo.sequenceId as SequenceId, { result: browserCheckResult as any, titleString: browserCheckResult.name, // TODO: We shouldn't reuse the checkRunId as the testResultId in the test. This isn't how it actually works. testResultId: includeTestResultIds ? browserCheckResult.checkRunId : undefined, + numRetries: 0, + links: includeTestResultIds + ? { + testResultLink: 'https://app.checklyhq.com/test-sessions/0c4c64b3-79c5-44a6-ae07-b580ce73f328/results/702961fd-7e2c-45f0-97be-1aa9eabd4d82', + testTraceLinks: [], + videoLinks: [], + screenshotLinks: [], + } + : undefined, }], ])], ['folder/api.check.ts', new Map([ - [apiCheckResult.checkRunId, { + [apiCheckResult.sourceInfo.sequenceId as SequenceId, { result: apiCheckResult as any, titleString: apiCheckResult.name, testResultId: includeTestResultIds ? apiCheckResult.checkRunId : undefined, + numRetries: 0, + links: includeTestResultIds + ? { + testResultLink: 'https://app.checklyhq.com/test-sessions/0c4c64b3-79c5-44a6-ae07-b580ce73f328/results/1c0be612-a5ec-432e-ac1c-837d2f70c010', + testTraceLinks: [], + videoLinks: [], + screenshotLinks: [], + } + : undefined, }], ])], ]) diff --git a/packages/cli/src/reporters/abstract-list.ts b/packages/cli/src/reporters/abstract-list.ts index bca22d98..d93d2542 100644 --- a/packages/cli/src/reporters/abstract-list.ts +++ b/packages/cli/src/reporters/abstract-list.ts @@ -1,9 +1,10 @@ import chalk from 'chalk' import indentString from 'indent-string' +import { TestResultsShortLinks } from '../rest/test-sessions' import { Reporter } from './reporter' import { CheckStatus, formatCheckTitle, getTestSessionUrl, printLn } from './util' -import type { CheckRunId, RunLocation } from '../services/abstract-check-runner' +import type { SequenceId, RunLocation } from '../services/abstract-check-runner' import { Check } from '../constructs/check' import { testSessions } from '../rest/api' @@ -11,12 +12,14 @@ import { testSessions } from '../rest/api' // This lets us print a structured list of the checks. // Map remembers the original insertion order, so each time we print the summary will be consistent. // Note that in the case of `checkly trigger`, the file will be `undefined`! -export type checkFilesMap = Map> export default abstract class AbstractListReporter implements Reporter { @@ -36,7 +39,7 @@ export default abstract class AbstractListReporter implements Reporter { this.verbose = verbose } - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string): void { + onBegin (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string): void { this.testSessionId = testSessionId this.numChecks = checks.length // Sort the check files and checks alphabetically. This makes sure that there's a consistent order between runs. @@ -44,19 +47,23 @@ export default abstract class AbstractListReporter implements Reporter { const sortedCheckFiles = [...new Set(checks.map(({ check }) => check.getSourceFile?.()))].sort() const sortedChecks = checks.sort(({ check: a }, { check: b }) => a.name.localeCompare(b.name)) this.checkFilesMap = new Map(sortedCheckFiles.map((file) => [file, new Map()])) - sortedChecks.forEach(({ check, testResultId, checkRunId }) => { + sortedChecks.forEach(({ check, sequenceId }) => { const fileMap = this.checkFilesMap!.get(check.getSourceFile?.())! - fileMap.set(checkRunId, { + fileMap.set(sequenceId, { check, titleString: formatCheckTitle(CheckStatus.SCHEDULING, check), checkStatus: CheckStatus.SCHEDULING, - testResultId, + numRetries: 0, }) }) } - onCheckInProgress (check: any, checkRunId: CheckRunId) { - const checkFile = this.checkFilesMap!.get(check.getSourceFile?.())!.get(checkRunId)! + onCheckInProgress (check: any, sequenceId: SequenceId) { + const checkFile = this.checkFilesMap!.get(check.getSourceFile?.())!.get(sequenceId)! + if (checkFile.checkStatus !== CheckStatus.SCHEDULING) { + // For simplicity, if a check is retrying, we leave it in that state when the next retry attempt starts. + return + } checkFile.titleString = formatCheckTitle(CheckStatus.RUNNING, check) checkFile.checkStatus = CheckStatus.RUNNING } @@ -67,11 +74,20 @@ export default abstract class AbstractListReporter implements Reporter { this._isSchedulingDelayExceeded = true } - onCheckEnd (checkRunId: CheckRunId, checkResult: any) { - const checkStatus = this.checkFilesMap!.get(checkResult.sourceFile)!.get(checkRunId)! + onCheckAttemptResult (sequenceId: string, checkResult: any, links?: TestResultsShortLinks | undefined): void { + const checkFile = this.checkFilesMap!.get(checkResult.sourceFile)!.get(sequenceId)! + checkFile.checkStatus = CheckStatus.RETRIED + checkFile.titleString = formatCheckTitle(CheckStatus.RETRIED, checkResult) + checkFile.numRetries++ + } + + onCheckEnd (sequenceId: SequenceId, checkResult: any, testResultId?: string, links?: TestResultsShortLinks) { + const checkStatus = this.checkFilesMap!.get(checkResult.sourceFile)!.get(sequenceId)! checkStatus.result = checkResult - const status = checkResult.hasFailures ? CheckStatus.FAILED : CheckStatus.SUCCESSFUL - checkStatus.titleString = formatCheckTitle(status, checkResult, { + checkStatus.links = links + checkStatus.testResultId = testResultId + checkStatus.checkStatus = checkResult.hasFailures ? CheckStatus.FAILED : CheckStatus.SUCCESSFUL + checkStatus.titleString = formatCheckTitle(checkStatus.checkStatus, checkResult, { includeSourceFile: false, }) } @@ -88,7 +104,7 @@ export default abstract class AbstractListReporter implements Reporter { } _printSummary (opts: { skipCheckCount?: boolean} = {}) { - const counts = { numFailed: 0, numPassed: 0, numRunning: 0, scheduling: 0 } + const counts = { numFailed: 0, numPassed: 0, numRunning: 0, numRetrying: 0, scheduling: 0 } const status = [] if (this.checkFilesMap!.size === 1 && this.checkFilesMap!.has(undefined)) { status.push(chalk.bold('Summary:')) @@ -98,6 +114,8 @@ export default abstract class AbstractListReporter implements Reporter { for (const [_, { titleString, result, checkStatus }] of checkMap.entries()) { if (checkStatus === CheckStatus.SCHEDULING) { counts.scheduling++ + } else if (checkStatus === CheckStatus.RETRIED) { + counts.numRetrying++ } else if (!result) { counts.numRunning++ } else if (result.hasFailures) { @@ -114,6 +132,7 @@ export default abstract class AbstractListReporter implements Reporter { status.push([ counts.scheduling ? chalk.bold.blue(`${counts.scheduling} scheduling`) : undefined, 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.numPassed ? chalk.bold.green(`${counts.numPassed} passed`) : undefined, `${this.numChecks} total`, diff --git a/packages/cli/src/reporters/ci.ts b/packages/cli/src/reporters/ci.ts index a2cf6480..67f3185f 100644 --- a/packages/cli/src/reporters/ci.ts +++ b/packages/cli/src/reporters/ci.ts @@ -2,10 +2,11 @@ import indentString from 'indent-string' import AbstractListReporter from './abstract-list' import { formatCheckTitle, formatCheckResult, CheckStatus, printLn } from './util' -import { CheckRunId } from '../services/abstract-check-runner' +import { SequenceId } from '../services/abstract-check-runner' +import { TestResultsShortLinks } from '../rest/test-sessions' export default class CiReporter extends AbstractListReporter { - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string) { + onBegin (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string) { super.onBegin(checks, testSessionId) printLn(`Running ${this.numChecks} checks in ${this._runLocationString()}:`, 2, 1) this._printSummary({ skipCheckCount: true }) @@ -17,8 +18,15 @@ export default class CiReporter extends AbstractListReporter { this._printTestSessionsUrl() } - onCheckEnd (checkRunId: CheckRunId, checkResult: any) { - super.onCheckEnd(checkRunId, checkResult) + onCheckAttemptResult (sequenceId: string, checkResult: any, links?: TestResultsShortLinks): void { + super.onCheckAttemptResult(sequenceId, checkResult, links) + + printLn(formatCheckTitle(CheckStatus.RETRIED, checkResult, { printRetryDuration: true })) + printLn(indentString(formatCheckResult(checkResult), 4), 2, 1) + } + + onCheckEnd (sequenceId: SequenceId, checkResult: any) { + super.onCheckEnd(sequenceId, checkResult) printLn(formatCheckTitle(checkResult.hasFailures ? CheckStatus.FAILED : CheckStatus.SUCCESSFUL, checkResult)) if (this.verbose || checkResult.hasFailures) { diff --git a/packages/cli/src/reporters/dot.ts b/packages/cli/src/reporters/dot.ts index ecb89d22..63dcada4 100644 --- a/packages/cli/src/reporters/dot.ts +++ b/packages/cli/src/reporters/dot.ts @@ -1,10 +1,10 @@ import chalk from 'chalk' import AbstractListReporter from './abstract-list' -import { CheckRunId } from '../services/abstract-check-runner' +import { SequenceId } from '../services/abstract-check-runner' import { print, printLn } from './util' export default class DotReporter extends AbstractListReporter { - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string) { + onBegin (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string) { super.onBegin(checks, testSessionId) printLn(`Running ${this.numChecks} checks in ${this._runLocationString()}.`, 2, 1) } @@ -14,8 +14,8 @@ export default class DotReporter extends AbstractListReporter { this._printTestSessionsUrl() } - onCheckEnd (checkRunId: CheckRunId, checkResult: any) { - super.onCheckEnd(checkRunId, checkResult) + onCheckEnd (sequenceId: SequenceId, checkResult: any) { + super.onCheckEnd(sequenceId, checkResult) if (checkResult.hasFailures) { print(`${chalk.red('F')}`) } else { diff --git a/packages/cli/src/reporters/github.ts b/packages/cli/src/reporters/github.ts index c6083de3..c48fa1bd 100644 --- a/packages/cli/src/reporters/github.ts +++ b/packages/cli/src/reporters/github.ts @@ -2,7 +2,7 @@ import * as fs from 'fs' import * as path from 'path' import AbstractListReporter, { checkFilesMap } from './abstract-list' -import { CheckRunId } from '../services/abstract-check-runner' +import { SequenceId } from '../services/abstract-check-runner' import { formatDuration, printLn, getTestSessionUrl } from './util' const outputFile = './checkly-github-report.md' @@ -96,7 +96,7 @@ export class GithubMdBuilder { } export default class GithubReporter extends AbstractListReporter { - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string) { + onBegin (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string) { super.onBegin(checks, testSessionId) printLn(`Running ${this.numChecks} checks in ${this._runLocationString()}.`, 2, 1) } diff --git a/packages/cli/src/reporters/json.ts b/packages/cli/src/reporters/json.ts index 5c9dae91..d711157e 100644 --- a/packages/cli/src/reporters/json.ts +++ b/packages/cli/src/reporters/json.ts @@ -2,7 +2,7 @@ import * as fs from 'fs' import * as path from 'path' import AbstractListReporter, { checkFilesMap } from './abstract-list' -import { CheckRunId } from '../services/abstract-check-runner' +import { CheckRunId, SequenceId } from '../services/abstract-check-runner' import { printLn, getTestSessionUrl } from './util' const outputFile = './checkly-json-report.json' @@ -40,7 +40,7 @@ export class JsonBuilder { // eslint-disable-next-line @typescript-eslint/no-unused-vars for (const [_, checkMap] of this.checkFilesMap.entries()) { // eslint-disable-next-line @typescript-eslint/no-unused-vars - for (const [_, { result, testResultId }] of checkMap.entries()) { + for (const [_, { result, testResultId, numRetries }] of checkMap.entries()) { const check: any = { result: result.hasFailures ? 'Fail' : 'Pass', name: result.name, @@ -49,6 +49,7 @@ export class JsonBuilder { filename: null, link: null, runError: result.runError || null, + retries: numRetries, } if (this.hasFilenames) { @@ -68,7 +69,7 @@ export class JsonBuilder { } export default class JsonReporter extends AbstractListReporter { - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string) { + onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, sequenceId: SequenceId }>, testSessionId?: string) { super.onBegin(checks, testSessionId) printLn(`Running ${this.numChecks} checks in ${this._runLocationString()}.`, 2, 1) } diff --git a/packages/cli/src/reporters/list.ts b/packages/cli/src/reporters/list.ts index a4e8eb3b..00c318ff 100644 --- a/packages/cli/src/reporters/list.ts +++ b/packages/cli/src/reporters/list.ts @@ -2,19 +2,19 @@ import indentString from 'indent-string' import chalk from 'chalk' import AbstractListReporter from './abstract-list' -import { CheckRunId } from '../services/abstract-check-runner' +import { SequenceId } from '../services/abstract-check-runner' import { formatCheckTitle, formatCheckResult, CheckStatus, printLn } from './util' import { TestResultsShortLinks } from '../rest/test-sessions' export default class ListReporter extends AbstractListReporter { - onBegin (checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string) { + onBegin (checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string) { super.onBegin(checks, testSessionId) printLn(`Running ${this.numChecks} checks in ${this._runLocationString()}.`, 2, 1) this._printSummary() } - onCheckInProgress (check: any, checkRunId: CheckRunId) { - super.onCheckInProgress(check, checkRunId) + onCheckInProgress (check: any, sequenceId: SequenceId) { + super.onCheckInProgress(check, sequenceId) this._clearSummary() this._printSummary() } @@ -31,8 +31,30 @@ export default class ListReporter extends AbstractListReporter { this._printTestSessionsUrl() } - onCheckEnd (checkRunId: CheckRunId, checkResult: any, links?: TestResultsShortLinks) { - super.onCheckEnd(checkRunId, checkResult) + onCheckAttemptResult (sequenceId: string, checkResult: any, links?: TestResultsShortLinks): void { + super.onCheckAttemptResult(sequenceId, checkResult) + this._clearSummary() + + printLn(formatCheckTitle(CheckStatus.RETRIED, checkResult, { printRetryDuration: true })) + printLn(indentString(formatCheckResult(checkResult), 4), 2, 1) + if (links) { + printLn(indentString('View result: ' + chalk.underline.cyan(`${links.testResultLink}`), 4)) + if (links.testTraceLinks?.length) { + // TODO: print all video files URLs + printLn(indentString('View trace : ' + chalk.underline.cyan(links.testTraceLinks.join(', ')), 4)) + } + if (links.videoLinks?.length) { + // TODO: print all trace files URLs + printLn(indentString('View video : ' + chalk.underline.cyan(`${links.videoLinks.join(', ')}`), 4)) + } + printLn('') + } + + this._printSummary() + } + + onCheckEnd (sequenceId: SequenceId, checkResult: any, testResultId?: string, links?: TestResultsShortLinks) { + super.onCheckEnd(sequenceId, checkResult, testResultId, links) this._clearSummary() if (this.verbose) { diff --git a/packages/cli/src/reporters/reporter.ts b/packages/cli/src/reporters/reporter.ts index 6e622390..c3b32a65 100644 --- a/packages/cli/src/reporters/reporter.ts +++ b/packages/cli/src/reporters/reporter.ts @@ -1,5 +1,5 @@ import { TestResultsShortLinks } from '../rest/test-sessions' -import { RunLocation, CheckRunId } from '../services/abstract-check-runner' +import { RunLocation, SequenceId } from '../services/abstract-check-runner' import CiReporter from './ci' import DotReporter from './dot' import GithubReporter from './github' @@ -7,10 +7,11 @@ import ListReporter from './list' import JsonReporter from './json' export interface Reporter { - onBegin(checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, testSessionId?: string): void; - onCheckInProgress(check: any, checkRunId: CheckRunId): void; + onBegin(checks: Array<{ check: any, sequenceId: SequenceId }>, testSessionId?: string): void; + onCheckInProgress(check: any, sequenceId: SequenceId): void; + onCheckAttemptResult(sequenceId: SequenceId, checkResult: any, links?: TestResultsShortLinks): void; + onCheckEnd(sequenceId: SequenceId, checkResult: any, testResultId?: string, links?: TestResultsShortLinks): void; onEnd(): void; - onCheckEnd(checkRunId: CheckRunId, checkResult: any, links?: TestResultsShortLinks): void; onError(err: Error): void, onSchedulingDelayExceeded(): void } diff --git a/packages/cli/src/reporters/util.ts b/packages/cli/src/reporters/util.ts index f57780d5..988480c3 100644 --- a/packages/cli/src/reporters/util.ts +++ b/packages/cli/src/reporters/util.ts @@ -10,6 +10,7 @@ import { getDefaults } from '../rest/api' export enum CheckStatus { SCHEDULING, RUNNING, + RETRIED, FAILED, SUCCESSFUL, } @@ -22,9 +23,13 @@ export function formatDuration (ms: number): string { } } -export function formatCheckTitle (status: CheckStatus, check: any, opts: { includeSourceFile?: boolean } = {}) { +export function formatCheckTitle ( + status: CheckStatus, + check: any, + opts: { includeSourceFile?: boolean, printRetryDuration?: boolean } = {}, +) { let duration - if (check.startedAt && check.stoppedAt) { + if ((opts.printRetryDuration || status !== CheckStatus.RETRIED) && check.startedAt && check.stoppedAt) { const durationMs = DateTime.fromISO(check.stoppedAt) .diff(DateTime.fromISO(check.startedAt)) .toMillis() @@ -42,6 +47,9 @@ export function formatCheckTitle (status: CheckStatus, check: any, opts: { inclu } else if (status === CheckStatus.SCHEDULING) { statusString = '~' format = chalk.bold.dim + } else if (status === CheckStatus.RETRIED) { + statusString = '↺' + format = chalk.bold } else { statusString = '-' format = chalk.bold.dim diff --git a/packages/cli/src/rest/test-sessions.ts b/packages/cli/src/rest/test-sessions.ts index 5fad391d..cea6eb59 100644 --- a/packages/cli/src/rest/test-sessions.ts +++ b/packages/cli/src/rest/test-sessions.ts @@ -1,5 +1,6 @@ import type { AxiosInstance } from 'axios' import { GitInformation } from '../services/util' +import { RetryStrategy } from '../constructs' type RunTestSessionRequest = { name: string, @@ -20,6 +21,7 @@ type TriggerTestSessionRequest = { environmentVariables: Array<{ key: string, value: string }>, repoInfo: GitInformation | null, environment: string | null, + testRetryStrategy: RetryStrategy | null, } export type TestResultsShortLinks = { diff --git a/packages/cli/src/services/abstract-check-runner.ts b/packages/cli/src/services/abstract-check-runner.ts index 1d41fbf1..fe81af15 100644 --- a/packages/cli/src/services/abstract-check-runner.ts +++ b/packages/cli/src/services/abstract-check-runner.ts @@ -11,6 +11,7 @@ import { TestResultsShortLinks } from '../rest/test-sessions' export enum Events { CHECK_REGISTERED = 'CHECK_REGISTERED', CHECK_INPROGRESS = 'CHECK_INPROGRESS', + CHECK_ATTEMPT_RESULT = 'CHECK_ATTEMPT_RESULT', CHECK_FAILED = 'CHECK_FAILED', CHECK_SUCCESSFUL = 'CHECK_SUCCESSFUL', CHECK_FINISHED = 'CHECK_FINISHED', @@ -32,17 +33,18 @@ export type PublicRunLocation = { export type RunLocation = PublicRunLocation | PrivateRunLocation export type CheckRunId = string +export type SequenceId = string export const DEFAULT_CHECK_RUN_TIMEOUT_SECONDS = 600 const DEFAULT_SCHEDULING_DELAY_EXCEEDED_MS = 20000 export default abstract class AbstractCheckRunner extends EventEmitter { - checks: Map + checks: Map testSessionId?: string // If there's an error in the backend and no check result is sent, the check run could block indefinitely. // To avoid this case, we set a per-check timeout. - timeouts: Map + timeouts: Map schedulingDelayExceededTimeout?: NodeJS.Timeout accountId: string timeout: number @@ -66,7 +68,7 @@ export default abstract class AbstractCheckRunner extends EventEmitter { abstract scheduleChecks (checkRunSuiteId: string): Promise<{ testSessionId?: string, - checks: Array<{ check: any, checkRunId: CheckRunId, testResultId?: string }>, + checks: Array<{ check: any, sequenceId: SequenceId }>, }> async run () { @@ -81,7 +83,7 @@ export default abstract class AbstractCheckRunner extends EventEmitter { const { testSessionId, checks } = await this.scheduleChecks(checkRunSuiteId) this.testSessionId = testSessionId this.checks = new Map( - checks.map(({ check, checkRunId, testResultId }) => [checkRunId, { check, testResultId }]), + checks.map(({ check, sequenceId }) => [sequenceId, { check }]), ) // `processMessage()` assumes that `this.timeouts` always has an entry for non-timed-out checks. @@ -116,38 +118,47 @@ export default abstract class AbstractCheckRunner extends EventEmitter { socketClient.on('message', (topic: string, rawMessage: string|Buffer) => { const message = JSON.parse(rawMessage.toString('utf8')) const topicComponents = topic.split('/') - const checkRunId = topicComponents[4] - const subtopic = topicComponents[5] + const sequenceId = topicComponents[4] + const checkRunId = topicComponents[5] + const subtopic = topicComponents[6] - this.queue.add(() => this.processMessage(checkRunId, subtopic, message)) + this.queue.add(() => this.processMessage(sequenceId, subtopic, message)) }) - await socketClient.subscribe(`account/${this.accountId}/ad-hoc-check-results/${checkRunSuiteId}/+/+`) + await socketClient.subscribe(`account/${this.accountId}/ad-hoc-check-results/${checkRunSuiteId}/+/+/+`) } - private async processMessage (checkRunId: string, subtopic: string, message: any) { - if (!this.timeouts.has(checkRunId)) { + private async processMessage (sequenceId: SequenceId, subtopic: string, message: any) { + if (!sequenceId) { + // There should always be a sequenceId, but let's have an early return to make forwards-compatibility easier. + return + } + + if (!this.timeouts.has(sequenceId)) { // The check has already timed out. We return early to avoid reporting a duplicate result. return } - if (!this.checks.get(checkRunId)) { - // The check has no checkRunId associated. + if (!this.checks.get(sequenceId)) { return } - const { check, testResultId } = this.checks.get(checkRunId)! + const { check } = this.checks.get(sequenceId)! if (subtopic === 'run-start') { - this.emit(Events.CHECK_INPROGRESS, check, checkRunId) - } else if (subtopic === 'run-end') { - this.disableTimeout(checkRunId) - const { result } = message + this.emit(Events.CHECK_INPROGRESS, check, sequenceId) + } else if (subtopic === 'result') { + const { result, testResultId, resultType } = message await this.processCheckResult(result) const links = testResultId && result.hasFailures && await this.getShortLinks(testResultId) - this.emit(Events.CHECK_SUCCESSFUL, checkRunId, check, result, links) - this.emit(Events.CHECK_FINISHED, check) + if (resultType === 'FINAL') { + this.disableTimeout(sequenceId) + this.emit(Events.CHECK_SUCCESSFUL, sequenceId, check, result, testResultId, links) + this.emit(Events.CHECK_FINISHED, check) + } else if (resultType === 'ATTEMPT') { + this.emit(Events.CHECK_ATTEMPT_RESULT, sequenceId, check, result, links) + } } else if (subtopic === 'error') { - this.disableTimeout(checkRunId) - this.emit(Events.CHECK_FAILED, checkRunId, check, message) + this.disableTimeout(sequenceId) + this.emit(Events.CHECK_FAILED, sequenceId, check, message) this.emit(Events.CHECK_FINISHED, check) } } @@ -181,16 +192,16 @@ export default abstract class AbstractCheckRunner extends EventEmitter { } private setAllTimeouts () { - Array.from(this.checks.entries()).forEach(([checkRunId, { check }]) => - this.timeouts.set(checkRunId, setTimeout(() => { - this.timeouts.delete(checkRunId) + Array.from(this.checks.entries()).forEach(([sequenceId, { check }]) => + this.timeouts.set(sequenceId, setTimeout(() => { + this.timeouts.delete(sequenceId) let errorMessage = `Reached timeout of ${this.timeout} seconds waiting for check result.` // Checkly should always report a result within 240s. // If the default timeout was used, we should point the user to the status page and support email. if (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, checkRunId, check, errorMessage) + this.emit(Events.CHECK_FAILED, sequenceId, check, errorMessage) this.emit(Events.CHECK_FINISHED, check) }, this.timeout * 1000), )) diff --git a/packages/cli/src/services/checkly-config-loader.ts b/packages/cli/src/services/checkly-config-loader.ts index 563d279c..eef8fbb2 100644 --- a/packages/cli/src/services/checkly-config-loader.ts +++ b/packages/cli/src/services/checkly-config-loader.ts @@ -68,6 +68,7 @@ export type ChecklyConfig = { privateRunLocation?: string, verbose?: boolean, reporters?: ReporterType[], + retries?: number, } } diff --git a/packages/cli/src/services/test-runner.ts b/packages/cli/src/services/test-runner.ts index 75f59104..cb132353 100644 --- a/packages/cli/src/services/test-runner.ts +++ b/packages/cli/src/services/test-runner.ts @@ -1,8 +1,8 @@ import { testSessions } from '../rest/api' -import AbstractCheckRunner, { RunLocation, CheckRunId } from './abstract-check-runner' +import AbstractCheckRunner, { RunLocation, SequenceId } from './abstract-check-runner' import { GitInformation } from './util' import { Check } from '../constructs/check' -import { Project } from '../constructs' +import { RetryStrategy, Project } from '../constructs' import { pullSnapshots } from '../services/snapshot-service' import * as uuid from 'uuid' @@ -16,6 +16,8 @@ export default class TestRunner extends AbstractCheckRunner { environment: string | null updateSnapshots: boolean baseDirectory: string + testRetryStrategy: RetryStrategy | null + constructor ( accountId: string, project: Project, @@ -28,6 +30,7 @@ export default class TestRunner extends AbstractCheckRunner { environment: string | null, updateSnapshots: boolean, baseDirectory: string, + testRetryStrategy: RetryStrategy | null, ) { super(accountId, timeout, verbose) this.project = project @@ -38,22 +41,25 @@ export default class TestRunner extends AbstractCheckRunner { this.environment = environment this.updateSnapshots = updateSnapshots this.baseDirectory = baseDirectory + this.testRetryStrategy = testRetryStrategy } async scheduleChecks ( checkRunSuiteId: string, ): Promise<{ testSessionId?: string, - checks: Array<{ check: any, checkRunId: CheckRunId, testSessionId?: string }>, + checks: Array<{ check: any, sequenceId: SequenceId }>, }> { - const checkRunIdMap = new Map( - this.checkConstructs.map((check) => [uuid.v4(), check]), - ) - const checkRunJobs = Array.from(checkRunIdMap.entries()).map(([checkRunId, check]) => ({ + const checkRunJobs = this.checkConstructs.map(check => ({ ...check.synthesize(), + testRetryStrategy: this.testRetryStrategy, group: check.groupId ? this.project.data['check-group'][check.groupId.ref].synthesize() : undefined, groupId: undefined, - sourceInfo: { checkRunSuiteId, checkRunId, updateSnapshots: this.updateSnapshots }, + sourceInfo: { + checkRunSuiteId, + checkRunId: uuid.v4(), + updateSnapshots: this.updateSnapshots, + }, logicalId: check.logicalId, filePath: check.getSourceFile(), })) @@ -70,9 +76,8 @@ export default class TestRunner extends AbstractCheckRunner { environment: this.environment, shouldRecord: this.shouldRecord, }) - const { testSessionId, testResultIds } = data - const checks = Array.from(checkRunIdMap.entries()) - .map(([checkRunId, check]) => ({ check, checkRunId, testResultId: testResultIds?.[check.logicalId] })) + const { testSessionId, sequenceIds } = data + const checks = this.checkConstructs.map(check => ({ check, sequenceId: sequenceIds?.[check.logicalId] })) return { testSessionId, checks } } catch (err: any) { throw new Error(err.response?.data?.message ?? err.response?.data?.error ?? err.message) diff --git a/packages/cli/src/services/trigger-runner.ts b/packages/cli/src/services/trigger-runner.ts index 143c0d78..9d6beba6 100644 --- a/packages/cli/src/services/trigger-runner.ts +++ b/packages/cli/src/services/trigger-runner.ts @@ -1,5 +1,6 @@ +import { RetryStrategy } from '../constructs' import { testSessions } from '../rest/api' -import AbstractCheckRunner, { RunLocation, CheckRunId } from './abstract-check-runner' +import AbstractCheckRunner, { RunLocation, SequenceId } from './abstract-check-runner' import { GitInformation } from './util' export class NoMatchingChecksError extends Error {} @@ -12,6 +13,7 @@ export default class TriggerRunner extends AbstractCheckRunner { repoInfo: GitInformation | null environment: string | null testSessionName: string | undefined + testRetryStrategy: RetryStrategy | null constructor ( accountId: string, @@ -24,6 +26,7 @@ export default class TriggerRunner extends AbstractCheckRunner { repoInfo: GitInformation | null, environment: string | null, testSessionName: string | undefined, + testRetryStrategy: RetryStrategy | null, ) { super(accountId, timeout, verbose) this.shouldRecord = shouldRecord @@ -33,13 +36,14 @@ export default class TriggerRunner extends AbstractCheckRunner { this.repoInfo = repoInfo this.environment = environment this.testSessionName = testSessionName + this.testRetryStrategy = testRetryStrategy } async scheduleChecks ( checkRunSuiteId: string, ): Promise<{ testSessionId?: string, - checks: Array<{ check: any, checkRunId: CheckRunId, testSessionId?: string }>, + checks: Array<{ check: any, sequenceId: SequenceId }>, }> { try { const { data } = await testSessions.trigger({ @@ -51,31 +55,26 @@ export default class TriggerRunner extends AbstractCheckRunner { environmentVariables: this.envVars, repoInfo: this.repoInfo, environment: this.environment, + testRetryStrategy: this.testRetryStrategy, }) const { checks, - checkRunIds, testSessionId, - testResultIds, + sequenceIds, }: { checks: Array, - checkRunIds: Record, testSessionId: string, testResultIds: Record, + sequenceIds: Record } = data if (!checks.length) { // Currently the BE will never return an empty `checks` array, it returns a 403 instead. // This is added to make the old CLI versions compatible if we ever change this, though. throw new NoMatchingChecksError() } - const checksMap: Record = checks.reduce((acc: Record, check: any) => { - acc[check.id] = check - return acc - }, {}) - const augmentedChecks = Object.entries(checkRunIds).map(([checkId, checkRunId]) => ({ - checkRunId, - check: checksMap[checkId], - testResultId: testResultIds?.[checkId], + const augmentedChecks = checks.map(check => ({ + check, + sequenceId: sequenceIds?.[check.id], })) return { checks: augmentedChecks, testSessionId } } catch (err: any) {