mengw15 commented on code in PR #8288:
URL: https://github.com/apache/texera/pull/8288#discussion_r3894137049
##########
frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts:
##########
@@ -206,30 +215,34 @@ describe("WorkflowExecutionHistoryComponent", () => {
it("draws a username pie, a status pie, and a process-time bar chart",
async () => {
await setup();
- // 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 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(gd("#execution-userName-pie-chart").layout).toMatchObject({
+ // ngAfterViewInit plots each chart by id; assert on what the component
+ // handed Plotly rather than on the DOM Plotly would build from it.
+ const plot = (id: string) => {
+ const call = vi.mocked(Plotly.newPlot).mock.calls.find(c => c[0] ===
id);
+ if (!call) throw new Error(`no Plotly.newPlot call for ${id}`);
Review Comment:
Good point — switched the helper to `filter(...)` and asserted the id
appears exactly once, so a duplicate render for the same id fails the test
alongside a missing one.
##########
frontend/src/app/dashboard/component/user/user-workflow/ngbd-modal-workflow-executions/workflow-execution-history.component.spec.ts:
##########
@@ -38,9 +38,17 @@ import { StubOperatorMetadataService } from
"../../../../../workspace/service/op
import { commonTestProviders } from "../../../../../common/testing/test-utils";
import { DebugElement } from "@angular/core";
import { By } from "@angular/platform-browser";
+import * as Plotly from "plotly.js-basic-dist-min";
import { NoopAnimationsModule } from "@angular/platform-browser/animations";
import { NzPopoverDirective } from "ng-zorro-antd/popover";
+// The component's job at this boundary is choosing a chart id and computing
the
+// data and layout for it; turning those into DOM is Plotly's. Rendering for
real
+// in jsdom asserts nothing extra -- the chart test below reads back exactly
the
+// arguments the component passed -- while it dominates the file's runtime,
since
+// 36 of these tests build the component and each build plots twice. See #8287.
+vi.mock("plotly.js-basic-dist-min", () => ({ newPlot: vi.fn() }));
Review Comment:
Leaving the mock at `vi.fn()`. The component never awaits or thens `newPlot`
today and neither does this spec, so a resolved-Promise return would only guard
a hypothetical future caller. When one actually shows up, the PR that adds the
`await` can bring the mock along.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
