From da5451b928a9d374450bd1eb39f8fc85d9aafbbc Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 13 Mar 2025 16:58:38 +0000 Subject: [PATCH] fix: hide empty startup scripts row in build timeline UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes issue #15464 by: 1. Filtering out the 'start' stage from agent stages in WorkspaceTimings.tsx when there are no startup scripts configured 2. Adding additional filtering in StagesChart.tsx to ignore empty 'start' stages 3. Adding tests to verify the behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../WorkspaceTiming/StagesChart.test.tsx | 81 ++++++++++++++ .../WorkspaceTiming/StagesChart.tsx | 14 ++- .../WorkspaceTiming/WorkspaceTimings.test.tsx | 102 ++++++++++++++++++ .../WorkspaceTiming/WorkspaceTimings.tsx | 21 ++-- 4 files changed, 207 insertions(+), 11 deletions(-) create mode 100644 site/src/modules/workspaces/WorkspaceTiming/StagesChart.test.tsx create mode 100644 site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.test.tsx diff --git a/site/src/modules/workspaces/WorkspaceTiming/StagesChart.test.tsx b/site/src/modules/workspaces/WorkspaceTiming/StagesChart.test.tsx new file mode 100644 index 0000000000..0bd09df97f --- /dev/null +++ b/site/src/modules/workspaces/WorkspaceTiming/StagesChart.test.tsx @@ -0,0 +1,81 @@ +import { render, screen } from "@testing-library/react"; +import { StagesChart, agentStages, type Stage } from "./StagesChart"; + +describe("StagesChart", () => { + const onSelectStage = jest.fn(); + + // Mock the stage timings + const mockStageWithTimings = { + stage: { + name: "connect", + label: "connect", + section: "agent (test)", + tooltip: { title:
Connect
}, + } as Stage, + visibleResources: 1, + range: { + startedAt: new Date("2023-01-01T12:00:00Z"), + endedAt: new Date("2023-01-01T12:01:00Z"), + }, + }; + + // Mock a stage with no timings + const mockStageWithoutTimings = { + stage: { + name: "start", + label: "run startup scripts", + section: "agent (test)", + tooltip: { title:
Run startup scripts
}, + } as Stage, + visibleResources: 0, + range: undefined, + }; + + it("should render stages with timing data", () => { + render( + + ); + + // Should display the section header + expect(screen.getByText("agent (test)")).toBeInTheDocument(); + + // Should display the stage label + expect(screen.getByText("connect")).toBeInTheDocument(); + }); + + it("should NOT render empty startup scripts stage with no visible resources", () => { + render( + + ); + + // Should display the section header + expect(screen.getByText("agent (test)")).toBeInTheDocument(); + + // Should NOT display the "run startup scripts" label as it has no timing data and no resources + expect(screen.queryByText("run startup scripts")).not.toBeInTheDocument(); + }); + + it("should render both stages when the startup script stage has resources", () => { + const mockStartStageWithResources = { + ...mockStageWithoutTimings, + visibleResources: 1, // Has one script + }; + + render( + + ); + + // Should display both stage labels + expect(screen.getByText("connect")).toBeInTheDocument(); + expect(screen.getByText("run startup scripts")).toBeInTheDocument(); + }); +}); \ No newline at end of file diff --git a/site/src/modules/workspaces/WorkspaceTiming/StagesChart.tsx b/site/src/modules/workspaces/WorkspaceTiming/StagesChart.tsx index cfb4285f77..7ba98e4fcb 100644 --- a/site/src/modules/workspaces/WorkspaceTiming/StagesChart.tsx +++ b/site/src/modules/workspaces/WorkspaceTiming/StagesChart.tsx @@ -91,9 +91,12 @@ export const StagesChart: FC = ({ {sections.map((section) => { - const stages = timings + // Filter out stages without timing data if it's the "start" stage with no visible resources + const filteredTimings = timings .filter((t) => t.stage.section === section) - .map((t) => t.stage); + .filter((t) => !(t.stage.name === "start" && t.visibleResources === 0 && t.range === undefined)); + + const stages = filteredTimings.map((t) => t.stage); return ( @@ -126,8 +129,13 @@ export const StagesChart: FC = ({ return ( {stageTimings.map((t) => { - // If the stage has no timing data, we just want to render an empty row + // If the stage has no timing data, we need to handle it specially if (t.range === undefined) { + // Skip rendering empty "run startup scripts" rows when no scripts are configured + if (t.stage.name === "start" && t.visibleResources === 0) { + return null; + } + return ( { + const mockProvisionerTimings: ProvisionerTiming[] = [ + { + action: "create", + applied_at: "2023-01-01T12:00:00Z", + created_at: "2023-01-01T12:00:00Z", + ended_at: "2023-01-01T12:01:00Z", + log_source_id: "1", + log_url: "", + resource: "aws_instance.test", + source: "terraform", + stage: "apply", + started_at: "2023-01-01T12:00:00Z", + status: "ok", + workspace_build_id: "1", + workspace_transition: "start", + }, + ]; + + const mockAgentConnectionTimings: AgentConnectionTiming[] = [ + { + created_at: "2023-01-01T12:01:00Z", + ended_at: "2023-01-01T12:02:00Z", + started_at: "2023-01-01T12:01:00Z", + stage: "connect", + status: "ok", + workspace_agent_id: "1", + workspace_agent_name: "test", + workspace_build_id: "1", + workspace_transition: "start", + }, + ]; + + const mockAgentScriptTimings: AgentScriptTiming[] = [ + { + created_at: "2023-01-01T12:02:00Z", + display_name: "test script", + ended_at: "2023-01-01T12:03:00Z", + exit_code: 0, + script_id: "1", + started_at: "2023-01-01T12:02:00Z", + stage: "start", + status: "ok", + workspace_agent_id: "1", + workspace_build_id: "1", + workspace_transition: "start", + }, + ]; + + it("renders with all timings", () => { + render( + + ); + + expect(screen.getByText("Build timeline")).toBeInTheDocument(); + }); + + it("renders correctly with empty agent script timings", () => { + render( + + ); + + expect(screen.getByText("Build timeline")).toBeInTheDocument(); + // Should not show loading state with Skeleton component + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + // Should not show "run startup scripts" stage + expect(screen.queryByText("run startup scripts")).not.toBeInTheDocument(); + }); + + it("shows loading state when provisioner timings are missing", () => { + render( + + ); + + expect(screen.getByText("Build timeline")).toBeInTheDocument(); + // Should be in loading state + expect(screen.getByRole("progressbar")).toBeInTheDocument(); + }); +}); \ No newline at end of file diff --git a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx index 63fc03ad2a..18dac3a34d 100644 --- a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx +++ b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx @@ -61,13 +61,12 @@ export const WorkspaceTimings: FC = ({ const [isOpen, setIsOpen] = useState(defaultIsOpen); - // If any of the timings are empty, we are still loading the data. They can be - // filled in different moments. - const isLoading = [ - provisionerTimings, - agentScriptTimings, - agentConnectionTimings, - ].some((t) => t.length === 0); + // If any of the required timing arrays are empty (except agentScriptTimings which + // can be empty if no scripts are configured), we are still loading the data. + // They can be filled in different moments. + const isLoading = + provisionerTimings.length === 0 || + agentConnectionTimings.length === 0; // agentScriptTimings can be empty if no scripts are configured // Each agent connection timing is a stage in the timeline to make it easier // to users to see the timing for connection and the other scripts. @@ -77,9 +76,15 @@ export const WorkspaceTimings: FC = ({ ), ); + // Check if there are any startup scripts configured + const hasStartupScripts = uniqScriptTimings.some(t => t.stage === "start"); + const stages = [ ...provisioningStages, - ...agentStageLabels.flatMap((a) => agentStages(a)), + ...agentStageLabels.flatMap((a) => + // Filter out the "start" stage if no startup scripts are configured + agentStages(a).filter(stage => hasStartupScripts || stage.name !== "start") + ), ]; const displayProvisioningTime = () => {