fix(FN-2227): stabilize NewAgentDialog async lifecycle tests

- Trigger model loading when the dialog opens instead of only on initial mount
- Guard NewAgentDialog effects with isOpen to avoid late async state updates after close/unmount
- Update AgentsView dialog tests to await settled UI state before asserting controls
- Harden NewAgentDialog tests with async act/waitFor patterns around render, reopen, and fetch side effects
This commit is contained in:
Fusion
2026-04-22 02:05:26 -07:00
committed by gsxdsm
parent 898a5f9afc
commit cc7d7d9040
3 changed files with 102 additions and 58 deletions

View File

@@ -63,8 +63,9 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
const [favoriteProviders, setFavoriteProviders] = useState<string[]>([]); const [favoriteProviders, setFavoriteProviders] = useState<string[]>([]);
const [favoriteModels, setFavoriteModels] = useState<string[]>([]); const [favoriteModels, setFavoriteModels] = useState<string[]>([]);
// Load models on mount (global data, not per-agent) // Load models when dialog opens — guard prevents async setState after test assertions
useEffect(() => { useEffect(() => {
if (!isOpen) return;
setModelsLoading(true); setModelsLoading(true);
fetchModels() fetchModels()
.then((response) => { .then((response) => {
@@ -76,7 +77,7 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
// Gracefully handle — dropdown will show empty list // Gracefully handle — dropdown will show empty list
}) })
.finally(() => setModelsLoading(false)); .finally(() => setModelsLoading(false));
}, []); }, [isOpen]);
// Selected model in "provider/modelId" format, or "" for default // Selected model in "provider/modelId" format, or "" for default
const selectedModel = runtimeConfig.model.includes("/") const selectedModel = runtimeConfig.model.includes("/")

View File

