From 5df13fa7a78dd84ef368f4972280bde3ff4bbf5a Mon Sep 17 00:00:00 2001 From: umair Date: Mon, 20 Apr 2026 20:13:00 +0100 Subject: [PATCH] Replace fixed sleeps with vi.waitFor to fix flaky unit tests These tests used `setTimeout` (either scheduling a callback to fire during a `runCommand`, or a fixed `await new Promise(setTimeout, N)`) to wait for async work to complete before asserting. When the work took longer than the timeout the assertion ran against stale state and the test failed. Switched to `vi.waitFor(() => expect(...))` which polls until the condition holds, removing the race: - logs/connection-lifecycle/subscribe: 5 tests fired the message callback at +50ms from test start, but the command's `channel.subscribe()` was not always captured by then. - interactive-terminal-behavior: two tests slept 100ms after `rl.emit("line", ...)` then asserted on async prompt/close state. - spaces/locations/subscribe: two tests slept 50ms after starting the command before invoking the captured subscribe handler. - hooks/interactive-did-you-mean: slept 30ms after the hook returned before asserting on readline state restoration. --- .../interactive-terminal-behavior.test.ts | 14 +- .../connection-lifecycle/subscribe.test.ts | 187 +++++++++--------- .../spaces/locations/subscribe.test.ts | 49 +++-- .../hooks/interactive-did-you-mean.test.ts | 9 +- 4 files changed, 127 insertions(+), 132 deletions(-) diff --git a/test/unit/commands/interactive-terminal-behavior.test.ts b/test/unit/commands/interactive-terminal-behavior.test.ts index 5c14a7b36..7afcfa6a6 100644 --- a/test/unit/commands/interactive-terminal-behavior.test.ts +++ b/test/unit/commands/interactive-terminal-behavior.test.ts @@ -273,11 +273,10 @@ describe("Interactive Mode - Terminal Behavior Unit Tests", () => { // Simulate command execution rl.emit("line", "invalid command"); - // Wait for async processing - await new Promise((resolve) => setTimeout(resolve, 100)); - // Prompt should be called again after error - expect(promptStub).toHaveBeenCalled(); + await vi.waitFor(() => { + expect(promptStub).toHaveBeenCalled(); + }); }); it("should handle special characters in autocomplete", async () => { @@ -393,11 +392,10 @@ describe("Interactive Mode - Terminal Behavior Unit Tests", () => { // Simulate exit command rl.emit("line", "exit"); - // Wait for async processing - await new Promise((resolve) => setTimeout(resolve, 100)); - // Readline should be closed - expect(closeStub).toHaveBeenCalled(); + await vi.waitFor(() => { + expect(closeStub).toHaveBeenCalled(); + }); await vi.waitFor(() => { expect(exitStub).toHaveBeenCalled(); diff --git a/test/unit/commands/logs/connection-lifecycle/subscribe.test.ts b/test/unit/commands/logs/connection-lifecycle/subscribe.test.ts index 538954f70..642ba151b 100644 --- a/test/unit/commands/logs/connection-lifecycle/subscribe.test.ts +++ b/test/unit/commands/logs/connection-lifecycle/subscribe.test.ts @@ -100,7 +100,6 @@ describe("LogsConnectionLifecycleSubscribe", function () { const mock = getMockAblyRealtime(); const channel = mock.channels._getChannel("[meta]connection.lifecycle"); - // Capture the subscription callback let messageCallback: ((message: unknown) => void) | null = null; channel.subscribe.mockImplementation( (callback: (message: unknown) => void) => { @@ -108,29 +107,30 @@ describe("LogsConnectionLifecycleSubscribe", function () { }, ); - // Simulate receiving a message - setTimeout(() => { - if (messageCallback) { - messageCallback({ - name: "connection.opened", - data: { - connectionId: "test-connection-123", - transport: "websocket", - ipAddress: "192.168.1.1", - }, - timestamp: Date.now(), - clientId: "test-client", - connectionId: "test-connection-123", - id: "msg-123", - }); - } - }, 50); - - const { stdout } = await runCommand( + const commandPromise = runCommand( ["logs:connection-lifecycle:subscribe"], import.meta.url, ); + await vi.waitFor(() => { + expect(messageCallback).not.toBeNull(); + }); + + messageCallback!({ + name: "connection.opened", + data: { + connectionId: "test-connection-123", + transport: "websocket", + ipAddress: "192.168.1.1", + }, + timestamp: Date.now(), + clientId: "test-client", + connectionId: "test-connection-123", + id: "msg-123", + }); + + const { stdout } = await commandPromise; + expect(stdout).toContain("connection.opened"); }); @@ -138,7 +138,6 @@ describe("LogsConnectionLifecycleSubscribe", function () { const mock = getMockAblyRealtime(); const channel = mock.channels._getChannel("[meta]connection.lifecycle"); - // Capture the subscription callback let messageCallback: ((message: unknown) => void) | null = null; channel.subscribe.mockImplementation( (callback: (message: unknown) => void) => { @@ -146,26 +145,26 @@ describe("LogsConnectionLifecycleSubscribe", function () { }, ); - // Simulate receiving a message - setTimeout(() => { - if (messageCallback) { - messageCallback({ - name: "connection.opened", - data: { connectionId: "test-connection-123" }, - timestamp: Date.now(), - clientId: "test-client", - connectionId: "test-connection-123", - id: "msg-123", - }); - } - }, 50); - - const { stdout } = await runCommand( + const commandPromise = runCommand( ["logs:connection-lifecycle:subscribe", "--json"], import.meta.url, ); - // Verify JSON output - the output contains the event name in JSON format + await vi.waitFor(() => { + expect(messageCallback).not.toBeNull(); + }); + + messageCallback!({ + name: "connection.opened", + data: { connectionId: "test-connection-123" }, + timestamp: Date.now(), + clientId: "test-client", + connectionId: "test-connection-123", + id: "msg-123", + }); + + const { stdout } = await commandPromise; + expect(stdout).toContain("connection.opened"); }); @@ -173,7 +172,6 @@ describe("LogsConnectionLifecycleSubscribe", function () { const mock = getMockAblyRealtime(); const channel = mock.channels._getChannel("[meta]connection.lifecycle"); - // Capture the subscription callback let messageCallback: ((message: unknown) => void) | null = null; channel.subscribe.mockImplementation( (callback: (message: unknown) => void) => { @@ -181,28 +179,29 @@ describe("LogsConnectionLifecycleSubscribe", function () { }, ); - // Simulate receiving a connection state change event - setTimeout(() => { - if (messageCallback) { - messageCallback({ - name: "connection.connected", - data: { - connectionId: "test-connection-456", - transport: "websocket", - }, - timestamp: Date.now(), - clientId: "test-client", - connectionId: "test-connection-456", - id: "msg-state-change", - }); - } - }, 50); - - const { stdout } = await runCommand( + const commandPromise = runCommand( ["logs:connection-lifecycle:subscribe"], import.meta.url, ); + await vi.waitFor(() => { + expect(messageCallback).not.toBeNull(); + }); + + messageCallback!({ + name: "connection.connected", + data: { + connectionId: "test-connection-456", + transport: "websocket", + }, + timestamp: Date.now(), + clientId: "test-client", + connectionId: "test-connection-456", + id: "msg-state-change", + }); + + const { stdout } = await commandPromise; + expect(channel.subscribe).toHaveBeenCalled(); expect(stdout).toContain("connection.connected"); }); @@ -211,7 +210,6 @@ describe("LogsConnectionLifecycleSubscribe", function () { const mock = getMockAblyRealtime(); const channel = mock.channels._getChannel("[meta]connection.lifecycle"); - // Capture the subscription callback let messageCallback: ((message: unknown) => void) | null = null; channel.subscribe.mockImplementation( (callback: (message: unknown) => void) => { @@ -219,28 +217,29 @@ describe("LogsConnectionLifecycleSubscribe", function () { }, ); - // Simulate receiving different event types - setTimeout(() => { - if (messageCallback) { - messageCallback({ - name: "connection.closed", - data: { - connectionId: "test-connection-123", - reason: "client closed", - }, - timestamp: Date.now(), - clientId: "test-client", - connectionId: "test-connection-123", - id: "msg-456", - }); - } - }, 50); - - const { stdout } = await runCommand( + const commandPromise = runCommand( ["logs:connection-lifecycle:subscribe"], import.meta.url, ); + await vi.waitFor(() => { + expect(messageCallback).not.toBeNull(); + }); + + messageCallback!({ + name: "connection.closed", + data: { + connectionId: "test-connection-123", + reason: "client closed", + }, + timestamp: Date.now(), + clientId: "test-client", + connectionId: "test-connection-123", + id: "msg-456", + }); + + const { stdout } = await commandPromise; + expect(stdout).toContain("connection.closed"); }); @@ -248,7 +247,6 @@ describe("LogsConnectionLifecycleSubscribe", function () { const mock = getMockAblyRealtime(); const channel = mock.channels._getChannel("[meta]connection.lifecycle"); - // Capture the subscription callback let messageCallback: ((message: unknown) => void) | null = null; channel.subscribe.mockImplementation( (callback: (message: unknown) => void) => { @@ -256,28 +254,29 @@ describe("LogsConnectionLifecycleSubscribe", function () { }, ); - // Simulate receiving a channel state change event - setTimeout(() => { - if (messageCallback) { - messageCallback({ - name: "channel.attached", - data: { - channelName: "test-channel", - state: "attached", - }, - timestamp: Date.now(), - clientId: "test-client", - connectionId: "test-connection-123", - id: "msg-channel-state", - }); - } - }, 50); - - const { stdout } = await runCommand( + const commandPromise = runCommand( ["logs:connection-lifecycle:subscribe"], import.meta.url, ); + await vi.waitFor(() => { + expect(messageCallback).not.toBeNull(); + }); + + messageCallback!({ + name: "channel.attached", + data: { + channelName: "test-channel", + state: "attached", + }, + timestamp: Date.now(), + clientId: "test-client", + connectionId: "test-connection-123", + id: "msg-channel-state", + }); + + const { stdout } = await commandPromise; + expect(channel.subscribe).toHaveBeenCalled(); expect(stdout).toContain("channel.attached"); }); diff --git a/test/unit/commands/spaces/locations/subscribe.test.ts b/test/unit/commands/spaces/locations/subscribe.test.ts index 2d6d05c43..57eb092a9 100644 --- a/test/unit/commands/spaces/locations/subscribe.test.ts +++ b/test/unit/commands/spaces/locations/subscribe.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { runCommand } from "@oclif/test"; import { getMockAblySpaces } from "../../../../helpers/mock-ably-spaces.js"; import { getMockAblyRealtime } from "../../../../helpers/mock-ably-realtime.js"; @@ -69,19 +69,18 @@ describe("spaces:locations:subscribe command", () => { import.meta.url, ); - // Wait a tick for the subscribe to be set up - await new Promise((resolve) => setTimeout(resolve, 50)); + await vi.waitFor(() => { + expect(locationHandler).toBeDefined(); + }); - if (locationHandler) { - locationHandler({ - member: { - clientId: "user-1", - connectionId: "conn-1", - }, - currentLocation: { room: "lobby" }, - previousLocation: { room: "entrance" }, - }); - } + locationHandler!({ + member: { + clientId: "user-1", + connectionId: "conn-1", + }, + currentLocation: { room: "lobby" }, + previousLocation: { room: "entrance" }, + }); const { stdout } = await runPromise; @@ -109,18 +108,18 @@ describe("spaces:locations:subscribe command", () => { import.meta.url, ); - await new Promise((resolve) => setTimeout(resolve, 50)); - - if (locationHandler) { - locationHandler({ - member: { - clientId: "user-1", - connectionId: "conn-1", - }, - currentLocation: { room: "lobby" }, - previousLocation: null, - }); - } + await vi.waitFor(() => { + expect(locationHandler).toBeDefined(); + }); + + locationHandler!({ + member: { + clientId: "user-1", + connectionId: "conn-1", + }, + currentLocation: { room: "lobby" }, + previousLocation: null, + }); const { stdout } = await runPromise; diff --git a/test/unit/hooks/interactive-did-you-mean.test.ts b/test/unit/hooks/interactive-did-you-mean.test.ts index 59f64289d..d65887614 100644 --- a/test/unit/hooks/interactive-did-you-mean.test.ts +++ b/test/unit/hooks/interactive-did-you-mean.test.ts @@ -334,8 +334,10 @@ describe("Did You Mean Hook - Interactive Mode", function () { context, }); - // Wait for async restoration - await new Promise((resolve) => setTimeout(resolve, 30)); + // Wait for async restoration — all state changes happen together in the hook's cleanup path + await vi.waitFor(() => { + expect(mockReadline.resume).toHaveBeenCalled(); + }); // Verify readline was paused during prompt expect(mockReadline.pause).toHaveBeenCalled(); @@ -347,9 +349,6 @@ describe("Did You Mean Hook - Interactive Mode", function () { expect(mockReadline.on.mock.calls[index]).toEqual(["line", listener]); }); - // Verify readline was resumed - expect(mockReadline.resume).toHaveBeenCalled(); - // Verify terminal state was restored expect(process.stdin.setRawMode).toHaveBeenCalledWith(false); } finally {