tests: Use pollFor logic instead of static wait times in integration tests (#6803)

Author: jdolle

integration-tests/testkit/flow.ts

@@ -39,27 +39,59 @@ export function waitFor(ms: number) {
 
 function pollInternal(
   check: () => Promise<boolean>,
+
+  /** In milliseconds */
   pollFrequency: number,
+
+  /** In milliseconds */
+  maxWait: number,
+
+  /** In milliseconds. A random number between 0 and Jitter is added to the pollFrequency to add some
+   * noise and prevent test cases where exact durations are required.
+   * */
+  jitter: number,
+
   resolve: (value: void | PromiseLike<void>) => void,
   reject: (reason?: any) => void,
+
+  /** In milliseconds */
+  startTime: number = Date.now(),
 ) {
-  setTimeout(async () => {
-    try {
-      const passes = await check();
-      if (passes) {
-        resolve();
-      } else {
-        pollInternal(check, pollFrequency, resolve, reject);
+  setTimeout(
+    async () => {
+      try {
+        const passes = await check();
+        if (passes) {
+          resolve();
+        } else {
+          const waited = Date.now() - startTime;
+          if (waited > maxWait) {
+            reject(new Error(`Polling failed. Condition was not satisfied within ${maxWait}ms`));
+          } else {
+            pollInternal(check, pollFrequency, maxWait, jitter, resolve, reject, startTime);
+          }
+        }
+      } catch (e) {
+        reject(e);
       }
-    } catch (e) {
-      reject(e);
-    }
-  }, pollFrequency);
+    },
+    Math.round(pollFrequency + Math.random() * jitter),
+  );
 }
 
-export function pollFor(check: () => Promise<boolean>, pollFrequency = 500): Promise<void> {
+export function pollFor(
+  check: () => Promise<boolean>,
+  opts?: { pollFrequency?: number; maxWait?: number; jitter?: number },
+): Promise<void> {
   return new Promise((resolve, reject) => {
-    pollInternal(check, pollFrequency, resolve, reject);
+    pollInternal(
+      check,
+      opts?.pollFrequency ?? 500,
+      opts?.maxWait ?? 10_000,
+      opts?.jitter ?? 100,
+      resolve,
+      reject,
+    );
   });
 }
 

integration-tests/testkit/graphql.ts

@@ -66,7 +66,7 @@ export async function execute<TResult, TVariables>(
     rawBody: body,
     status: response.status,
     expectGraphQLErrors() {
-      if (!body.errors?.length) {
+      if (!body?.errors?.length) {
         throw new Error(
           `Expected GraphQL response to have errors, but no errors were found!${detailsDump}`,
         );
@@ -75,7 +75,7 @@ export async function execute<TResult, TVariables>(
       return body.errors!;
     },
     expectNoGraphQLErrors: async () => {
-      if (body.errors?.length) {
+      if (body?.errors?.length) {
         throw new Error(
           `Expected GraphQL response to have no errors, but got ${
             body.errors.length

integration-tests/testkit/seed.ts

@@ -766,6 +766,38 @@ export function initSeed() {
 
                   return pollFor(check);
                 },
+                async waitForRequestsCollected(
+                  n: number,
+                  opts?: {
+                    from?: number;
+                    to?: number;
+                    target?: TargetOverwrite;
+                  },
+                ) {
+                  const from = formatISO(opts?.from ?? subHours(Date.now(), 1));
+                  const to = formatISO(opts?.to ?? Date.now());
+                  const check = async () => {
+                    const statsResult = await readOperationsStats(
+                      {
+                        organizationSlug: organization.slug,
+                        projectSlug: project.slug,
+                        targetSlug: (opts?.target ?? target).slug,
+                        period: {
+                          from,
+                          to,
+                        },
+                      },
+                      ownerToken,
+                    ).then(r => r.expectNoGraphQLErrors());
+                    const totalRequests = statsResult.operationsStats.operations.nodes.reduce(
+                      (total, node) => total + node.count,
+                      0,
+                    );
+                    return totalRequests == n;
+                  };
+
+                  return pollFor(check);
+                },
                 async readOperationsStats(
                   from: string,
                   to: string,

integration-tests/tests/api/app-deployments.spec.ts

@@ -1,6 +1,5 @@
 import { buildASTSchema, parse } from 'graphql';
 import { createLogger } from 'graphql-yoga';
-import { waitFor } from 'testkit/flow';
 import { initSeed } from 'testkit/seed';
 import { getServiceHost } from 'testkit/utils';
 import { createHive } from '@graphql-hive/core';
@@ -1697,7 +1696,8 @@ test('app deployment usage reporting', async () => {
   const { createOrg, ownerToken } = await initSeed().createOwner();
   const { createProject, setFeatureFlag, organization } = await createOrg();
   await setFeatureFlag('appDeployments', true);
-  const { createTargetAccessToken, project, target } = await createProject();
+  const { createTargetAccessToken, project, target, waitForOperationsCollected } =
+    await createProject();
   const token = await createTargetAccessToken({});
 
   const { createAppDeployment } = await execute({
@@ -1806,7 +1806,7 @@ test('app deployment usage reporting', async () => {
     'app-name~app-version~aaa',
   );
 
-  await waitFor(5000);
+  await waitForOperationsCollected(1);
 
   data = await execute({
     document: GetAppDeployment,

integration-tests/tests/api/legacy-auth.spec.ts

@@ -10,9 +10,12 @@ test.concurrent(
     const { createOrg } = await initSeed().createOwner();
     const { inviteAndJoinMember, createProject } = await createOrg();
     await inviteAndJoinMember();
-    const { createTargetAccessToken, readOperationsStats, readOperationBody } = await createProject(
-      ProjectType.Single,
-    );
+    const {
+      createTargetAccessToken,
+      readOperationsStats,
+      readOperationBody,
+      waitForOperationsCollected,
+    } = await createProject(ProjectType.Single);
     const { publishSchema, collectLegacyOperations: collectOperations } =
       await createTargetAccessToken({});
 
@@ -40,7 +43,7 @@ test.concurrent(
 
     expect(collectResult.status).toEqual(200);
 
-    await waitFor(5000);
+    await waitForOperationsCollected(1);
 
     const from = formatISO(subHours(Date.now(), 6));
     const to = formatISO(Date.now());

integration-tests/tests/api/organization/get-started.spec.ts

@@ -1,5 +1,4 @@
 import { ProjectType } from 'testkit/gql/graphql';
-import { waitFor } from '../../../testkit/flow';
 import { initSeed } from '../../../testkit/seed';
 
 test.concurrent(
@@ -23,8 +22,13 @@ test.concurrent(
   async ({ expect }) => {
     const { createOrg } = await initSeed().createOwner();
     const { inviteAndJoinMember, fetchOrganizationInfo, createProject } = await createOrg();
-    const { target, project, createTargetAccessToken, toggleTargetValidation } =
-      await createProject(ProjectType.Single);
+    const {
+      target,
+      project,
+      createTargetAccessToken,
+      toggleTargetValidation,
+      waitForOperationsCollected,
+    } = await createProject(ProjectType.Single);
 
     const { getStarted: steps } = await fetchOrganizationInfo();
     expect(steps?.creatingProject).toBe(true); // modified
@@ -87,7 +91,7 @@ test.concurrent(
         },
       },
     ]);
-    await waitFor(8000);
+    await waitForOperationsCollected(1);
     const { getStarted: steps5 } = await fetchOrganizationInfo();
     expect(steps5?.creatingProject).toBe(true);
     expect(steps5?.publishingSchema).toBe(true);

integration-tests/tests/api/rate-limit/emails.spec.ts

@@ -1,6 +1,6 @@
 import { ProjectType } from 'testkit/gql/graphql';
 import * as emails from '../../../testkit/emails';
-import { updateOrgRateLimit, waitFor } from '../../../testkit/flow';
+import { pollFor, updateOrgRateLimit, waitFor } from '../../../testkit/flow';
 import { initSeed } from '../../../testkit/seed';
 
 function filterEmailsByOrg(orgSlug: string, emails: emails.Email[]) {
@@ -15,7 +15,9 @@ function filterEmailsByOrg(orgSlug: string, emails: emails.Email[]) {
 test('rate limit approaching and reached for organization', async () => {
   const { createOrg, ownerToken, ownerEmail } = await initSeed().createOwner();
   const { createProject, organization } = await createOrg();
-  const { createTargetAccessToken } = await createProject(ProjectType.Single);
+  const { createTargetAccessToken, waitForRequestsCollected } = await createProject(
+    ProjectType.Single,
+  );
 
   await updateOrgRateLimit(
     {
@@ -44,7 +46,13 @@ test('rate limit approaching and reached for organization', async () => {
   const collectResult = await collectOperations(new Array(10).fill(op));
   expect(collectResult.status).toEqual(200);
 
-  await waitFor(8000);
+  await waitForRequestsCollected(10);
+
+  // wait for the rate limit email to send...
+  await pollFor(async () => {
+    let sent = await emails.history();
+    return filterEmailsByOrg(organization.slug, sent)?.length === 1;
+  });
 
   let sent = await emails.history();
   expect(sent).toContainEqual({
@@ -58,7 +66,13 @@ test('rate limit approaching and reached for organization', async () => {
   const collectMoreResult = await collectOperations([op, op]);
   expect(collectMoreResult.status).toEqual(200);
 
-  await waitFor(7000);
+  await waitForRequestsCollected(12);
+
+  // wait for the quota email to send...
+  await pollFor(async () => {
+    let sent = await emails.history();
+    return filterEmailsByOrg(organization.slug, sent)?.length === 2;
+  });
 
   sent = await emails.history();
 
@@ -73,7 +87,8 @@ test('rate limit approaching and reached for organization', async () => {
   const collectEvenMoreResult = await collectOperations([op, op]);
   expect(collectEvenMoreResult.status).toEqual(429);
 
-  await waitFor(5000);
+  // @note we can't poll for any state here because nothing should change. Must wait unfortunately...
+  await waitFor(4_000);
 
   // Nothing new
   sent = await emails.history();

integration-tests/tests/api/schema/unused.spec.ts

@@ -35,7 +35,9 @@ test.concurrent(
   async ({ expect }) => {
     const { createOrg } = await initSeed().createOwner();
     const { createProject } = await createOrg();
-    const { createTargetAccessToken, target } = await createProject(ProjectType.Single);
+    const { createTargetAccessToken, target, waitForOperationsCollected } = await createProject(
+      ProjectType.Single,
+    );
 
     // Create a token with write rights
     const writeToken = await createTargetAccessToken({});
@@ -123,7 +125,7 @@ test.concurrent(
       ],
     });
     expect(collectResult.status).toEqual(200);
-    await waitFor(8000);
+    await waitForOperationsCollected(1);
 
     const secondQuery = await execute({
       document: IntegrationTestsUnusedSchemaQuery,