This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6542-c453eb098653bcb2f49bfc201fd40e5ec1de26d3 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 8acc6c1486e136ced7cf90acee2b4eee103967ac Author: Meng Wang <[email protected]> AuthorDate: Sat Jul 18 19:25:16 2026 -0700 test(frontend): stabilize flaky Plotly chart tests (UserQuota, WorkflowExecutionHistory) (#6542) ### What changes were proposed in this PR? Fixes flaky chart tests in two frontend specs that mocked `plotly.js-basic-dist-min` at the module level (`vi.mock`) to assert the Plotly calls: - `user-quota.component.spec.ts` — `generatePieChart` / `generateLineChart` (added in #6524) - `workflow-execution-history.component.spec.ts` — `charts (ngAfterViewInit)` That interception is **not reliable across the CI matrix** — when another spec loads the module first, the mock does not apply, the real `Plotly.newPlot` runs, and it throws `No DOM element with id '…' exists on the page.`. The tests passed on macOS but failed on ubuntu/windows (a flaky failure now on `main`). The `workflow-execution-history` mock also tripped a vitest "`vi.mock` … is not at the top level … will become an error in a future version" warning. Both are fixed the same way — drop the module mock and render into a real DOM element, then assert on the `data` / `layout` Plotly attaches to each graph div: - `user-quota`: each chart test appends a `<div id="…">`, then reads the graph div. - `workflow-execution-history`: the fixture is attached to `document.body` in `setup()` so `ngAfterViewInit`'s charts resolve their target divs by id. This is deterministic across platforms and also clears the hoist warning. No production code was changed. ### Any related issues, documentation, discussions? Closes #6541 (follow-up to #6524). ### How was this PR tested? Both specs run locally in `frontend/` (all green; the failure path was verified for each by breaking a chart assertion to confirm the suite goes red): ``` ng test --watch=false --include src/app/dashboard/component/user/user-quota/user-quota.component.spec.ts # Test Files 1 passed (1) | Tests 11 passed (11) ng test --watch=false --include src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts # Test Files 1 passed (1) | Tests 46 passed (46) (and the vi.mock hoist warning is gone) prettier --write <specs> # clean eslint <specs> # clean ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../user/user-quota/user-quota.component.spec.ts | 51 ++++++++++------------ .../workflow-execution-history.component.spec.ts | 38 +++++++--------- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/user-quota/user-quota.component.spec.ts b/frontend/src/app/dashboard/component/user/user-quota/user-quota.component.spec.ts index 400e2fcdca..a659317792 100644 --- a/frontend/src/app/dashboard/component/user/user-quota/user-quota.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-quota/user-quota.component.spec.ts @@ -24,12 +24,19 @@ import { HttpClientTestingModule } from "@angular/common/http/testing"; import { commonTestProviders } from "../../../../common/testing/test-utils"; import { of } from "rxjs"; import type { Mocked } from "vitest"; -import * as Plotly from "plotly.js-basic-dist-min"; import { ExecutionQuota } from "../../../../common/type/user"; import { DatasetQuota } from "../../../type/quota-statistic.interface"; -// Plotly is a read-only ESM namespace (can't be spied on), so mock the whole module. -vi.mock("plotly.js-basic-dist-min", () => ({ newPlot: vi.fn() })); +// Real Plotly renders into a DOM element by id; create one and assert on the +// `data`/`layout` Plotly attaches to that graph div. (Module-level mocking of +// plotly.js-basic-dist-min was flaky across the CI matrix — the mock did not +// always intercept, letting the real newPlot throw "No DOM element with id".) +function chartDiv(id: string): void { + document.getElementById(id)?.remove(); // avoid duplicate ids across reruns/retries + const div = document.createElement("div"); + div.id = id; + document.body.appendChild(div); +} // ISO 'YYYY-MM-DD' for a date `days` before now (kept by the 1-year filter for small values). function isoDaysAgo(days: number): string { @@ -211,9 +218,8 @@ describe("UserQuotaComponent", () => { }); describe("chart generation", () => { - it("generatePieChart passes labels/values and the sizing layout to Plotly", () => { - const newPlot = vi.mocked(Plotly.newPlot); - newPlot.mockClear(); + it("generatePieChart renders the labels/values and sizing layout onto the target div", () => { + chartDiv("pieDiv"); component.generatePieChart( [ @@ -224,20 +230,15 @@ describe("UserQuotaComponent", () => { "pieDiv" ); - expect(newPlot).toHaveBeenCalledTimes(1); - const [chartId, data, layout] = newPlot.mock.calls[0]; - expect(chartId).toBe("pieDiv"); - expect(data).toEqual([{ values: [1, 2], labels: ["a", "b"], type: "pie" }]); - expect(layout).toMatchObject({ - height: component.DEFAULT_PIE_CHART_HEIGHT, - width: component.DEFAULT_PIE_CHART_WIDTH, - title: { text: "Pie Title" }, - }); + const gd = document.getElementById("pieDiv") as unknown as { data: any[]; layout: any }; + expect(gd.data[0]).toMatchObject({ values: [1, 2], labels: ["a", "b"], type: "pie" }); + expect(gd.layout.width).toBe(component.DEFAULT_PIE_CHART_WIDTH); + expect(gd.layout.height).toBe(component.DEFAULT_PIE_CHART_HEIGHT); + expect(gd.layout.title).toMatchObject({ text: "Pie Title" }); }); - it("generateLineChart passes x/y series and the axis labels to Plotly", () => { - const newPlot = vi.mocked(Plotly.newPlot); - newPlot.mockClear(); + it("generateLineChart renders the x/y series and axis labels onto the target div", () => { + chartDiv("lineDiv"); component.generateLineChart( [ @@ -250,15 +251,11 @@ describe("UserQuotaComponent", () => { "lineDiv" ); - expect(newPlot).toHaveBeenCalledTimes(1); - const [chartId, data, layout] = newPlot.mock.calls[0]; - expect(chartId).toBe("lineDiv"); - expect(data).toEqual([{ x: ["2024-01-01", "2024-01-02"], y: [1, 3], type: "scatter" }]); - expect(layout).toMatchObject({ - title: { text: "Line Title" }, - xaxis: { title: { text: "X Label" } }, - yaxis: { title: { text: "Y Label" } }, - }); + const gd = document.getElementById("lineDiv") as unknown as { data: any[]; layout: any }; + expect(gd.data[0]).toMatchObject({ x: ["2024-01-01", "2024-01-02"], y: [1, 3], type: "scatter" }); + expect(gd.layout.title).toMatchObject({ text: "Line Title" }); + expect(gd.layout.xaxis.title).toMatchObject({ text: "X Label" }); + expect(gd.layout.yaxis.title).toMatchObject({ text: "Y Label" }); }); }); }); diff --git a/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts index d553118cd9..76ea30b7d6 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts @@ -23,7 +23,6 @@ import { ActivatedRoute } from "@angular/router"; import { NZ_MODAL_DATA, NzModalModule, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import type { ModalOptions } from "ng-zorro-antd/modal"; import { config, of, throwError } from "rxjs"; -import * as Plotly from "plotly.js-basic-dist-min"; import Fuse from "fuse.js"; import { WorkflowExecutionHistoryComponent } from "./workflow-execution-history.component"; @@ -38,11 +37,6 @@ import { OperatorMetadataService } from "../../../../../workspace/service/operat import { StubOperatorMetadataService } from "../../../../../workspace/service/operator-metadata/stub-operator-metadata.service"; import { commonTestProviders } from "../../../../../common/testing/test-utils"; -// Plotly draws onto real canvas/WebGL surfaces jsdom does not provide; the component -// only ever calls newPlot, so replace the module wholesale (same precedent as -// menu.component.spec.ts mocking file-saver). -vi.mock("plotly.js-basic-dist-min", () => ({ newPlot: vi.fn() })); - function makeEntry(overrides: Partial<WorkflowExecutionsEntry> = {}): WorkflowExecutionsEntry { return { eId: 1, @@ -165,14 +159,17 @@ describe("WorkflowExecutionHistoryComponent", () => { ], }).compileComponents(); - vi.mocked(Plotly.newPlot).mockClear(); fixture = TestBed.createComponent(WorkflowExecutionHistoryComponent); component = fixture.componentInstance; + // Attach to the document so ngAfterViewInit's real Plotly.newPlot can resolve the + // chart divs by id (a detached fixture is not reachable via getElementById). + document.body.appendChild(fixture.nativeElement); // first detectChanges runs ngOnInit (table load) + ngAfterViewInit (charts) fixture.detectChanges(); } afterEach(() => { + fixture?.nativeElement.remove(); fixture?.destroy(); }); @@ -205,33 +202,30 @@ describe("WorkflowExecutionHistoryComponent", () => { it("draws a username pie, a status pie, and a process-time bar chart", async () => { await setup(); - const newPlot = vi.mocked(Plotly.newPlot); - expect(newPlot).toHaveBeenCalledTimes(3); + // ngAfterViewInit renders the charts via real Plotly, which attaches `data`/`layout` + // to each graph div (looked up by the id the component passes, incl. the leading '#'). + const gd = (id: string) => document.getElementById(id) as unknown as { data: any[]; layout: any }; - const [usernameChartId, usernameData, usernameLayout] = newPlot.mock.calls[0]; - expect(usernameChartId).toBe("#execution-userName-pie-chart"); - const usernamePie = (usernameData as unknown as Array<{ labels: string[]; values: number[]; type: string }>)[0]; + const usernamePie = gd("#execution-userName-pie-chart").data[0]; expect(usernamePie.type).toBe("pie"); expect(usernamePie.labels).toEqual(["alice", "bob"]); expect(usernamePie.values).toEqual([2, 1]); - expect(usernameLayout).toEqual( - expect.objectContaining({ width: 450, height: 450, title: { text: "Users who ran the execution" } }) - ); + expect(gd("#execution-userName-pie-chart").layout).toMatchObject({ + width: 450, + height: 450, + title: { text: "Users who ran the execution" }, + }); - const [statusChartId, statusData] = newPlot.mock.calls[1]; - expect(statusChartId).toBe("#execution-status-pie-chart"); - const statusPie = (statusData as unknown as Array<{ labels: string[]; values: number[] }>)[0]; + const statusPie = gd("#execution-status-pie-chart").data[0]; expect(statusPie.labels).toEqual(["Running", "Completed"]); expect(statusPie.values).toEqual([1, 2]); - const [barChartId, barData, barLayout] = newPlot.mock.calls[2]; - expect(barChartId).toBe("#execution-average-process-time-bar-chart"); - const bar = (barData as unknown as Array<{ x: string[]; y: number[]; type: string }>)[0]; + const bar = gd("#execution-average-process-time-bar-chart").data[0]; expect(bar.type).toBe("bar"); // ceil(3 rows / divider 10) = 1-row buckets; process times are 1, 2, 3 minutes expect(bar.x).toEqual(["1~1", "2~2", "3~3"]); expect(bar.y).toEqual([1, 2, 3]); - expect(barLayout).toEqual(expect.objectContaining({ width: 600, height: 600 })); + expect(gd("#execution-average-process-time-bar-chart").layout).toMatchObject({ width: 600, height: 600 }); }); it("buckets 20 rows into ceil(20/10)=2-row groups keyed by position and averages minutes", async () => {
