ci: fixing contract test breaking due to async hooks

packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts

@@ -190,9 +190,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {
   }
 
   if (options.hooks) {
-    cf.hooks = options.hooks.hooks.map(
-      (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),
-    );
+    cf.hooks = TestHook.forClient(options.hooks.hooks);
   }
 
   cf.fetchGoals = false;

packages/sdk/browser/contract-tests/suppressions.txt

@@ -15,6 +15,4 @@ tags/stream requests/{"applicationId":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKL
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":null}
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":""}
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345678"}
-tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"}
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order
+tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"}

packages/sdk/electron/contract-tests/entity/src/ClientEntity.ts

@@ -87,9 +87,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {
   }
 
   if (options.hooks) {
-    cf.hooks = options.hooks.hooks.map(
-      (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),
-    );
+    cf.hooks = TestHook.forClient(options.hooks.hooks);
   }
 
   if (options.tls) {

packages/sdk/electron/contract-tests/suppressions.txt

@@ -1,2 +0,0 @@
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order

packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts

@@ -77,9 +77,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {
   }
 
   if (options.hooks) {
-    cf.hooks = options.hooks.hooks.map(
-      (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),
-    );
+    cf.hooks = TestHook.forClient(options.hooks.hooks);
   }
 
   return cf;

packages/sdk/react/contract-tests/app/ClientEntity.ts

@@ -70,9 +70,7 @@ export function makeSdkConfig(options: SDKConfigParams, tag: string): LDOptions
   }
 
   if (options.hooks) {
-    cf.hooks = options.hooks.hooks.map(
-      (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),
-    );
+    cf.hooks = TestHook.forClient(options.hooks.hooks);
   }
 
   cf.fetchGoals = false;

packages/sdk/react/contract-tests/suppressions.txt

@@ -16,5 +16,3 @@ tags/stream requests/{"applicationId":"_________________________________________
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":""}
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345678"}
 tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"}
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order

packages/sdk/server-node/contract-tests/src/sdkClientEntity.ts

@@ -88,9 +88,7 @@ export function makeSdkConfig(options: ServerSDKConfigParams, tag: string): LDOp
   }
 
   if (options.hooks) {
-    cf.hooks = options.hooks.hooks.map(
-      (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),
-    );
+    cf.hooks = TestHook.forClient(options.hooks.hooks);
   }
 
   if (options.wrapper) {

packages/sdk/server-node/contract-tests/testharness-suppressions-fdv2.txt

@@ -11,5 +11,3 @@ streaming/fdv2/reconnection state management/replaces previously known state
 streaming/fdv2/reconnection state management/updates previously known state
 streaming/fdv2/ignores model version
 streaming/fdv2/can discard partial events on errors
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order

packages/sdk/server-node/contract-tests/testharness-suppressions.txt

@@ -3,6 +3,4 @@ streaming/validation/drop and reconnect if stream event has well-formed JSON not
 streaming/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has no trailing slash/GET
 streaming/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET
 polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has no trailing slash/GET
-polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order
+polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET

packages/shared/sdk-client/__tests__/hooks/HookRunner.test.ts

@@ -421,6 +421,6 @@ describe('given a hook runner and test hook', () => {
     expect(afterIdentifyOrder).toEqual(['c', 'b', 'a']);
 
     // Verify track hooks order
-    expect(afterTrackOrder).toEqual(['c', 'b', 'a']);
+    expect(afterTrackOrder).toEqual(['a', 'b', 'c']);
   });
 });

packages/shared/sdk-client/src/HookRunner.ts

