refactor(test): improve test scenario reliability and maintainability

packages/client/lib/client/enterprise-maintenance-manager.ts

@@ -51,9 +51,10 @@ interface Client {
   _pause: () => void;
   _unpause: () => void;
   _maintenanceUpdate: (update: MaintenanceUpdate) => void;
-  duplicate: (options: RedisClientOptions) => Client;
+  duplicate: () => Client;
   connect: () => Promise<Client>;
   destroy: () => void;
+  on: (event: string, callback: (value: unknown) => void) => void;
 }
 
 export default class EnterpriseMaintenanceManager {
@@ -211,21 +212,25 @@ export default class EnterpriseMaintenanceManager {
     dbgMaintenance("Creating new tmp client");
     let start = performance.now();
 
-    const tmpOptions = this.#options;
     // If the URL is provided, it takes precedense
-    if(tmpOptions.url) {
-      const u = new URL(tmpOptions.url);
+    // the options object could just be mutated
+    if(this.#options.url) {
+      const u = new URL(this.#options.url);
       u.hostname = host;
       u.port = String(port);
-      tmpOptions.url = u.toString();
+      this.#options.url = u.toString();
     } else {
-      tmpOptions.socket = {
-        ...tmpOptions.socket,
+      this.#options.socket = {
+        ...this.#options.socket,
         host,
         port
       }
     }
-    const tmpClient = this.#client.duplicate(tmpOptions);
+    const tmpClient = this.#client.duplicate();
+    tmpClient.on('error', (error: unknown) => {
+      //We dont know how to handle tmp client errors
+      dbgMaintenance(`[ERR]`, error)
+    });
     dbgMaintenance(`Tmp client created in ${( performance.now() - start ).toFixed(2)}ms`);
     dbgMaintenance(
       `Set timeout for tmp client to ${this.#options.maintRelaxedSocketTimeout}`,

packages/client/lib/tests/test-scenario/configuration.e2e.ts

@@ -0,0 +1,155 @@
+import assert from "node:assert";
+import diagnostics_channel from "node:diagnostics_channel";
+import { DiagnosticsEvent } from "../../client/enterprise-maintenance-manager";
+
+import {
+  RedisConnectionConfig,
+  createTestClient,
+  getDatabaseConfig,
+  getDatabaseConfigFromEnv,
+  getEnvConfig,
+} from "./test-scenario.util";
+import { createClient } from "../../..";
+import { FaultInjectorClient } from "./fault-injector-client";
+import { MovingEndpointType } from "../../../dist/lib/client/enterprise-maintenance-manager";
+import { RedisTcpSocketOptions } from "../../client/socket";
+
+describe("Parameter Configuration", () => {
+  describe("Handshake with endpoint type", () => {
+    let clientConfig: RedisConnectionConfig;
+    let client: ReturnType<typeof createClient<any, any, any, any>>;
+    let faultInjectorClient: FaultInjectorClient;
+    let log: DiagnosticsEvent[] = [];
+
+    before(() => {
+      const envConfig = getEnvConfig();
+      const redisConfig = getDatabaseConfigFromEnv(
+        envConfig.redisEndpointsConfigPath,
+      );
+
+      faultInjectorClient = new FaultInjectorClient(envConfig.faultInjectorUrl);
+      clientConfig = getDatabaseConfig(redisConfig);
+
+      diagnostics_channel.subscribe("redis.maintenance", (event) => {
+        log.push(event as DiagnosticsEvent);
+      });
+    });
+
+    beforeEach(() => {
+      log.length = 0;
+    });
+
+    afterEach(async () => {
+      if (client && client.isOpen) {
+        await client.flushAll();
+        client.destroy();
+      }
+    });
+
+    const endpoints: MovingEndpointType[] = [
+      "auto",
+      // "internal-ip",
+      // "internal-fqdn",
+      "external-ip",
+      "external-fqdn",
+      "none",
+    ];
+
+    for (const endpointType of endpoints) {
+      it(`should request \`${endpointType}\` movingEndpointType and receive it`, async () => {
+        try {
+          client = await createTestClient(clientConfig, {
+            maintMovingEndpointType: endpointType,
+          });
+          client.on('error', () => {})
+
+          //need to copy those because they will be mutated later
+          const oldOptions = JSON.parse(JSON.stringify(client.options));
+          assert.ok(oldOptions);
+
+          const { action_id } = await faultInjectorClient.migrateAndBindAction({
+            bdbId: clientConfig.bdbId,
+            clusterIndex: 0,
+          });
+
+          await faultInjectorClient.waitForAction(action_id);
+
+          const movingEvent = log.find((event) => event.type === "MOVING");
+          assert(!!movingEvent, "Didnt receive moving PN");
+
+          let endpoint: string | undefined;
+          try {
+            //@ts-ignore
+            endpoint = movingEvent.data.push[3];
+          } catch (err) {
+            assert(
+              false,
+              `couldnt get endpoint from event ${JSON.stringify(movingEvent)}`,
+            );
+          }
+
+          assert(endpoint !== undefined, "no endpoint");
+
+          const newOptions = client.options;
+          assert.ok(newOptions);
+
+          if (oldOptions?.url) {
+            if (endpointType === "none") {
+              assert.equal(
+                newOptions!.url,
+                oldOptions.url,
+                "For movingEndpointTpe 'none', we expect old and new url to be the same",
+              );
+            } else {
+              assert.equal(
+                newOptions.url,
+                endpoint,
+                "Expected what came through the wire to be set in the new client",
+              );
+              assert.notEqual(
+                newOptions!.url,
+                oldOptions.url,
+                `For movingEndpointTpe ${endpointType}, we expect old and new url to be different`,
+              );
+            }
+          } else {
+            const oldSocket = oldOptions.socket as RedisTcpSocketOptions;
+            const newSocket = newOptions.socket as RedisTcpSocketOptions;
+            assert.ok(oldSocket);
+            assert.ok(newSocket);
+
+            if (endpointType === "none") {
+              assert.equal(
+                newSocket.host,
+                oldSocket.host,
+                "For movingEndpointTpe 'none', we expect old and new host to be the same",
+              );
+            } else {
+              assert.equal(
+                newSocket.host + ":" + newSocket.port,
+                endpoint,
+                "Expected what came through the wire to be set in the new client",
+              );
+              assert.notEqual(
+                newSocket.host,
+                oldSocket.host,
+                `For movingEndpointTpe ${endpointType}, we expect old and new host to be different`,
+              );
+            }
+          }
+        } catch (error: any) {
+          console.log('endpointType', endpointType);
+          console.log('caught error', error);
+          if (
+            endpointType === "internal-fqdn" ||
+            endpointType === "internal-ip"
+          ) {
+            // errors are expected here, because we cannot connect to internal endpoints unless we are deployed in the same place as the server
+          } else {
+            assert(false, error);
+          }
+        }
+      });
+    }
+  });
+});