From f7bfde0bcc578883bdbfbbd5e61f168ac4b4bdd6 Mon Sep 17 00:00:00 2001 From: Sam Ainsworth Date: Wed, 3 Dec 2025 15:11:34 +0000 Subject: [PATCH 1/3] fix: Added gentler 1.5x ramp to retries with a cap, plus fast fail for unrecoverable response codes --- tests/playwright-tests/src/api/apiHelper.ts | 22 +++++++++++++++++---- tests/playwright-tests/src/config/env.ts | 4 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/playwright-tests/src/api/apiHelper.ts b/tests/playwright-tests/src/api/apiHelper.ts index 44c36b6296..abd4b5f0d2 100644 --- a/tests/playwright-tests/src/api/apiHelper.ts +++ b/tests/playwright-tests/src/api/apiHelper.ts @@ -4,7 +4,7 @@ import { config } from "../config/env"; const apiRetryDefault = Number(config.apiRetry); const initialWaitTimeDefault = Number(config.apiWaitTime) || 2000; -const stepWaitTimeDefault = Number(config.apiStepMs) || 5000; +const maxWaitTimeDefault = Number(config.apiMaxWaitMs) || 5000; const endpointCohortDistributionDataService = config.endpointCohortDistributionDataService; const endpointParticipantManagementDataService = config.endpointParticipantManagementDataService; const endpointExceptionManagementDataService = config.endpointExceptionManagementDataService; @@ -26,7 +26,7 @@ let response: APIResponse; export async function validateApiResponse( validationJson: any, request: any, - options?: { retries?: number; initialWaitMs?: number; stepMs?: number } + options?: { retries?: number; initialWaitMs?: number; maxWaitMs?: number } ): Promise<{ status: boolean; errorTrace?: any }> { let status = false; let endpoint = ""; @@ -34,7 +34,7 @@ export async function validateApiResponse( const maxAttempts = Math.max(1, options?.retries ?? apiRetryDefault); let waitTime = Math.max(0, options?.initialWaitMs ?? initialWaitTimeDefault); - const stepMs = Math.max(0, options?.stepMs ?? stepWaitTimeDefault); + const maxWaitTime = Math.max(0, options?.maxWaitMs ?? maxWaitTimeDefault); for (let attempt = 1; attempt <= maxAttempts; attempt++) { if (status) break; @@ -82,6 +82,19 @@ export async function validateApiResponse( if (response?.status?.() === 204) { console.info(`ℹ️\t Status 204: No data found in the table using endpoint ${endpoint}`); } + + // Fail fast for 5xx server errors - these won't resolve with retries + const statusCode = response?.status?.(); + if (statusCode && statusCode >= 500 && statusCode < 600) { + console.error(`❌ Server error ${statusCode} detected - failing fast without retries`); + break; // Exit retry loop immediately + } + + // Fail fast for 400 Bad Request - indicates a problem with the test data/request + if (statusCode === 400) { + console.error(`❌ Bad Request (400) detected - failing fast without retries`); + break; // Exit retry loop immediately + } } if (attempt < maxAttempts && !status) { @@ -90,7 +103,8 @@ export async function validateApiResponse( if (waitTime > 0) { await new Promise((resolve) => setTimeout(resolve, waitTime)); } - waitTime += stepMs; + // Capped exponential backoff: multiply by 1.5x, max configured wait time + waitTime = Math.min(Math.round(waitTime * 1.5), maxWaitTime); } } return { status, errorTrace }; diff --git a/tests/playwright-tests/src/config/env.ts b/tests/playwright-tests/src/config/env.ts index 960288e981..d2624a3517 100644 --- a/tests/playwright-tests/src/config/env.ts +++ b/tests/playwright-tests/src/config/env.ts @@ -74,8 +74,8 @@ export const config = { e2eTestFilesPath: 'e2e/testFiles', apiTestFilesPath: 'api/testFiles', apiRetry: Number(process.env.API_RETRY ?? 8), - apiWaitTime: Number(process.env.API_WAIT_MS ?? 5000), - apiStepMs: Number(process.env.API_STEP_MS ?? 5000), + apiWaitTime: Number(process.env.API_WAIT_MS ?? 1000), + apiMaxWaitMs: Number(process.env.API_MAX_WAIT_MS ?? 5000), nhsNumberKey: 'NHSNumber', nhsNumberKeyExceptionDemographic: 'NhsNumber', uniqueKeyCohortDistribution: 'CohortDistributionId', From 789ee8f2c90f6395f3a1e2c0733302db122ec2be Mon Sep 17 00:00:00 2001 From: Sam Ainsworth Date: Wed, 3 Dec 2025 15:28:54 +0000 Subject: [PATCH 2/3] feat: added extra output to logs about retry attempts and time remaining --- tests/playwright-tests/src/api/apiHelper.ts | 27 ++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/playwright-tests/src/api/apiHelper.ts b/tests/playwright-tests/src/api/apiHelper.ts index abd4b5f0d2..66ad073ded 100644 --- a/tests/playwright-tests/src/api/apiHelper.ts +++ b/tests/playwright-tests/src/api/apiHelper.ts @@ -36,6 +36,9 @@ export async function validateApiResponse( let waitTime = Math.max(0, options?.initialWaitMs ?? initialWaitTimeDefault); const maxWaitTime = Math.max(0, options?.maxWaitMs ?? maxWaitTimeDefault); + // Track timing for better diagnostics + const startTime = Date.now(); + for (let attempt = 1; attempt <= maxAttempts; attempt++) { if (status) break; @@ -76,6 +79,17 @@ export async function validateApiResponse( status = await validateFields(apiValidation, matchingObject, nhsNumber, matchingObjects); } } + + // Log success with timing information + if (status) { + const elapsedSecs = getElapsedSeconds(startTime); + if (attempt === 1) { + console.info(`✅ Data found on first attempt (${elapsedSecs}s)`); + } else { + console.info(`✅ Data found on attempt ${attempt}/${maxAttempts} (after ${elapsedSecs}s)`); + } + break; // Exit retry loop on success, to avoid unnecessary comparisons below + } } catch (error) { const errorMsg = `Endpoint: ${endpoint}, Status: ${response?.status?.()}, Error: ${error instanceof Error ? error.stack || error.message : error}`; errorTrace = errorMsg; @@ -99,7 +113,8 @@ export async function validateApiResponse( if (attempt < maxAttempts && !status) { const secs = waitTime > 0 ? Math.round(waitTime / 1000) : 0; - console.info(`🚧 Function processing in progress; will check again using data service ${endpoint} in ${secs} seconds...`); + const elapsedSecs = getElapsedSeconds(startTime); + console.info(`🚧 Retry ${attempt}/${maxAttempts}: will check again using data service ${endpoint} in ${secs} seconds (elapsed: ${elapsedSecs}s)...`); if (waitTime > 0) { await new Promise((resolve) => setTimeout(resolve, waitTime)); } @@ -107,6 +122,13 @@ export async function validateApiResponse( waitTime = Math.min(Math.round(waitTime * 1.5), maxWaitTime); } } + + // Log final status with total elapsed time + const totalElapsedSecs = getElapsedSeconds(startTime); + if (!status) { + console.error(`❌ Validation failed after ${maxAttempts} attempts (total time: ${totalElapsedSecs}s)`); + } + return { status, errorTrace }; } @@ -180,6 +202,9 @@ async function findMatchingObject(endpoint: string, responseBody: any[], apiVali return { matchingObject, nhsNumber, matchingObjects }; } +function getElapsedSeconds(startTime: number): string { + return ((Date.now() - startTime) / 1000).toFixed(1); +} async function validateFields(apiValidation: any, matchingObject: any, nhsNumber: any, matchingObjects: any): Promise { const fieldsToValidate = Object.entries(apiValidation.validations).filter(([key]) => key !== IGNORE_VALIDATION_KEY); From a52c9e8c7c9b3c37352e9f91051438a6dd1b7854 Mon Sep 17 00:00:00 2001 From: Sam Ainsworth Date: Thu, 4 Dec 2025 10:34:23 +0000 Subject: [PATCH 3/3] fix: avoid if/else antipattern based on suggestion --- tests/playwright-tests/src/api/apiHelper.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/playwright-tests/src/api/apiHelper.ts b/tests/playwright-tests/src/api/apiHelper.ts index 66ad073ded..9aacae8e3e 100644 --- a/tests/playwright-tests/src/api/apiHelper.ts +++ b/tests/playwright-tests/src/api/apiHelper.ts @@ -80,15 +80,12 @@ export async function validateApiResponse( } } - // Log success with timing information + // Log success and exit retry loop if (status) { const elapsedSecs = getElapsedSeconds(startTime); - if (attempt === 1) { - console.info(`✅ Data found on first attempt (${elapsedSecs}s)`); - } else { - console.info(`✅ Data found on attempt ${attempt}/${maxAttempts} (after ${elapsedSecs}s)`); - } - break; // Exit retry loop on success, to avoid unnecessary comparisons below + const attemptInfo = attempt === 1 ? 'on first attempt' : `on attempt ${attempt}/${maxAttempts}`; + console.info(`✅ Data found ${attemptInfo} (${elapsedSecs}s)`); + break; } } catch (error) { const errorMsg = `Endpoint: ${endpoint}, Status: ${response?.status?.()}, Error: ${error instanceof Error ? error.stack || error.message : error}`;