Skip to content

Commit 9fd7b3d

Browse files
PavelPashovPavelPashov
committed
refactor(test): improve test scenario reliability and maintainability
1 parent 74da657 commit 9fd7b3d

File tree

7 files changed

+377
-380
lines changed

7 files changed

+377
-380
lines changed

‎packages/client/lib/tests/test-scenario/connection-handoff.e2e.ts‎

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,45 @@
1-
import diagnostics_channel from "node:diagnostics_channel";
21
import { FaultInjectorClient } from "./fault-injector-client";
32
import {
3+
createTestClient,
44
getDatabaseConfig,
55
getDatabaseConfigFromEnv,
66
getEnvConfig,
77
RedisConnectionConfig,
88
} from "./test-scenario.util";
99
import { createClient } from "../../..";
10-
import { DiagnosticsEvent } from "../../client/enterprise-maintenance-manager";
1110
import { before } from "mocha";
1211
import { spy } from "sinon";
1312
import assert from "node:assert";
14-
import { TestCommandRunner } from "./test-command-runner";
1513
import net from "node:net";
1614

1715
describe("Connection Handoff", () => {
18-
const diagnosticsLog: DiagnosticsEvent[] = [];
19-
20-
const onMessageHandler = (message: unknown) => {
21-
diagnosticsLog.push(message as DiagnosticsEvent);
22-
};
23-
2416
let clientConfig: RedisConnectionConfig;
25-
let client: ReturnType<typeof createClient<any, any, any, 3>>;
17+
let client: ReturnType<typeof createClient<any, any, any, any>>;
2618
let faultInjectorClient: FaultInjectorClient;
2719
let connectSpy = spy(net, "createConnection");
2820

2921
before(() => {
3022
const envConfig = getEnvConfig();
3123
const redisConfig = getDatabaseConfigFromEnv(
32-
envConfig.redisEndpointsConfigPath,
24+
envConfig.redisEndpointsConfigPath
3325
);
3426

3527
faultInjectorClient = new FaultInjectorClient(envConfig.faultInjectorUrl);
3628
clientConfig = getDatabaseConfig(redisConfig);
3729
});
3830

3931
beforeEach(async () => {
40-
diagnosticsLog.length = 0;
41-
diagnostics_channel.subscribe("redis.maintenance", onMessageHandler);
42-
4332
connectSpy.resetHistory();
4433

45-
client = createClient({
46-
socket: {
47-
host: clientConfig.host,
48-
port: clientConfig.port,
49-
...(clientConfig.tls === true ? { tls: true } : {}),
50-
},
51-
password: clientConfig.password,
52-
username: clientConfig.username,
53-
RESP: 3,
54-
maintPushNotifications: "auto",
55-
maintMovingEndpointType: "external-ip",
56-
maintRelaxedCommandTimeout: 10000,
57-
maintRelaxedSocketTimeout: 10000,
58-
});
34+
client = await createTestClient(clientConfig);
5935

60-
client.on("error", (err: Error) => {
61-
throw new Error(`Client error: ${err.message}`);
62-
});
63-
64-
await client.connect();
6536
await client.flushAll();
6637
});
6738

6839
afterEach(() => {
69-
diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler);
70-
client.destroy();
40+
if (client && client.isOpen) {
41+
client.destroy();
42+
}
7143
});
7244

7345
describe("New Connection Establishment", () => {
@@ -80,11 +52,8 @@ describe("Connection Handoff", () => {
8052
clusterIndex: 0,
8153
});
8254

83-
const lowTimeoutWaitPromise = faultInjectorClient.waitForAction(
84-
lowTimeoutBindAndMigrateActionId,
85-
);
55+
await faultInjectorClient.waitForAction(lowTimeoutBindAndMigrateActionId);
8656

87-
await lowTimeoutWaitPromise;
8857
assert.equal(connectSpy.callCount, 2);
8958
});
9059
});
@@ -108,19 +77,13 @@ describe("Connection Handoff", () => {
10877
clusterIndex: 0,
10978
});
11079

111-
const workloadPromise = faultInjectorClient.waitForAction(action_id);
112-
113-
const commandPromises =
114-
await TestCommandRunner.fireCommandsUntilStopSignal(
115-
client,
116-
workloadPromise,
117-
);
80+
await faultInjectorClient.waitForAction(action_id);
11881