@@ -117,9 +117,7 @@ function executeAfterIdentify(
 }
 
 function executeAfterTrack(logger: LDLogger, hooks: Hook[], hookContext: TrackSeriesContext) {
-  // This iterates in reverse, versus reversing a shallow copy of the hooks,
-  // for efficiency.
-  for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) {
+  for (let hookIndex = 0; hookIndex < hooks.length; hookIndex += 1) {
     const hook = hooks[hookIndex];
     tryExecuteStage(
       logger,

packages/tooling/contract-test-utils/src/client-side/TestHook.ts

@@ -8,8 +8,15 @@ import {
 } from '@launchdarkly/js-client-sdk-common';
 
 import { BaseTestHook } from '../shared/BaseTestHook.js';
+import { HookPostQueue } from '../shared/HookPostQueue.js';
+import { SDKConfigHookInstance } from '../types/ConfigParams.js';
 
 export default class TestHook extends BaseTestHook implements Hook {
+  static forClient(configs: ReadonlyArray<SDKConfigHookInstance>): TestHook[] {
+    const queue = new HookPostQueue();
+    return configs.map((c) => new TestHook(c.name, c.callbackUri, c.data, c.errors, queue));
+  }
+
   protected async safePost(body: unknown): Promise<void> {
     try {
       await fetch(this.endpoint, {
@@ -52,7 +59,7 @@ export default class TestHook extends BaseTestHook implements Hook {
     if (this.hookErrors?.afterTrack) {
       throw new Error(this.hookErrors.afterTrack);
     }
-    this.safePost({
+    this.enqueuePost({
       trackSeriesContext: hookContext,
       stage: 'afterTrack',
     });

packages/tooling/contract-test-utils/src/server-side/TestHook.ts

@@ -1,8 +1,15 @@
 import { integrations, LDEvaluationDetail } from '@launchdarkly/js-server-sdk-common';
 
 import { BaseTestHook } from '../shared/BaseTestHook.js';
+import { HookPostQueue } from '../shared/HookPostQueue.js';
+import { SDKConfigHookInstance } from '../types/ConfigParams.js';
 
 export default class TestHook extends BaseTestHook implements integrations.Hook {
+  static forClient(configs: ReadonlyArray<SDKConfigHookInstance>): TestHook[] {
+    const queue = new HookPostQueue();
+    return configs.map((c) => new TestHook(c.name, c.callbackUri, c.data, c.errors, queue));
+  }
+
   protected async safePost(body: unknown): Promise<void> {
     try {
       await fetch(this.endpoint, {

packages/tooling/contract-test-utils/src/shared/BaseTestHook.ts

@@ -1,20 +1,34 @@
 import { HookData, HookErrors } from '../types/CommandParams.js';
 
+import { HookPostQueue } from './HookPostQueue.js';
+
 export abstract class BaseTestHook {
   protected readonly hookName: string;
   protected readonly endpoint: string;
   protected readonly hookData?: HookData;
   protected readonly hookErrors?: HookErrors;
+  private readonly _postQueue: HookPostQueue;
 
-  constructor(name: string, endpoint: string, data?: HookData, errors?: HookErrors) {
+  constructor(
+    name: string,
+    endpoint: string,
+    data?: HookData,
+    errors?: HookErrors,
+    postQueue?: HookPostQueue,
+  ) {
     this.hookName = name;
     this.endpoint = endpoint;
     this.hookData = data;
     this.hookErrors = errors;
+    this._postQueue = postQueue ?? new HookPostQueue();
   }
 
   protected abstract safePost(body: unknown): Promise<void>;
 
+  protected enqueuePost(body: unknown): void {
+    this._postQueue.enqueue(() => this.safePost(body));
+  }
+
   getMetadata() {
     return { name: this.hookName };
   }
@@ -26,7 +40,7 @@ export abstract class BaseTestHook {
     if (this.hookErrors?.beforeEvaluation) {
       throw new Error(this.hookErrors.beforeEvaluation);
     }
-    this.safePost({
+    this.enqueuePost({
       evaluationSeriesContext: hookContext,
       evaluationSeriesData: data,
       stage: 'beforeEvaluation',
@@ -42,7 +56,7 @@ export abstract class BaseTestHook {
     if (this.hookErrors?.afterEvaluation) {
       throw new Error(this.hookErrors.afterEvaluation);
     }
-    this.safePost({
+    this.enqueuePost({
       evaluationSeriesContext: hookContext,
       evaluationSeriesData: data,
       stage: 'afterEvaluation',

packages/tooling/contract-test-utils/src/shared/HookPostQueue.ts

@@ -0,0 +1,9 @@
+// The test harness records hook callbacks in network-arrival order, so
+// parallel fetches break ordering assertions. Chain them instead.
+export class HookPostQueue {
+  private _chain: Promise<void> = Promise.resolve();
+
+  enqueue(fn: () => Promise<void>): void {
+    this._chain = this._chain.then(fn).catch(() => {});
+  }
+}