From d7a0dbc6b19d57d68c5403a38169326af7a16832 Mon Sep 17 00:00:00 2001
From: Arya Gummadi <aryagummadi@google.com>
Date: Mon, 22 Sep 2025 15:56:09 -0700
Subject: [PATCH] fix(tests): fix flaky SettingsDialog tests (#8396)

Co-authored-by: Christie Warwick (Wilson) <bobcatfish@gmail.com>
---
 .../src/ui/components/SettingsDialog.test.tsx | 84 +++++++++++++------
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx
index 919c025f4e..e9d53b9f58 100644
--- a/packages/cli/src/ui/components/SettingsDialog.test.tsx
+++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx
@@ -22,7 +22,6 @@
  */
 
 import { render } from 'ink-testing-library';
-import { waitFor } from '@testing-library/react';
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { SettingsDialog } from './SettingsDialog.js';
 import { LoadedSettings, SettingScope } from '../../config/settings.js';
@@ -152,8 +151,32 @@ vi.mock('../../utils/settingsUtils.js', async () => {
 // const originalConsoleError = console.error;
 
 describe('SettingsDialog', () => {
+  // Simple delay function for remaining tests that need gradual migration
   const wait = (ms = 50) => new Promise((resolve) => setTimeout(resolve, ms));
 
+  // Custom waitFor utility for ink testing environment (not compatible with @testing-library/react)
+  const waitFor = async (
+    predicate: () => void,
+    options: { timeout?: number; interval?: number } = {},
+  ) => {
+    const { timeout = 1000, interval = 10 } = options;
+    const start = Date.now();
+    let lastError: unknown;
+    while (Date.now() - start < timeout) {
+      try {
+        predicate();
+        return;
+      } catch (e) {
+        lastError = e;
+      }
+      await new Promise((resolve) => setTimeout(resolve, interval));
+    }
+    if (lastError) {
+      throw lastError;
+    }
+    throw new Error('waitFor timed out');
+  };
+
   beforeEach(() => {
     // Reset keypress mock state (variables are commented out)
     // currentKeypressHandler = null;
@@ -337,21 +360,36 @@ describe('SettingsDialog', () => {
         </KeypressProvider>
       );
 
-      const { stdin, unmount } = render(component);
+      const { stdin, unmount, lastFrame } = render(component);
 
-      // Press Enter to toggle current setting
-      stdin.write(TerminalKeys.DOWN_ARROW as string);
-      await wait();
-      stdin.write(TerminalKeys.ENTER as string);
-      await wait();
+      // Wait for initial render and verify we're on Vim Mode (first setting)
+      await waitFor(() => {
+        expect(lastFrame()).toContain('● Vim Mode');
+      });
 
-      // Wait for the mock to be called with more generous timeout for Windows
-      await waitFor(
-        () => {
-          expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
-        },
-        { timeout: 1000 },
-      );
+      // Navigate to Disable Auto Update setting and verify we're there
+      act(() => {
+        stdin.write(TerminalKeys.DOWN_ARROW as string);
+      });
+      await waitFor(() => {
+        expect(lastFrame()).toContain('● Disable Auto Update');
+      });
+
+      // Toggle the setting
+      act(() => {
+        stdin.write(TerminalKeys.ENTER as string);
+      });
+      // Wait for the setting change to be processed
+      await waitFor(() => {
+        expect(
+          vi.mocked(saveModifiedSettings).mock.calls.length,
+        ).toBeGreaterThan(0);
+      });
+
+      // Wait for the mock to be called
+      await waitFor(() => {
+        expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
+      });
 
       expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalledWith(
         new Set<string>(['general.disableAutoUpdate']),
@@ -428,12 +466,9 @@ describe('SettingsDialog', () => {
         await wait();
         stdin.write(TerminalKeys.ENTER as string);
         await wait();
-        await waitFor(
-          () => {
-            expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
-          },
-          { timeout: 1000 },
-        );
+        await waitFor(() => {
+          expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
+        });
 
         expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalledWith(
           new Set<string>(['ui.theme']),
@@ -468,12 +503,9 @@ describe('SettingsDialog', () => {
         await wait();
         stdin.write(TerminalKeys.ENTER as string);
         await wait();
-        await waitFor(
-          () => {
-            expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
-          },
-          { timeout: 1000 },
-        );
+        await waitFor(() => {
+          expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled();
+        });
 
         expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalledWith(
           new Set<string>(['ui.theme']),