Improve test coverage + refactor

src/commands/synthetics/__tests__/batch.test.ts

@@ -1,26 +1,140 @@
-import {getResultsToReport} from '../batch'
+import {getResultsToReport, reportReceivedResults} from '../batch'
 import {Batch, ResultInBatch} from '../interfaces'
 
-import {getFailedResultInBatch, mockReporter} from './fixtures'
+import {
+  getFailedResultInBatch,
+  getInProgressResultInBatch,
+  getPassedResultInBatch,
+  getSkippedResultInBatch,
+  mockReporter,
+} from './fixtures'
 
 describe('getResultsToReport', () => {
-  describe('shouldContinuePolling: false', () => {
-    test('timed out retry', () => {
+  test.each([false])('timed out retry - shouldContinuePolling=%s', (shouldContinuePolling: boolean) => {
+    const timedOutRetry: ResultInBatch = {
+      ...getFailedResultInBatch(),
+      retries: 0,
+      max_retries: 1,
+      timed_out: true, // Can only be true when the backend timed out the batch, i.e. `shouldContinuePolling` is false.
+    }
+
+    const batch: Batch = {
+      status: 'failed',
+      results: [timedOutRetry],
+    }
+
+    const resultsToReport = getResultsToReport(
+      shouldContinuePolling,
+      batch,
+      [],
+      new Set(['rid']),
+      new Set(),
+      new Set(),
+      mockReporter
+    )
+
+    expect(resultsToReport).toStrictEqual([timedOutRetry])
+  })
+
+  test.each([false])(
+    'timed out retry never emitted before - shouldContinuePolling=%s',
+    (shouldContinuePolling: boolean) => {
       const timedOutRetry: ResultInBatch = {
         ...getFailedResultInBatch(),
         retries: 0,
         max_retries: 1,
-        timed_out: true,
+        timed_out: true, // Can only be true when the backend timed out the batch, i.e. `shouldContinuePolling` is false.
       }
 
       const batch: Batch = {
         status: 'failed',
         results: [timedOutRetry],
       }
 
-      const resultsToReport = getResultsToReport(false, batch, [], new Set(['rid']), new Set(), new Set(), mockReporter)
+      const resultsToReport = getResultsToReport(
+        shouldContinuePolling,
+        batch,
+        [timedOutRetry],
+        new Set(),
+        new Set(),
+        new Set(),
+        mockReporter
+      )
 
       expect(resultsToReport).toStrictEqual([timedOutRetry])
-    })
+    }
+  )
+})
+
+describe('reportReceivedResults', () => {
+  test('skipped', () => {
+    const skippedResult = getSkippedResultInBatch()
+
+    const batch: Batch = {
+      status: 'failed',
+      results: [skippedResult],
+    }
+
+    const emittedResultIds = new Set<string>()
+    const receivedResults = reportReceivedResults(batch, emittedResultIds, mockReporter)
+
+    expect(receivedResults).toStrictEqual([skippedResult])
+    expect(emittedResultIds).toContain('skipped-0')
+    expect(mockReporter.resultReceived).toHaveBeenCalledWith(skippedResult)
+  })
+
+  test('final', () => {
+    const result = getPassedResultInBatch()
+
+    const batch: Batch = {
+      status: 'passed',
+      results: [result],
+    }
+
+    const emittedResultIds = new Set<string>()
+    const receivedResults = reportReceivedResults(batch, emittedResultIds, mockReporter)
+
+    expect(receivedResults).toStrictEqual([result])
+    expect(emittedResultIds).toContain('rid')
+    expect(mockReporter.resultReceived).toHaveBeenCalledWith(result)
+  })
+
+  test('non final', () => {
+    const result: ResultInBatch = {
+      ...getInProgressResultInBatch(),
+      retries: 0,
+      max_retries: 1,
+    }
+
+    const batch: Batch = {
+      status: 'in_progress',
+      results: [result],
+    }
+
+    const emittedResultIds = new Set<string>()
+    const receivedResults = reportReceivedResults(batch, emittedResultIds, mockReporter)
+
+    expect(receivedResults).toStrictEqual([result])
+    expect(emittedResultIds).toContain('rid')
+    expect(mockReporter.resultReceived).toHaveBeenCalledWith(result)
+  })
+
+  test('timed out', () => {
+    const timedOut: ResultInBatch = {
+      ...getFailedResultInBatch(),
+      timed_out: true,
+    }
+
+    const batch: Batch = {
+      status: 'failed',
+      results: [timedOut],
+    }
+
+    const emittedResultIds = new Set<string>()
+    const receivedResults = reportReceivedResults(batch, emittedResultIds, mockReporter)
+
+    expect(receivedResults).toStrictEqual([timedOut])
+    expect(emittedResultIds).toContain('rid')
+    expect(mockReporter.resultReceived).toHaveBeenCalledWith(timedOut)
   })
 })

src/commands/synthetics/batch.ts

@@ -109,22 +109,18 @@ export const getResultsToReport = (
   )
 
   const resultsToReport = newlyReceivedResults
-    .filter((r) => isResultInBatchSkippedBySelectiveRerun(r) || !incompleteResultIds.has(r.result_id))
+    .filter(
+      (r) => isResultInBatchSkippedBySelectiveRerun(r) || !isResidualResult(r, emittedResultIds, incompleteResultIds)
+    )
     .concat(newlyCompleteResults)
 
   if (shouldContinuePolling) {
     return resultsToReport
   }
 
-  // Residual results are either:
-  //  - Still in progress (from the batch POV): they were never emitted.
-  //  - Or still incomplete (from the poll results POV): report them with their incomplete data and a warning.
-  //  - Or the current result ID was already emitted but it used to be non-final result, and it's now final.
-  const residualResults = excludeSkipped(batch.results).filter(
-    (r) =>
-      !emittedResultIds.has(r.result_id) ||
-      incompleteResultIds.has(r.result_id) ||
-      isTimedOutRetry(r.retries, r.max_retries, r.timed_out)
+  // Results that we failed to report for some reason are finally reported as "residues".
+  const residualResults = excludeSkipped(batch.results).filter((r) =>
+    isResidualResult(r, emittedResultIds, incompleteResultIds)
   )
 
   const errors: string[] = []
@@ -143,7 +139,7 @@ export const getResultsToReport = (
   return resultsToReport.concat(residualResults)
 }
 
-const reportReceivedResults = (batch: Batch, emittedResultIds: Set<string>, reporter: MainReporter) => {
+export const reportReceivedResults = (batch: Batch, emittedResultIds: Set<string>, reporter: MainReporter) => {
   const receivedResults: ResultInBatch[] = []
 
   for (const [index, result] of batch.results.entries()) {
@@ -303,6 +299,27 @@ const getPollResultMap = async (api: APIHelper, resultIds: string[]) => {
   }
 }
 
+const isResidualResult = (
+  result: BaseResultInBatch,
+  emittedResultIds: Set<string>,
+  incompleteResultIds: Set<string>
+) => {
+  if (incompleteResultIds.has(result.result_id)) {
+    // The poll results endpoint returned an incomplete result: report it with incomplete data and a warning.
+    return true
+  }
+  if (!emittedResultIds.has(result.result_id)) {
+    // Was never emitted, which means the batch never set a final status for it.
+    return true
+  }
+  if (emittedResultIds.has(result.result_id) && isTimedOutRetry(result.retries, result.max_retries, result.timed_out)) {
+    // The result ID was already emitted but it used to be non-final result, and it's now a timed out retry.
+    return true
+  }
+
+  return false
+}
+
 const getTestByPublicId = (id: string, tests: Test[]): Test => tests.find((t) => t.public_id === id)!
 
 const getResultIds = (results: ResultInBatch[]): string[] => excludeSkipped(results).map((r) => r.result_id)