Skip to content

Commit dc1db41

Browse files
mnoah1mnoah1
authored
fix(web): simplify Stovepipe results and separate repository status (#904)
## Why? Project result percentages add noise, and the repository result is mixed into the project table. ## What? Show Green or Breakage without percentages, keeping No result recorded for missing data. Move Repository result into Request status above History, leaving only projects in the Project results table. Preserve successful results after a failed refresh. ## Test Plan - Stovepipe library: 40 tests passed, including result labels, repository placement, missing sections, and refresh behavior. - Full web check passed: 31 test targets, including React 18 package rendering and hydration, lint, typechecks, and deployable image builds. Repository lint, dependency, and build-file checks passed. - Real-stack E2E: Stovepipe (5 tests) and SubmitQueue (5 tests) passed locally, including server-rendered styling, project cursors, refresh, missing results, and accessibility checks. E2E typecheck also passed. - Validation command: `./tool/bazel test //web/test/e2e/stovepipe:web_test //web/test/e2e/submitqueue:web_test //web/test/e2e/stovepipe:typecheck_typecheck_test --test_output=errors`.
1 parent 294257b commit dc1db41

7 files changed

Lines changed: 116 additions & 67 deletions

File tree

‎web/stovepipe/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ projects; `?projects=<opaque-token>` carries its continuation and can be shared.
2020
First-page navigation removes the cursor. Pages are live rather than a snapshot;
2121
return to the first page to see newly recorded projects.
2222

23+
The repository result appears in the request summary above history; the project results table contains only projects. Recorded results show Green or Breakage without percentages.
24+
2325
Repository breakage, per-project breakage, and recording completion are separate.
2426
Zero breakage is green; absent results are never treated as green. Completion
2527
describes result production, not whether all pages have been displayed. Project

‎web/stovepipe/view/app.test.tsx‎

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,11 @@ it("displays the backend summary above the retained history", () => {
6262
expect(within(status).getByText("all_targets_passed")).toBeVisible();
6363
expect(within(status).getByText("Accepted")).toBeVisible();
6464
expect(within(status).getByText("Updated")).toBeVisible();
65+
expect(within(status).getByText("Repository result")).toBeVisible();
66+
expect(within(status).getByText("No result recorded")).toBeVisible();
6567
const history = screen.getByRole("region", { name: "History" });
6668
expect(within(history).getByText("accepted")).toBeVisible();
69+
expect(status.compareDocumentPosition(history) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
6770
expect(
6871
screen.getAllByRole("heading", { level: 2 }).map((heading) => heading.textContent),
6972
).toEqual(["Request status", "History", "Project results"]);
@@ -103,6 +106,8 @@ it("retains the summary and history after a failed refresh", async () => {
103106
expect(screen.getByText("accepted")).toBeVisible();
104107
await waitFor(() => expect(screen.getAllByText("Showing the last successful result.")).toHaveLength(2));
105108
expect(screen.getByText("green-project")).toBeVisible();
109+
expect(within(screen.getByRole("region", { name: "Request status" }))
110+
.getByText("No result recorded")).toBeVisible();
106111
});
107112
it("discards previous results when the request changes", () => {
108113
const { rerender } = render(<StovepipeApp model={model} />);
@@ -125,10 +130,12 @@ it("renders recorded project results, completion and shareable pagination", () =
125130
render(<StovepipeApp model={model} />);
126131
const projects = screen.getByRole("region", { name: "Project results" });
127132
const table = within(projects).getByRole("table", { name: "Recorded project results" });
128-
expect(within(table).getByRole("rowheader", { name: "Repository" })).toBeVisible();
129-
expect(within(projects).getByText("Green (0%)")).toBeVisible();
130-
expect(within(projects).getByText("Breakage (25%)")).toBeVisible();
131-
expect(within(projects).getAllByText("No result recorded")).toHaveLength(2);
133+
expect(within(table).queryByText(/Repository/)).toBeNull();
134+
expect(within(table).getByRole("columnheader", { name: "Project" })).toBeVisible();
135+
expect(within(projects).getByText("Green")).toBeVisible();
136+
expect(within(projects).getByText("Breakage")).toBeVisible();
137+
expect(within(projects).getByText("No result recorded")).toBeVisible();
138+
expect(within(projects).queryByText(/%/)).toBeNull();
132139
expect(within(projects).getByText(/still in progress/)).toBeVisible();
133140
expect(within(projects).getByRole("link", { name: /Next projects/ })).toHaveAttribute(
134141
"href", "/repo%2Fmain/request/7?projects=opaque%2B%2F%3D",
@@ -161,6 +168,44 @@ it("shows complete empty results without inventing green or a continuation", ()
161168
expect(screen.getByText("No project results recorded on this page.")).toBeVisible();
162169
expect(screen.queryByRole("link", { name: /Next projects/ })).toBeNull();
163170
const table = screen.getByRole("table", { name: "Recorded project results" });
164-
expect(within(table).getByRole("rowheader", { name: "Repository" })).toBeVisible();
165-
expect(within(table).getByText("No result recorded")).toBeVisible();
171+
expect(within(table).getAllByRole("row")).toHaveLength(1);
172+
expect(within(screen.getByRole("region", { name: "Request status" }))
173+
.getByText("No result recorded")).toBeVisible();
174+
});
175+
176+
it.each([
177+
[0, "Green"],
178+
[0.25, "Breakage"],
179+
[null, "No result recorded"],
180+
] as const)("shows repository result %s above history as %s", (degree, label) => {
181+
if (!model.projectStatus.ok) throw new Error("No projects");
182+
render(<StovepipeApp model={{
183+
...model,
184+
projectStatus: { ok: true, data: {
185+
...model.projectStatus.data, repositoryBreakageDegree: degree,
186+
} },
187+
}} />);
188+
const status = screen.getByRole("region", { name: "Request status" });
189+
expect(within(status).getByText(label)).toBeVisible();
190+
expect(within(status).queryByText(/%/)).toBeNull();
191+
});
192+
193+
it("shows repository and project results when history is unavailable", async () => {
194+
render(<StovepipeApp model={{
195+
...model, history: { ok: false, error: "History unavailable" },
196+
}} />);
197+
expect(await screen.findByRole("alert")).toHaveTextContent("History unavailable");
198+
expect(within(screen.getByRole("region", { name: "Request status" }))
199+
.getByText("No result recorded")).toBeVisible();
200+
expect(screen.getByText("green-project")).toBeVisible();
201+
});
202+
203+
it("shows history when repository and project results are unavailable", async () => {
204+
render(<StovepipeApp model={{
205+
...model, projectStatus: { ok: false, error: "Projects unavailable" },
206+
}} />);
207+
expect(await screen.findByRole("alert")).toHaveTextContent("Projects unavailable");
208+
expect(screen.getByText("accepted")).toBeVisible();
209+
expect(screen.getByText("git://repo/sha")).toBeVisible();
210+
expect(screen.queryByText("Repository result")).toBeNull();
166211
});

‎web/stovepipe/view/app.tsx‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
"use client";
22

3-
import { Timestamp } from "@submitqueue/web-platform";
3+
import { Fact, Timestamp } from "@submitqueue/web-platform";
44
import { Block } from "baseui/block/index.js";
55
import { Tag } from "baseui/tag/index.js";
66
import { HeadingLarge, HeadingSmall, LabelMedium, ParagraphSmall } from "baseui/typography/index.js";
77
import { RefreshControls } from "./refresh.js";
88
import type { StovepipePageModel } from "../entity/index.js";
99
import { Link, Section } from "./components.js";
1010
import { RequestList, RequestDetails, History } from "./pages.js";
11-
import { ProjectResults } from "./projects.js";
11+
import { ProjectResults, RecordedResult } from "./projects.js";
1212

1313
function Page({ model }: { model: StovepipePageModel }) {
1414
return (
@@ -59,31 +59,38 @@ function Page({ model }: { model: StovepipePageModel }) {
5959
)}
6060
{model.kind === "queue" ? (
6161
<Section result={model.requests}>
62-
{(data) => <RequestList {...data} />}
62+
{(data) => data && <RequestList {...data} />}
6363
</Section>
6464
) : (
65-
<Block display="grid" gridTemplateColumns="minmax(0, 1fr)" gridGap="scale600">
66-
<Section result={model.history}>
67-
{({ request, events }) => (
68-
<>
69-
<section aria-label="Request status">
70-
<HeadingSmall as="h2" marginTop="0">Request status</HeadingSmall>
71-
<RequestDetails request={request} />
72-
</section>
73-
<section aria-label="History">
74-
<HeadingSmall as="h2" marginTop="0">History</HeadingSmall>
75-
<History events={events} />
76-
</section>
77-
</>
78-
)}
79-
</Section>
80-
<section aria-label="Project results">
81-
<HeadingSmall as="h2" marginTop="0">Project results</HeadingSmall>
65+
<Section result={model.history}>
66+
{(history) => (
8267
<Section result={model.projectStatus}>
83-
{(data) => <ProjectResults model={data} />}
68+
{(projects) => (
69+
<Block display="grid" gridTemplateColumns="minmax(0, 1fr)" gridGap="scale600">
70+
<section aria-label="Request status">
71+
<HeadingSmall as="h2" marginTop="0">Request status</HeadingSmall>
72+
{history && <RequestDetails request={history.request} />}
73+
{projects && (
74+
<Block as="dl" margin="0">
75+
<Fact label="Repository result">
76+
<RecordedResult degree={projects.repositoryBreakageDegree} />
77+
</Fact>
78+
</Block>
79+
)}
80+
</section>
81+
<section aria-label="History">
82+
<HeadingSmall as="h2" marginTop="0">History</HeadingSmall>
83+
{history && <History events={history.events} />}
84+
</section>
85+
<section aria-label="Project results">
86+
<HeadingSmall as="h2" marginTop="0">Project results</HeadingSmall>
87+
{projects && <ProjectResults model={projects} />}
88+
</section>
89+
</Block>
90+
)}
8491
</Section>
85-
</section>
86-
</Block>
92+
)}
93+
</Section>
8794
)}
8895
</Block>
8996
<Block as="footer" paddingTop="scale300" paddingBottom="scale300" font="ParagraphXSmall" color="contentSecondary">

‎web/stovepipe/view/components.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export function Section<T>({
1818
children,
1919
}: {
2020
result: ReadResult<T>;
21-
children(data: T): ReactNode;
21+
children(data: T | null): ReactNode;
2222
}) {
2323
const [previous, setPrevious] = useState<T | null>(
2424
result.ok ? result.data : null,
@@ -33,7 +33,7 @@ export function Section<T>({
3333
{previous !== null && <ParagraphSmall>Showing the last successful result.</ParagraphSmall>}
3434
</Notification>
3535
)}
36-
{data !== null ? children(data) : null}
36+
{children(data)}
3737
</>
3838
);
3939
}

‎web/stovepipe/view/projects.tsx‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,24 @@ import { ParagraphSmall } from "baseui/typography/index.js";
88
import type { ProjectStatusModel } from "../entity/index.js";
99
import { Link, Table } from "./components.js";
1010

11-
function Result({ degree }: { degree: number | null }) {
11+
export function RecordedResult({ degree }: { degree: number | null }) {
1212
return (
1313
<Tag closeable={false} size="small" kind={
1414
degree === null ? "neutral" : degree === 0 ? "positive" : "negative"
1515
}>
16-
{degree === null ? "No result recorded" : (
17-
<>{degree === 0 ? "Green" : "Breakage"} ({
18-
(degree * 100).toLocaleString("en-US", { maximumSignificantDigits: 6 })
19-
}%)</>
20-
)}
16+
{degree === null ? "No result recorded" : degree === 0 ? "Green" : "Breakage"}
2117
</Tag>
2218
);
2319
}
2420

2521
export function ProjectResults({ model }: { model: ProjectStatusModel }) {
2622
return (
2723
<>
28-
<ParagraphSmall color="contentSecondary">
29-
Breakage ranges from 0% (green) to 100%; missing results are not green.
30-
</ParagraphSmall>
31-
<Table label="Recorded project results" headers={["Scope", "Recorded result"]}>
32-
<StyledTableBodyRow>
33-
<StyledTableBodyCell $as="th" scope="row">Repository</StyledTableBodyCell>
34-
<StyledTableBodyCell><Result degree={model.repositoryBreakageDegree} /></StyledTableBodyCell>
35-
</StyledTableBodyRow>
24+
<Table label="Recorded project results" headers={["Project", "Recorded result"]}>
3625
{model.projects.map((entry) => (
3726
<StyledTableBodyRow key={entry.project}>
3827
<StyledTableBodyCell>{entry.project}</StyledTableBodyCell>
39-
<StyledTableBodyCell><Result degree={entry.breakageDegree} /></StyledTableBodyCell>
28+
<StyledTableBodyCell><RecordedResult degree={entry.breakageDegree} /></StyledTableBodyCell>
4029
</StyledTableBodyRow>
4130
))}
4231
</Table>

‎web/test/e2e/stovepipe/stovepipe.spec.ts‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@ test.describe("server-rendered styling", () => {
2020
});
2121
test("separates request status fields before hydration", async ({ page }) => {
2222
await page.goto("/web%2Fpagination/request/51");
23-
const rows = page.getByRole("region", { name: "Request status" }).locator("dl > div");
24-
await expect(rows).toHaveCount(6);
23+
const status = page.getByRole("region", { name: "Request status" });
24+
const rows = status.locator("dl > div");
25+
await expect(rows).toHaveCount(7);
26+
await expect(status.getByText("Repository result", { exact: true })).toBeVisible();
27+
await expect(status.getByText("Breakage", { exact: true })).toBeVisible();
2528
const layout = await rows.evaluateAll(fields => fields.map(field => ({
2629
height: field.getBoundingClientRect().height,
2730
labelGap: field.querySelector("dd")!.getBoundingClientRect().top - field.querySelector("dt")!.getBoundingClientRect().bottom,
@@ -157,38 +160,38 @@ test("real ingestion, queue selection, cursors, request summaries, history, proj
157160
});
158161
});
159162
await test.step("Read recorded project results and reload their cursor", async () => {
163+
const status = page.getByRole("region", { name: "Request status", exact: true });
160164
const section = page.getByRole("region", { name: "Project results", exact: true });
161165
const projects = section.getByRole("table", { name: "Recorded project results", exact: true });
162-
await expect(projects.locator("tbody tr")).toHaveCount(51);
163-
await expect(
164-
projects.getByRole("row").filter({
165-
has: page.getByRole("rowheader", { name: "Repository", exact: true }),
166-
}),
167-
).toContainText("Breakage (25%)");
166+
await expect(projects.locator("tbody tr")).toHaveCount(50);
167+
await expect(status.getByText("Repository result", { exact: true })).toBeVisible();
168+
await expect(status.getByText("Breakage", { exact: true })).toBeVisible();
169+
await expect(projects.getByText(/Repository|%/)).toHaveCount(0);
168170
await expect(
169171
projects.getByRole("row").filter({
170172
has: page.getByRole("cell", { name: "project-01", exact: true }),
171-
}),
172-
).toContainText("Breakage (25%)");
173+
}).getByRole("cell", { name: "Breakage", exact: true }),
174+
).toBeVisible();
173175
await expect(
174176
projects.getByRole("row").filter({
175177
has: page.getByRole("cell", { name: "project-02", exact: true }),
176-
}),
177-
).toContainText("Green (0%)");
178+
}).getByRole("cell", { name: "Green", exact: true }),
179+
).toBeVisible();
178180
await refresh.click();
179181
await expect(refresh).toBeEnabled();
180-
await expect(projects.locator("tbody tr")).toHaveCount(51);
182+
await expect(projects.locator("tbody tr")).toHaveCount(50);
181183
await section.getByRole("link", { name: "Next projects →", exact: true }).click();
182-
await expect(projects.locator("tbody tr")).toHaveCount(2);
184+
await expect(projects.locator("tbody tr")).toHaveCount(1);
183185
await expect(projects.getByRole("cell", { name: "project-51", exact: true })).toBeVisible();
184186
const cursor = new URL(page.url()).searchParams.get("projects");
185187
expect(cursor).toBeTruthy();
186188
await page.reload();
187-
await expect(projects.locator("tbody tr")).toHaveCount(2);
189+
await expect(projects.locator("tbody tr")).toHaveCount(1);
190+
await expect(status.getByText("Breakage", { exact: true })).toBeVisible();
188191
expect(new URL(page.url()).searchParams.get("projects")).toBe(cursor);
189192
await expect(section.getByRole("link", { name: "Next projects →", exact: true })).toHaveCount(0);
190193
await section.getByRole("link", { name: "First projects", exact: true }).click();
191-
await expect(projects.locator("tbody tr")).toHaveCount(51);
194+
await expect(projects.locator("tbody tr")).toHaveCount(50);
192195
expect(new URL(page.url()).searchParams.has("projects")).toBe(false);
193196
expect((await new AxeBuilder({ page }).analyze()).violations).toEqual([]);
194197
});
@@ -197,10 +200,11 @@ test("real ingestion, queue selection, cursors, request summaries, history, proj
197200
const section = page.getByRole("region", { name: "Project results", exact: true });
198201
await expect(section.getByText("No project results recorded on this page.", { exact: true })).toBeVisible();
199202
const projects = section.getByRole("table", { name: "Recorded project results", exact: true });
200-
await expect(projects.locator("tbody tr")).toHaveCount(1);
201-
await expect(projects.getByRole("rowheader", { name: "Repository", exact: true })).toBeVisible();
202-
await expect(projects.getByRole("cell", { name: "No result recorded", exact: true })).toBeVisible();
203-
await expect(section.getByText("Green (0%)", { exact: true })).toHaveCount(0);
203+
await expect(projects.locator("tbody tr")).toHaveCount(0);
204+
const status = page.getByRole("region", { name: "Request status", exact: true });
205+
await expect(status.getByText("Repository result", { exact: true })).toBeVisible();
206+
await expect(status.getByText("No result recorded", { exact: true })).toBeVisible();
207+
await expect(page.getByText("Green", { exact: true })).toHaveCount(0);
204208
});
205209
await test.step("Follow and reload the queue cursor, then select an empty queue", async () => {
206210
await page.getByRole("link", { name: "← Queue requests" }).click();

‎web/test/package/react18/react18-runtime.mjs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ for (const { route, page, theme, html, stylesheets } of pages) {
110110
expected.remove();
111111
if (route === "/repo%2Fmain/request/1") {
112112
assert.match(container.textContent, /example-project/u);
113-
assert.match(container.textContent, /Breakage \(25%\)/u);
113+
assert.match(container.textContent, /Repository resultBreakage/u);
114+
assert.match(container.textContent, /example-projectGreen/u);
115+
assert.doesNotMatch(container.textContent, /%/u);
114116
}
115117
await act(async () => root.unmount());
116118
container.remove();

0 commit comments

Comments
 (0)