[SYNTH-16456] Make server result optional in Result

src/commands/synthetics/__tests__/fixtures.ts

@@ -177,6 +177,7 @@ export const getSummary = (): Summary => ({
 })
 
 const getBaseResult = (resultId: string, test: Test): Omit<BaseResult, 'result'> => ({
+  duration: 1000,
   executionRule: ExecutionRule.BLOCKING,
   location: 'Frankfurt (AWS)',
   passed: true,
@@ -192,12 +193,16 @@ export const getBrowserResult = (
   resultId: string,
   test: Test,
   resultOpts: Partial<BrowserServerResult> = {}
-): BaseResult => ({
+): BaseResult & {result: ServerResult} => ({
   ...getBaseResult(resultId, test),
   result: getBrowserServerResult(resultOpts),
 })
 
-export const getApiResult = (resultId: string, test: Test, resultOpts: Partial<ApiServerResult> = {}): BaseResult => ({
+export const getApiResult = (
+  resultId: string,
+  test: Test,
+  resultOpts: Partial<ApiServerResult> = {}
+): BaseResult & {result: ServerResult} => ({
   ...getBaseResult(resultId, test),
   result: getApiServerResult(resultOpts),
 })
@@ -208,14 +213,15 @@ export const getIncompleteServerResult = (): ServerResult => {
 
 export const getBrowserServerResult = (opts: Partial<BrowserServerResult> = {}): BrowserServerResult => ({
   device: {height: 1100, id: 'chrome.laptop_large', width: 1440},
-  duration: 0,
+  duration: 1000,
   passed: true,
   startUrl: '',
   stepDetails: [],
   ...opts,
 })
 
 export const getTimedOutBrowserResult = (): Result => ({
+  duration: 0,
   executionRule: ExecutionRule.BLOCKING,
   location: 'Location name',
   passed: false,
@@ -234,12 +240,13 @@ export const getTimedOutBrowserResult = (): Result => ({
 })
 
 export const getFailedBrowserResult = (): Result => ({
+  duration: 22000,
   executionRule: ExecutionRule.BLOCKING,
   location: 'Location name',
   passed: false,
   result: {
     device: {height: 1100, id: 'chrome.laptop_large', width: 1440},
-    duration: 20000,
+    duration: 22000,
     failure: {code: 'STEP_TIMEOUT', message: 'Step failed because it took more than 20 seconds.'},
     passed: false,
     startUrl: 'https://example.org/',
@@ -259,7 +266,7 @@ export const getFailedBrowserResult = (): Result => ({
       {
         ...getStep(),
         allowFailure: true,
-        description: 'Navigate',
+        description: 'Navigate again',
         duration: 1000,
         error: 'Navigation failure',
         skipped: true,
@@ -272,10 +279,9 @@ export const getFailedBrowserResult = (): Result => ({
       {
         ...getStep(),
         description: 'Assert',
-        duration: 1000,
-        error: 'Step failure',
+        duration: 20000,
+        error: 'Step timeout',
         publicId: 'abc-def-hij',
-        skipped: true,
         stepId: 3,
         type: 'assertElementContent',
         url: 'https://example.org/',
@@ -303,13 +309,13 @@ export const getApiServerResult = (opts: Partial<ApiServerResult> = {}): ApiServ
   ],
   passed: true,
   timings: {
-    total: 123,
+    total: 1000,
   },
   ...opts,
 })
 
 export const getMultiStepsServerResult = (): MultiStepsServerResult => ({
-  duration: 123,
+  duration: 1000,
   passed: true,
   steps: [],
 })
@@ -507,6 +513,7 @@ export const getResults = (resultsFixtures: ResultFixtures[]): Result[] => {
 
 export const getInProgressResultInBatch = (): BaseResultInBatch => {
   return {
+    duration: 0,
     execution_rule: ExecutionRule.BLOCKING,
     location: mockLocation.name,
     result_id: 'rid',
@@ -543,6 +550,7 @@ export const getSkippedResultInBatch = (): ResultInBatchSkippedBySelectiveRerun
 export const getPassedResultInBatch = (): BaseResultInBatch => {
   return {
     ...getInProgressResultInBatch(),
+    duration: 1000,
     retries: 0,
     status: 'passed',
     timed_out: false,
@@ -552,6 +560,7 @@ export const getPassedResultInBatch = (): BaseResultInBatch => {
 export const getFailedResultInBatch = (): BaseResultInBatch => {
   return {
     ...getInProgressResultInBatch(),
+    duration: 1000,
     retries: 0,
     status: 'failed',
     timed_out: false,

src/commands/synthetics/__tests__/reporters/junit.test.ts

@@ -505,14 +505,14 @@ describe('GitLab test report compatibility', () => {
         step: 'Assert',
         type: 'assertion',
       },
-      _: 'Step failure',
+      _: 'Step timeout',
     }
 
     expect(testCase.$).toHaveProperty('classname', 'tests.json') // Suite
     expect(testCase.$).toHaveProperty('name', name) // Name
     expect(testCase.$).toHaveProperty('file', 'tests.json') // Filename
     expect(testCase.failure).toStrictEqual([failure]) // Status
-    expect(testCase.$).toHaveProperty('time', 20) // Duration
+    expect(testCase.$).toHaveProperty('time', 22) // Duration
   })
 
   test('the icon in the Status column is correct (blocking vs. non-blocking)', () => {

src/commands/synthetics/__tests__/utils/public.test.ts

@@ -831,7 +831,8 @@ describe('utils', () => {
 
     const batch: Batch = getBatch()
     const apiTest = getApiTest('pid')
-    const result: Result = {
+    const result: BaseResult & {result: ServerResult} = {
+      duration: 1000,
       executionRule: ExecutionRule.BLOCKING,
       initialResultId: undefined,
       isNonFinal: false,
@@ -1035,6 +1036,7 @@ describe('utils', () => {
             // Second test
             {
               ...getInProgressResultInBatch(), // stays in progress
+              duration: 1000,
               retries: 0, // `retries` is set => first attempt failed, but will be fast retried
               test_public_id: 'other-public-id',
               timed_out: false,
@@ -1054,6 +1056,7 @@ describe('utils', () => {
       // One result received
       expect(mockReporter.resultReceived).toHaveBeenNthCalledWith(3, {
         ...batch.results[0],
+        duration: 1000,
         status: 'in_progress',
         test_public_id: 'other-public-id',
         result_id: 'rid-3',
@@ -1428,8 +1431,9 @@ describe('utils', () => {
         ],
       })
 
-      const expectedDeadlineResult = {
+      const expectedDeadlineResult: Result = {
         ...result,
+        duration: 0,
         result: {
           ...result.result,
           failure: {

src/commands/synthetics/batch.ts

@@ -252,6 +252,7 @@ const getResultFromBatch = (
   const isUnhealthy = pollResult.result.unhealthy ?? false
 
   return {
+    duration: resultInBatch.duration,
     executionRule: resultInBatch.execution_rule,
     initialResultId: resultInBatch.initial_result_id,
     isNonFinal: isNonFinalResult(resultInBatch),

src/commands/synthetics/interfaces.ts

@@ -127,14 +127,16 @@ export type SelectiveRerunDecision =
     }
 
 export interface BaseResult {
+  /** Duration of the result in milliseconds. */
+  duration: number
   executionRule: ExecutionRule
   initialResultId?: string
   /** Whether the result is an intermediary result that is expected to be retried. */
   isNonFinal?: boolean
   location: string
   /** Whether the result is passed or not, according to `failOnCriticalErrors` and `failOnTimeout`. */
   passed: boolean
-  result: ServerResult
+  result?: ServerResult
   resultId: string
   /** Number of retries, including this result. */
   retries: number
@@ -149,7 +151,7 @@ export interface BaseResult {
 // Inside this type, `.resultId` is a linked result ID from a previous batch.
 export type ResultSkippedBySelectiveRerun = Omit<
   BaseResult,
-  'location' | 'result' | 'retries' | 'maxRetries' | 'timestamp'
+  'duration' | 'location' | 'result' | 'retries' | 'maxRetries' | 'timestamp'
 > & {
   executionRule: ExecutionRule.SKIPPED
   selectiveRerun: Extract<SelectiveRerunDecision, {decision: 'skip'}>
@@ -161,6 +163,7 @@ type Status = 'passed' | 'failed' | 'in_progress' | 'skipped'
 type BatchStatus = 'passed' | 'failed' | 'in_progress'
 
 export interface BaseResultInBatch {
+  duration: number
   execution_rule: ExecutionRule
   initial_result_id?: string
   location: string
@@ -173,7 +176,7 @@ export interface BaseResultInBatch {
   timed_out: boolean | null
 }
 
-type SkippedResultInBatch = Omit<BaseResultInBatch, 'location' | 'result_id'> & {
+type SkippedResultInBatch = Omit<BaseResultInBatch, 'duration' | 'location' | 'result_id'> & {
   execution_rule: ExecutionRule.SKIPPED
   status: 'skipped'
 }

src/commands/synthetics/reporters/default.ts

@@ -19,10 +19,9 @@ import {
   UserConfigOverride,
   ResultInBatch,
 } from '../interfaces'
-import {hasResult, isTimedOutRetry} from '../utils/internal'
+import {isBaseResult, isTimedOutRetry} from '../utils/internal'
 import {
   getBatchUrl,
-  getResultDuration,
   getResultOutcome,
   getResultUrl,
   getTestOverridesCount,
@@ -111,12 +110,12 @@ const renderApiError = (errorCode: string, errorMessage: string, color: chalk.Ch
 
 // Test execution rendering
 const renderResultOutcome = (
-  result: ServerResult,
+  result: ServerResult | undefined,
   test: Test,
   icon: string,
   color: chalk.Chalk
 ): string | undefined => {
-  if (result.unhealthy) {
+  if (result?.unhealthy) {
     const error =
       result.failure && result.failure.message !== 'Unknown error' ? result.failure.message : 'General Error'
 
@@ -129,7 +128,7 @@ const renderResultOutcome = (
   if (test.type === 'api') {
     const requestDescription = renderApiRequestDescription(test.subtype, test.config)
 
-    if (result.failure) {
+    if (result?.failure) {
       return [
         `  ${icon} ${color(requestDescription)}`,
         renderApiError(result.failure.code, result.failure.message, color),
@@ -142,12 +141,12 @@ const renderResultOutcome = (
   if (test.type === 'browser') {
     const lines: string[] = []
 
-    if (result.failure) {
+    if (result?.failure) {
       lines.push(chalk.red(`    [${chalk.bold(result.failure.code)}] - ${chalk.dim(result.failure.message)}`))
     }
 
     // We render the step only if the test hasn't passed to avoid cluttering the output.
-    if (!result.passed && 'stepDetails' in result) {
+    if (result && !result.passed && 'stepDetails' in result) {
       const criticalFailedStepIndex = result.stepDetails.findIndex((s) => s.error && !s.allowFailure) + 1
       lines.push(...result.stepDetails.slice(0, criticalFailedStepIndex).map(renderStep))
 
@@ -221,9 +220,8 @@ const renderExecutionResult = (test: Test, execution: Result, baseUrl: string, b
   const outputLines = [resultIdentification]
 
   // Unhealthy test results don't have a duration or result URL
-  if (hasResult(execution) && !execution.result.unhealthy) {
-    const duration = getResultDuration(execution.result)
-    const durationText = duration ? ` Total duration: ${duration} ms -` : ''
+  if (isBaseResult(execution) && !execution.result?.unhealthy) {
+    const durationText = execution.duration ? ` Total duration: ${execution.duration} ms -` : ''
 
     const resultUrl = getResultUrl(baseUrl, test, resultId, batchId)
     const resultUrlStatus = getResultUrlSuffix(execution)
@@ -249,7 +247,7 @@ const renderExecutionResult = (test: Test, execution: Result, baseUrl: string, b
 }
 
 const getResultIdentificationSuffix = (execution: Result, setColor: chalk.Chalk) => {
-  if (hasResult(execution)) {
+  if (isBaseResult(execution)) {
     const {result, passed, retries, maxRetries, timedOut} = execution
     const location = execution.location ? setColor(`location: ${chalk.bold(execution.location)}`) : ''
     const device = result && isDeviceIdSet(result) ? ` - ${setColor(`device: ${chalk.bold(result.device.id)}`)}` : ''
@@ -262,7 +260,7 @@ const getResultIdentificationSuffix = (execution: Result, setColor: chalk.Chalk)
 }
 
 export const getResultUrlSuffix = (execution: Result) => {
-  if (hasResult(execution)) {
+  if (isBaseResult(execution)) {
     const {retries, maxRetries, timedOut} = execution
     const timedOutRetry = isTimedOutRetry(retries, maxRetries, timedOut)