Skip to content

Commit

Permalink
FI-1484 feat: rename getParamsFromUrl to getParamsFromUrlOrThrow
Browse files Browse the repository at this point in the history
fix: test timeout and interrupt timeout during `waitBeforeRetry`
  • Loading branch information
uid11 committed Nov 3, 2024
1 parent 0ae2c51 commit 1b32763
Show file tree
Hide file tree
Showing 19 changed files with 89 additions and 55 deletions.
6 changes: 4 additions & 2 deletions autotests/packs/allTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ const filterTestsIntoPack: FilterTestsIntoPack = ({options}) => options.meta.tes
const userAgent =
'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.35 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.35';

const packTimeoutInMinutes = 5;
const msInMinute = 60_000;
const packTimeoutInMinutes = 5;

const waitBeforeRetryTimeout = 1_000;

/**
* Pack of tests or tasks (pack configuration object).
Expand Down Expand Up @@ -82,7 +84,7 @@ export const pack: Pack = {
userAgent,
viewportHeight: 1080,
viewportWidth: 1920,
waitBeforeRetry: () => 0,
waitBeforeRetry: () => waitBeforeRetryTimeout,
waitForAllRequestsComplete: {
maxIntervalBetweenRequestsInMs: 500,
timeout: 30_000,
Expand Down
2 changes: 1 addition & 1 deletion autotests/routes/apiRoutes/AddUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const pathStart = '/api/users';
* Client API route for adding user-worker.
*/
export class AddUser extends ApiRoute<Params, ApiAddUserRequest, ApiAddUserResponse> {
static override getParamsFromUrl(url: Url): Params {
static override getParamsFromUrlOrThrow(url: Url): Params {
const urlObject = new URL(url);

assertValueIsTrue(
Expand Down
2 changes: 1 addition & 1 deletion autotests/routes/apiRoutes/CreateProduct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class CreateProduct extends ApiRoute<
ApiCreateProductRequest,
ApiCreateProductResponse
> {
static override getParamsFromUrl(url: Url): Params {
static override getParamsFromUrlOrThrow(url: Url): Params {
const urlObject = new URL(url);

assertValueIsTrue(
Expand Down
2 changes: 1 addition & 1 deletion autotests/routes/pageRoutes/Search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Params = Readonly<{searchQuery: string}> | undefined;
* Route of the Search page.
*/
export class Search extends PageRoute<Params> {
static override getParamsFromUrl(url: Url): Params {
static override getParamsFromUrlOrThrow(url: Url): Params {
const {pathname, searchParams} = new URL(url);

assertValueIsTrue(pathname === '/search', 'search route matches on url', {url});
Expand Down
4 changes: 2 additions & 2 deletions autotests/tests/internalTypeTests/mockApiRoute.skip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ void mockApiRoute(Main, apiMockFunction);
// @ts-expect-error: unmockApiRoute require API route as first argument
void unmockApiRoute(Main);

// @ts-expect-error: mockApiRoute require API route with static method getParamsFromUrl
// @ts-expect-error: mockApiRoute require API route with static method getParamsFromUrlOrThrow
void mockApiRoute(CreateDevice, apiMockFunction);

// @ts-expect-error: unmockApiRoute require API route with static method getParamsFromUrl
// @ts-expect-error: unmockApiRoute require API route with static method getParamsFromUrlOrThrow
void unmockApiRoute(CreateDevice);

// ok
Expand Down
4 changes: 2 additions & 2 deletions autotests/tests/internalTypeTests/waitForEvents.skip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void waitForRequestToRoute(AddUser, ({delay}, {requestBody, url}) => {
request.requestBody.job === 'foo' && 'delay' in routeParams && routeParams.delay > 0,
);

// @ts-expect-error: waitForRequestToRoute does not accept routes without `getParamsFromUrl` method
// @ts-expect-error: waitForRequestToRoute does not accept routes without `getParamsFromUrlOrThrow` method
void waitForRequestToRoute(GetUser);

// ok
Expand All @@ -84,5 +84,5 @@ void waitForResponseToRoute(AddUser, ({delay}, {responseBody, request: {requestB
routeParams.delay > 0,
);

// @ts-expect-error: waitForResponseToRoute does not accept routes without `getParamsFromUrl` method
// @ts-expect-error: waitForResponseToRoute does not accept routes without `getParamsFromUrlOrThrow` method
void waitForResponseToRoute(GetUser);
4 changes: 2 additions & 2 deletions autotests/tests/main/exists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ test('exists', {meta: {testId: '1'}, testIdleTimeout: 10_000, testTimeout: 15_00
const searchPage = await assertPage(Search, {searchQuery});

/**
* Do not use the following pageParams and url (by getParamsFromUrl) checks in your code.
* Do not use the following pageParams and url (by getParamsFromUrlOrThrow) checks in your code.
* These are e2ed internal checks. Use `assertPage` instead.
*/
await expect(searchPage.pageParams, 'pageParams is correct after assertPage').eql({
Expand All @@ -86,7 +86,7 @@ test('exists', {meta: {testId: '1'}, testIdleTimeout: 10_000, testTimeout: 15_00

const url = await getDocumentUrl();

await expect(SearchRoute.getParamsFromUrl(url), 'page url has expected params').eql({
await expect(SearchRoute.getParamsFromUrlOrThrow(url), 'page url has expected params').eql({
searchQuery,
});

Expand Down
2 changes: 1 addition & 1 deletion autotests/tests/mockApiRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ test(

const fetchUrl = `https://reqres.in/api/product/${productId}?size=${product.size}` as Url;

const productRouteParams = CreateProductRoute.getParamsFromUrl(fetchUrl);
const productRouteParams = CreateProductRoute.getParamsFromUrlOrThrow(fetchUrl);

const productRouteFromUrl = new CreateProductRoute(productRouteParams);

Expand Down
2 changes: 1 addition & 1 deletion scripts/writePrunedPackageJson.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @file Generates and writes pruned lightweight package.json for npm package.
* @file Generates and writes pruned lightweight `package.json` for npm package.
*/

import {writeFileSync} from 'node:fs';
Expand Down
2 changes: 1 addition & 1 deletion src/Route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export abstract class Route<RouteParams> {
* Returns route params from the passed url.
* @throws {Error} If the route does not match on the url.
*/
static getParamsFromUrl?(url: Url, method?: Method): unknown;
static getParamsFromUrlOrThrow?(url: Url, method?: Method): unknown;

/**
* Returns the url of the route.
Expand Down
4 changes: 2 additions & 2 deletions src/actions/mock/mockApiRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import type {

/**
* Mock API for some API route.
* Applicable only for routes with the `getParamsFromUrl` method.
* Applicable only for routes with the `getParamsFromUrlOrThrow` method.
* The mock is applied to a request that matches the route by url
* (by methods `getParamsFromUrl` and `isMatchUrl`) and by HTTP method (by `getMethod`).
* (by methods `getParamsFromUrlOrThrow` and `isMatchUrl`) and by HTTP method (by `getMethod`).
*/
export const mockApiRoute = async <
RouteParams,
Expand Down
4 changes: 2 additions & 2 deletions src/types/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ export type ApiRouteClassType<
};

/**
* API Route class with static method getParamsFromUrl.
* API Route class with static method getParamsFromUrlOrThrow.
*/
export type ApiRouteClassTypeWithGetParamsFromUrl<
RouteParams = Any,
SomeRequest extends Request = Request,
SomeResponse extends Response = Response,
> = ApiRouteClassType<RouteParams, SomeRequest, SomeResponse> &
Readonly<{
getParamsFromUrl: Exclude<(typeof ApiRoute)['getParamsFromUrl'], undefined>;
getParamsFromUrlOrThrow: Exclude<(typeof ApiRoute)['getParamsFromUrlOrThrow'], undefined>;
}>;
2 changes: 1 addition & 1 deletion src/utils/fullMocks/FullMocksRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {FullMocksRouteParams, Url} from '../../types/internal';
* @internal
*/
export class FullMocksRoute extends ApiRoute<FullMocksRouteParams> {
static override getParamsFromUrl(url: Url): FullMocksRouteParams {
static override getParamsFromUrlOrThrow(url: Url): FullMocksRouteParams {
const {fullMocks: fullMocksConfig} = getFullPackConfig();
const fullMocksState = getFullMocksState();

Expand Down
8 changes: 4 additions & 4 deletions src/utils/getRouteInstanceFromUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ type Return<RouteParams> =

/**
* Get route instance and route params from url and method by route class.
* @throws {Error} If `url` accepted without errors by `getParamsFromUrl`,
* @throws {Error} If `url` accepted without errors by `getParamsFromUrlOrThrow`,
* but not match by `isMatchUrl` method.
* If `url` not accepted by `getParamsFromUrl`, returns `undefined`.
* If `url` not accepted by `getParamsFromUrlOrThrow`, returns `undefined`.
* @internal
*/
export const getRouteInstanceFromUrl = <RouteParams>(
Expand All @@ -22,15 +22,15 @@ export const getRouteInstanceFromUrl = <RouteParams>(
let routeParams: RouteParams | undefined;

try {
routeParams = Route.getParamsFromUrl(url) as RouteParams;
routeParams = Route.getParamsFromUrlOrThrow(url) as RouteParams;
route = new Route(routeParams);
} catch {
return undefined;
}

if (route.isMatchUrl(url) !== true) {
throw new E2edError(
`Inconsistency in "${Route.name}" route: isMatchUrl does not accept url accepted without errors by getParamsFromUrl`,
`Inconsistency in "${Route.name}" route: isMatchUrl does not accept url accepted without errors by getParamsFromUrlOrThrow`,
{route, url},
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/utils/resourceUsage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const availableCpuCount = availableParallelism();
let previousCores = cpus();
let previousCpuUsage = process.cpuUsage();
let previousTimeInMs = Date.now();
let timeoutInterval: NodeJS.Timeout;
let timeoutObject: NodeJS.Timeout;

const logResourceUsage = (): void => {
const cores = cpus();
Expand Down Expand Up @@ -65,15 +65,15 @@ const logResourceUsage = (): void => {
* @internal
*/
export const startResourceUsageReading = (resourceUsageReadingInternal: number): void => {
assertValueIsUndefined(timeoutInterval, 'timeoutInterval in not defined', {
assertValueIsUndefined(timeoutObject, 'timeoutObject in not defined', {
resourceUsageReadingInternal,
});

if (isUiMode) {
return;
}

timeoutInterval = setInterval(logResourceUsage, resourceUsageReadingInternal);
timeoutObject = setInterval(logResourceUsage, resourceUsageReadingInternal);

timeoutInterval.unref();
timeoutObject.unref();
};
2 changes: 1 addition & 1 deletion src/utils/retry/runPackWithRetries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const runPackWithRetries = async (): Promise<void> => {

endE2ed(EndE2edReason.RetriesCycleEnded);
} catch (error) {
generalLog('Caught an error on running testso', {
generalLog('Caught an error on running test', {
error,
retriesState: truncateRetriesStateForLogs(retriesState),
});
Expand Down
6 changes: 2 additions & 4 deletions src/utils/test/beforeTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Options = Readonly<{
testStaticOptions: TestStaticOptions;
}>;

const additionalDurationToPlaywrightTestTimeoutInMs = 500;
const additionToPlaywrightTestTimeout = 500;

/**
* Internal before test hook.
Expand Down Expand Up @@ -60,9 +60,7 @@ export const beforeTest = ({
const testIdleTimeout = options.testIdleTimeout ?? testIdleTimeoutFromConfig;
const testTimeout = options.testTimeout ?? testTimeoutFromConfig;

test.setTimeout(
testTimeout + additionalDurationToPlaywrightTestTimeoutInMs + (beforeRetryTimeout ?? 0),
);
test.setTimeout(testTimeout + additionToPlaywrightTestTimeout + (beforeRetryTimeout ?? 0));

setTestIdleTimeout(testIdleTimeout);
setTestTimeout(testTimeout);
Expand Down
37 changes: 37 additions & 0 deletions src/utils/test/getPreviousRunId.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {assertValueIsTrue} from '../asserts';

import type {RunId} from '../../types/internal';

type Return = Readonly<{previousRunId: RunId | undefined; retryIndex: number}>;

/**
* Get previous `runId` if any, by current `runId`.
* @internal
*/
export const getPreviousRunId = (runId: RunId): Return => {
const indexOfRetryIndex = runId.lastIndexOf('-');

assertValueIsTrue(
indexOfRetryIndex > 0 && indexOfRetryIndex < runId.length - 1,
'runId has dash',
{runId},
);

const retryIndex = Number(runId.slice(indexOfRetryIndex + 1));

assertValueIsTrue(
Number.isInteger(retryIndex) && retryIndex > 0,
'retryIndex from runId is correct',
{runId},
);

const previousRetryIndex = retryIndex - 1;

if (previousRetryIndex < 1) {
return {previousRunId: undefined, retryIndex};
}

const previousRunId = `${runId.slice(0, indexOfRetryIndex)}-${previousRetryIndex}` as RunId;

return {previousRunId, retryIndex};
};
43 changes: 20 additions & 23 deletions src/utils/test/waitBeforeRetry.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import {assertValueIsTrue} from '../asserts';
import {getFullPackConfig} from '../config';
import {getTestRunEventFileName, readEventFromFile} from '../fs';
import {getTestRunEventFileName, readEventFromFile, writeLogEventTime} from '../fs';
import {generalLog} from '../generalLog';
import {getTimeoutPromise} from '../promise';

import {getPreviousRunId} from './getPreviousRunId';

import type {FullTestRun, RunId, TestStaticOptions} from '../../types/internal';

import {test} from '@playwright/test';

const additionToTimeout = 500;

/**
* Waits before running test for some time from pack config (for retries).
* @internal
Expand All @@ -14,30 +19,12 @@ export const waitBeforeRetry = async (
runId: RunId,
testStaticOptions: TestStaticOptions,
): Promise<number | undefined> => {
const indexOfRetryIndex = runId.lastIndexOf('-');

assertValueIsTrue(
indexOfRetryIndex > 0 && indexOfRetryIndex < runId.length - 1,
'runId has dash',
{runId, testStaticOptions},
);

const retryIndex = Number(runId.slice(indexOfRetryIndex + 1));

assertValueIsTrue(
Number.isInteger(retryIndex) && retryIndex > 0,
'retryIndex from runId is correct',
{runId, testStaticOptions},
);
const {previousRunId, retryIndex} = getPreviousRunId(runId);

const previousRetryIndex = retryIndex - 1;

if (previousRetryIndex < 1) {
if (previousRunId === undefined) {
return;
}

const previousRunId = `${runId.slice(0, indexOfRetryIndex)}-${previousRetryIndex}` as RunId;

const fileName = getTestRunEventFileName(previousRunId);
const fileText = await readEventFromFile(fileName);

Expand All @@ -55,7 +42,7 @@ export const waitBeforeRetry = async (
const fullTestRun = JSON.parse(fileText) as FullTestRun;

const {runError, status} = fullTestRun;
const {waitBeforeRetry: waitBeforeRetryFromConfig} = getFullPackConfig();
const {testIdleTimeout, waitBeforeRetry: waitBeforeRetryFromConfig} = getFullPackConfig();

const previousError = runError === undefined ? undefined : String(runError);

Expand All @@ -70,8 +57,18 @@ export const waitBeforeRetry = async (
return;
}

test.setTimeout(timeoutInMs + additionToTimeout);

const timeoutObject = setInterval(() => {
void writeLogEventTime();
}, testIdleTimeout);

timeoutObject.unref();

await getTimeoutPromise(timeoutInMs);

clearInterval(timeoutObject);

return timeoutInMs;
} catch (error) {
generalLog('Caught an error on getting timeout for "before retry" waiting', {
Expand Down

0 comments on commit 1b32763

Please sign in to comment.