119-
const rejected = (
120-
await Promise.all(commandPromises.commandPromises)
121-
).filter((result) => result.status === "rejected");
82+
const currentTime = Date.now().toString();
83+
await client.set("key", currentTime);
84+
const result = await client.get("key");
12285

123-
assert.ok(rejected.length === 0);
86+
assert.strictEqual(result, currentTime);
12487
});
12588
});
12689
});

‎packages/client/lib/tests/test-scenario/fault-injector-client.ts‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class FaultInjectorClient {
4747
* @param action The action request to trigger
4848
* @throws {Error} When the HTTP request fails or response cannot be parsed as JSON
4949
*/
50-
public triggerAction<T = unknown>(action: ActionRequest): Promise<T> {
50+
public triggerAction<T extends { action_id: string }>(action: ActionRequest): Promise<T> {
5151
return this.#request<T>("POST", "/action", action);
5252
}
5353

@@ -60,20 +60,6 @@ export class FaultInjectorClient {
6060
return this.#request<T>("GET", `/action/${actionId}`);
6161
}
6262

63-
/**
64-
* Executes an rladmin command.
65-
* @param command The rladmin command to execute
66-
* @param bdbId Optional database ID to target
67-
* @throws {Error} When the HTTP request fails or response cannot be parsed as JSON
68-
*/
69-
public executeRladminCommand<T = unknown>(
70-
command: string,
71-
bdbId?: string
72-
): Promise<T> {
73-
const cmd = bdbId ? `rladmin -b ${bdbId} ${command}` : `rladmin ${command}`;
74-
return this.#request<T>("POST", "/rladmin", cmd);
75-
}
76-
7763
/**
7864
* Waits for an action to complete.
7965
* @param actionId The ID of the action to wait for

‎packages/client/lib/tests/test-scenario/push-notification.e2e.ts‎

Lines changed: 90 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import assert from "node:assert";
22
import diagnostics_channel from "node:diagnostics_channel";
33
import { FaultInjectorClient } from "./fault-injector-client";
44
import {
5+
createTestClient,
56
getDatabaseConfig,
67
getDatabaseConfigFromEnv,
78
getEnvConfig,
@@ -12,14 +13,21 @@ import { DiagnosticsEvent } from "../../client/enterprise-maintenance-manager";
1213
import { before } from "mocha";
1314

1415
describe("Push Notifications", () => {
15-
const diagnosticsLog: DiagnosticsEvent[] = [];
16-
17-
const onMessageHandler = (message: unknown) => {
18-
diagnosticsLog.push(message as DiagnosticsEvent);
16+
const createNotificationMessageHandler = (
17+
result: Record<DiagnosticsEvent["type"], number>,
18+
notifications: Array<DiagnosticsEvent["type"]>
19+
) => {
20+
return (message: unknown) => {
21+
if (notifications.includes((message as DiagnosticsEvent).type)) {
22+
const event = message as DiagnosticsEvent;
23+
result[event.type] = (result[event.type] ?? 0) + 1;
24+
}
25+
};
1926
};
2027

28+
let onMessageHandler: ReturnType<typeof createNotificationMessageHandler>;
2129
let clientConfig: RedisConnectionConfig;
22-
let client: ReturnType<typeof createClient<any, any, any, 3>>;
30+
let client: ReturnType<typeof createClient<any, any, any, any>>;
2331
let faultInjectorClient: FaultInjectorClient;
2432

2533
before(() => {
@@ -33,62 +41,97 @@ describe("Push Notifications", () => {
3341
});
3442

3543
beforeEach(async () => {
36-
diagnosticsLog.length = 0;
37-
diagnostics_channel.subscribe("redis.maintenance", onMessageHandler);
44+
client = await createTestClient(clientConfig);
3845

39-
client = createClient({
40-
socket: {
41-
host: clientConfig.host,
42-
port: clientConfig.port,
43-
...(clientConfig.tls === true ? { tls: true } : {}),
44-
},
45-
password: clientConfig.password,
46-
username: clientConfig.username,
47-
RESP: 3,
48-
maintPushNotifications: "auto",
49-
maintMovingEndpointType: "external-ip",
50-
maintRelaxedCommandTimeout: 10000,
51-
maintRelaxedSocketTimeout: 10000,
52-
});
53-
54-
client.on("error", (err: Error) => {
55-
throw new Error(`Client error: ${err.message}`);
56-
});
57-
58-
await client.connect();
46+
await client.flushAll();
5947
});
6048

6149
afterEach(() => {
62-
diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler);
63-
client.destroy();
50+
if (onMessageHandler!) {
51+
diagnostics_channel.unsubscribe("redis.maintenance", onMessageHandler);
52+
}
53+
54+
if (client && client.isOpen) {
55+
client.destroy();
56+
}
6457
});
6558

6659
it("should receive MOVING, MIGRATING, and MIGRATED push notifications", async () => {
67-
const { action_id: migrateActionId } =
68-
await faultInjectorClient.triggerAction<{ action_id: string }>({
69-
type: "migrate",
70-
parameters: {
71-
cluster_index: "0",
72-
},
60+
const notifications: Array<DiagnosticsEvent["type"]> = [
61+
"MOVING",
62+
"MIGRATING",
63+
"MIGRATED",
64+
];
65+
66+
const diagnosticsMap: Record<DiagnosticsEvent["type"], number> = {};
67+
68+
onMessageHandler = createNotificationMessageHandler(
69+
diagnosticsMap,
70+
notifications
71+
);
72+
73+
diagnostics_channel.subscribe("redis.maintenance", onMessageHandler);
74+
75+
const { action_id: bindAndMigrateActionId } =
76+
await faultInjectorClient.migrateAndBindAction({
77+
bdbId: clientConfig.bdbId,
78+
clusterIndex: 0,
7379
});
7480

75-
await faultInjectorClient.waitForAction(migrateActionId);
81+
await faultInjectorClient.waitForAction(bindAndMigrateActionId);
7682

77-
const { action_id: bindActionId } =
78-
await faultInjectorClient.triggerAction<{ action_id: string }>({
79-
type: "bind",
83+
assert.strictEqual(
84+
diagnosticsMap.MOVING,
85+
1,
86+
"Should have received exactly one MOVING notification"
87+
);
88+
assert.strictEqual(
89+
diagnosticsMap.MIGRATING,
90+
1,
91+
"Should have received exactly one MIGRATING notification"
92+
);
93+
assert.strictEqual(
94+
diagnosticsMap.MIGRATED,
95+
1,
96+
"Should have received exactly one MIGRATED notification"
97+
);
98+
});
99+
100+
it("should receive FAILING_OVER and FAILED_OVER push notifications", async () => {
101+
const notifications: Array<DiagnosticsEvent["type"]> = [
102+
"FAILING_OVER",
103+
"FAILED_OVER",
104+
];
105+
106+
const diagnosticsMap: Record<DiagnosticsEvent["type"], number> = {};
107+
108+
onMessageHandler = createNotificationMessageHandler(
109+
diagnosticsMap,
110+
notifications
111+
);
112+
113+
diagnostics_channel.subscribe("redis.maintenance", onMessageHandler);
114+
115+
const { action_id: failoverActionId } =
116+
await faultInjectorClient.triggerAction({
117+
type: "failover",
80118
parameters: {
81-
cluster_index: "0",
82-
bdb_id: `${clientConfig.bdbId}`,
119+
bdb_id: clientConfig.bdbId.toString(),
120+
cluster_index: 0,
83121
},
84122
});
85123

86-
await faultInjectorClient.waitForAction(bindActionId);
124+
await faultInjectorClient.waitForAction(failoverActionId);
87125

88-
const pushNotificationLogs = diagnosticsLog.filter((log) => {
89-
return ["MOVING", "MIGRATING", "MIGRATED"].includes(log?.type);
90-
});
91-
92-
assert.strictEqual(pushNotificationLogs.length, 3);
126+
assert.strictEqual(
127+
diagnosticsMap.FAILING_OVER,
128+
1,
129+
"Should have received exactly one FAILING_OVER notification"
130+
);
131+
assert.strictEqual(
132+
diagnosticsMap.FAILED_OVER,
133+
1,
134+
"Should have received exactly one FAILED_OVER notification"
135+
);
93136
});
94137
});

0 commit comments

Comments
 (0)