From 5cf21eb7dc0d99d21f4cf25c61ab87e60798ed5c Mon Sep 17 00:00:00 2001 From: corentin Date: Tue, 10 Sep 2024 17:59:39 +0200 Subject: [PATCH] Improve test coverage + refactor --- .../synthetics/__tests__/batch.test.ts | 128 +++++++++++++++++- src/commands/synthetics/batch.ts | 39 ++++-- 2 files changed, 149 insertions(+), 18 deletions(-) diff --git a/src/commands/synthetics/__tests__/batch.test.ts b/src/commands/synthetics/__tests__/batch.test.ts index 13e83fcb2..b5d3f3a3c 100644 --- a/src/commands/synthetics/__tests__/batch.test.ts +++ b/src/commands/synthetics/__tests__/batch.test.ts @@ -1,16 +1,49 @@ -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 = { @@ -18,9 +51,90 @@ describe('getResultsToReport', () => { 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() + 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() + 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() + 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() + const receivedResults = reportReceivedResults(batch, emittedResultIds, mockReporter) + + expect(receivedResults).toStrictEqual([timedOut]) + expect(emittedResultIds).toContain('rid') + expect(mockReporter.resultReceived).toHaveBeenCalledWith(timedOut) }) }) diff --git a/src/commands/synthetics/batch.ts b/src/commands/synthetics/batch.ts index bd5cefd8f..40b3524df 100644 --- a/src/commands/synthetics/batch.ts +++ b/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, reporter: MainReporter) => { +export const reportReceivedResults = (batch: Batch, emittedResultIds: Set, 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, + incompleteResultIds: Set +) => { + 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)