@@ -243,7 +243,9 @@ describe("AgentsView", () => {
it("shows refresh button", async () => { it("shows refresh button", async () => {
render(<AgentsView addToast={mockAddToast} />); render(<AgentsView addToast={mockAddToast} />);
const refreshBtn = screen.getByTitle("Refresh"); // Use findBy to ensure React has flushed all pending state updates before asserting.
// This prevents act(...) warnings from any async effects triggered during render.
const refreshBtn = await screen.findByTitle("Refresh");
expect(refreshBtn).toBeTruthy(); expect(refreshBtn).toBeTruthy();
}); });
}); });
@@ -677,7 +679,10 @@ describe("AgentsView", () => {
fireEvent.click(screen.getByText("New Agent")); fireEvent.click(screen.getByText("New Agent"));
expect(screen.getByPlaceholderText("e.g. Frontend Reviewer")).toBeTruthy(); // Wait for the dialog to settle after the model fetch completes
await waitFor(() => {
expect(screen.getByPlaceholderText("e.g. Frontend Reviewer")).toBeTruthy();
});
}); });
it("does not allow proceeding with empty name", async () => { it("does not allow proceeding with empty name", async () => {
@@ -689,6 +694,12 @@ describe("AgentsView", () => {
fireEvent.click(screen.getByText("New Agent")); fireEvent.click(screen.getByText("New Agent"));
// Wait for the dialog to settle after the model fetch completes
await waitFor(() => {
const nextBtn = screen.getByText("Next");
expect(nextBtn).toBeTruthy();
});
// Next button should be disabled when name is empty // Next button should be disabled when name is empty
const nextBtn = screen.getByText("Next"); const nextBtn = screen.getByText("Next");
expect(nextBtn.hasAttribute("disabled")).toBe(true); expect(nextBtn.hasAttribute("disabled")).toBe(true);

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from "vitest"; import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, fireEvent, waitFor, within } from "@testing-library/react"; import { render, screen, fireEvent, waitFor, within, act } from "@testing-library/react";
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
import { NewAgentDialog } from "../NewAgentDialog"; import { NewAgentDialog } from "../NewAgentDialog";
import * as apiModule from "../../api"; import * as apiModule from "../../api";
@@ -131,19 +131,23 @@ describe("NewAgentDialog", () => {
expect(container.innerHTML).toBe(""); expect(container.innerHTML).toBe("");
}); });
it("renders the dialog when isOpen is true", () => { it("renders the dialog when isOpen is true", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
expect(screen.getByRole("dialog", { name: "Create new agent" })).toBeTruthy(); expect(screen.getByRole("dialog", { name: "Create new agent" })).toBeTruthy();
}); });
}); });
describe("model dropdown", () => { describe("model dropdown", () => {
it("fetches models on mount", () => { it("fetches models on mount", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
expect(mockFetchModels).toHaveBeenCalledOnce(); expect(mockFetchModels).toHaveBeenCalledOnce();
}); });
@@ -428,11 +432,15 @@ describe("NewAgentDialog", () => {
expect(mockOnClose).toHaveBeenCalled(); expect(mockOnClose).toHaveBeenCalled();
// Unmount and reopen - state should be reset // Unmount and reopen - state should be reset; wait for the second fetchModels useEffect to settle
unmount(); unmount();
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
await waitFor(() => expect(mockFetchModels).toHaveBeenCalledTimes(2));
// Name should be empty // Name should be empty
const newNameInput = screen.getByLabelText(/Name/) as HTMLInputElement; const newNameInput = screen.getByLabelText(/Name/) as HTMLInputElement;
@@ -441,18 +449,22 @@ describe("NewAgentDialog", () => {
}); });
describe("AI generation integration", () => { describe("AI generation integration", () => {
it("shows Generate with AI button in step 0", () => { it("shows Generate with AI button in step 0", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
expect(screen.getByText("Generate with AI")).toBeTruthy(); expect(screen.getByText("Generate with AI")).toBeTruthy();
}); });
it("opens AgentGenerationModal when Generate with AI is clicked", async () => { it("opens AgentGenerationModal when Generate with AI is clicked", async () => {
const user = userEvent.setup(); const user = userEvent.setup();
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
// Generation modal should not be open initially // Generation modal should not be open initially
expect(screen.queryByTestId("agent-generation-modal")).toBeNull(); expect(screen.queryByTestId("agent-generation-modal")).toBeNull();
@@ -606,18 +618,22 @@ describe("NewAgentDialog", () => {
}); });
describe("preset selection", () => { describe("preset selection", () => {
it("renders all 20 preset cards in step 0", () => { it("renders all 20 preset cards in step 0", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
const presetCards = screen.getAllByTestId(/^preset-/); const presetCards = screen.getAllByTestId(/^preset-/);
expect(presetCards).toHaveLength(20); expect(presetCards).toHaveLength(20);
}); });
it("shows the quick start header text", () => { it("shows the quick start header text", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
expect(screen.getByText("Choose a preset or fill in details manually")).toBeTruthy(); expect(screen.getByText("Choose a preset or fill in details manually")).toBeTruthy();
}); });
@@ -742,11 +758,15 @@ describe("NewAgentDialog", () => {
await user.click(screen.getByLabelText("Close")); await user.click(screen.getByLabelText("Close"));
expect(mockOnClose).toHaveBeenCalled(); expect(mockOnClose).toHaveBeenCalled();
// Re-open — state should be reset // Re-open — state should be reset; wait for the second fetchModels useEffect to settle
unmount(); unmount();
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
await waitFor(() => expect(mockFetchModels).toHaveBeenCalledTimes(2));
// Name should be empty (no preset selected) // Name should be empty (no preset selected)
const nameInput = screen.getByLabelText(/Name/) as HTMLInputElement; const nameInput = screen.getByLabelText(/Name/) as HTMLInputElement;
@@ -798,10 +818,12 @@ describe("NewAgentDialog", () => {
expect(typeof createCall.instructionsText).toBe("string"); expect(typeof createCall.instructionsText).toBe("string");
}); });
it("preset card titles show the professional title", () => { it("preset card titles show the professional title", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
const ceoCard = screen.getByTestId("preset-ceo"); const ceoCard = screen.getByTestId("preset-ceo");
expect(ceoCard.getAttribute("title")).toBe("Chief Executive Officer"); expect(ceoCard.getAttribute("title")).toBe("Chief Executive Officer");
@@ -810,10 +832,12 @@ describe("NewAgentDialog", () => {
expect(ctoCard.getAttribute("title")).toBe("Chief Technology Officer"); expect(ctoCard.getAttribute("title")).toBe("Chief Technology Officer");
}); });
it("renders descriptions in all 20 preset cards", () => { it("renders descriptions in all 20 preset cards", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
// Every preset should have a description element rendered // Every preset should have a description element rendered
const descriptionElements = screen.getAllByText(/.\./, { const descriptionElements = screen.getAllByText(/.\./, {
@@ -827,11 +851,13 @@ describe("NewAgentDialog", () => {
}); });
}); });
it("all presets have non-empty description strings", () => { it("all presets have non-empty description strings", async () => {
// Import the array directly by checking the rendered cards // Import the array directly by checking the rendered cards
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
const presetIds = [ const presetIds = [
"ceo", "cto", "cmo", "cfo", "engineer", "backend-engineer", "ceo", "cto", "cmo", "cfo", "engineer", "backend-engineer",
@@ -867,10 +893,12 @@ describe("NewAgentDialog", () => {
expect(titleInput.value).toBe("Oversees project strategy, sets priorities, and coordinates between departments to ensure alignment with business goals."); expect(titleInput.value).toBe("Oversees project strategy, sets priorities, and coordinates between departments to ensure alignment with business goals.");
}); });
it("name label shows required indicator when no preset is selected", () => { it("name label shows required indicator when no preset is selected", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
// On initial render (step 0, no preset), the * required indicator should be visible // On initial render (step 0, no preset), the * required indicator should be visible
const nameLabel = screen.getByText("Name", { selector: "label" }); const nameLabel = screen.getByText("Name", { selector: "label" });
@@ -922,10 +950,12 @@ describe("NewAgentDialog", () => {
expect(screen.getByText("Next")).not.toBeDisabled(); expect(screen.getByText("Next")).not.toBeDisabled();
}); });
it("Next button is disabled when no preset is selected and name is empty", () => { it("Next button is disabled when no preset is selected and name is empty", async () => {
render( await act(async () => {
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, render(
); <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
);
});
// On initial render (step 0, no preset, empty name), Next should be disabled // On initial render (step 0, no preset, empty name), Next should be disabled
expect(screen.getByText("Next")).toBeDisabled(); expect(screen.getByText("Next")).toBeDisabled();
@@ -1057,16 +1087,18 @@ describe("NewAgentDialog", () => {
// Select a preset // Select a preset
await user.click(screen.getByTestId("preset-cto")); await user.click(screen.getByTestId("preset-cto"));
// Close the dialog // Close the dialog — wait for the state updates to flush before unmounting
await user.click(screen.getByLabelText("Close")); await user.click(screen.getByLabelText("Close"));
expect(mockOnClose).toHaveBeenCalled(); expect(mockOnClose).toHaveBeenCalled();
// Re-open — state should be reset // Re-open — wait for the fetchModels useEffect to settle after remount
unmount(); unmount();
render( render(
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />, <NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
); );
await waitFor(() => expect(mockFetchModels).toHaveBeenCalledTimes(2));
// Soul and instructionsText should be empty // Soul and instructionsText should be empty
const soulTextarea = screen.getByLabelText(/Soul/) as HTMLTextAreaElement; const soulTextarea = screen.getByLabelText(/Soul/) as HTMLTextAreaElement;
expect(soulTextarea.value).toBe(""); expect(soulTextarea.value).toBe("");