You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/01 09:08:24 UTC
(texera) 01/01: test(frontend): extend workspace mini-map, jupyter-panel and preset specs (#7241)
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-7241-a03782846338f0883d0c680328b35d41d9a2f7de
in repository https://gitbox.apache.org/repos/asf/texera.git
commit 1ad5163f83fe3d48b4451bf76f04ec578f252645
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Sat Aug 1 02:08:03 2026 -0700
test(frontend): extend workspace mini-map, jupyter-panel and preset specs (#7241)
### What changes were proposed in this PR?
Three existing specs under `frontend/src/app/workspace/`, extended to
cover what their subjects had left untested:
| File | Before | Tests now |
|---|---|---|
| `mini-map.component.ts` | 42.9% | 16 |
| `preset.service.ts` | 81.0% | 55 |
| `jupyter-panel.service.ts` | 82.2% | 41 |
Two of these have traps that make a naive extension pass for the wrong
reason, which is most of why they were left half-covered:
**jsdom has no layout.** `src/jsdom-svg-polyfill.ts` stubs
`getScreenCTM`/`getCTM` to an identity matrix and `getBBox` to a zero
rect, and `offsetWidth`/`offsetHeight` are 0. So `ngAfterViewInit`
computes `scale = 0`, and a geometry assertion reads `"0px"` — true
because there is no layout, not because the formula is right. The
mini-map specs use a stub paper with explicit, mutually distinguishable
geometry plus `Object.defineProperty`'d offset dimensions, and every
expected value is a non-zero literal (`200px`, `100px`, `37.5px`,
`matrix(0.25,0,0,0.25,240,135)`), so a regression back to zero-layout
fails.
**`catchError` swallows the difference between two branches.**
`fetchNotebookAndMapping` wraps its `switchMap` in `catchError(() =>
of(0))`. The base mock has no `sendNotebookToJupyter`, so flushing
`{exists:true}` without defining it throws a `TypeError` that becomes
`0` — indistinguishable from the send-failed branch under test. Every
spec on that path defines the mock explicitly *and* asserts it was
called; the reject case additionally asserts `console.error` was not
called, which is what separates it from the `catchError` path.
Also handled: Angular's automatic fixture teardown fires `ngOnDestroy`
after every test, which writes `localStorage["mini-map"]`. The key is
cleared before `TestBed.createComponent` and again in `afterEach`, so
ordering cannot decide a result.
Two things deliberately left uncovered, with the reasoning in the files:
- `updatePreset`'s splice/replace branches. The method has **no
production caller** and carries a real bug — lodash `indexOf` against a
list that was just `JSON.parse`'d always returns `-1`, so `splice(-1,
1)` deletes the *last* preset instead of the intended one, and
`presets[-1] = replacement` writes a non-index property that
`JSON.stringify` silently drops. Its sibling `updateOrCreatePreset`
already uses the correct `findIndex(p => isEqual(p, original))`. Testing
it would cement the bug; it wants a separate fix-or-delete.
- The two `no default save preset info/warning message` throws — pinning
them would assert a limitation that adding a default message would fix.
Assertion strength was checked by mutation and every mutation reverted;
the production diff is empty.
### Any related issues, documentation, discussions?
Closes #7238
### How was this PR tested?
```
npx ng test --watch=false --include="**/mini-map.component.spec.ts" --include="**/jupyter-panel.service.spec.ts" --include="**/preset.service.spec.ts"
```
```
✓ src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts (41 tests)
✓ src/app/workspace/service/preset/preset.service.spec.ts (55 tests)
✓ src/app/workspace/component/workflow-editor/mini-map/mini-map.component.spec.ts (16 tests)
Test Files 3 passed (3)
```
`yarn format:ci` passes (prettier-eslint + eslint), which Vitest does
not cover on its own.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
---
.../mini-map/mini-map.component.spec.ts | 358 ++++++++++++++++++++-
.../jupyter-panel/jupyter-panel.service.spec.ts | 239 ++++++++++++++
.../service/preset/preset.service.spec.ts | 60 ++++
3 files changed, 655 insertions(+), 2 deletions(-)
diff --git a/frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.spec.ts
index 3686454ca6..2087b5e72b 100644
--- a/frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.spec.ts
+++ b/frontend/src/app/workspace/component/workflow-editor/mini-map/mini-map.component.spec.ts
@@ -19,18 +19,100 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { HttpClientTestingModule } from "@angular/common/http/testing";
+import { ReplaySubject } from "rxjs";
+import * as joint from "jointjs";
import { MiniMapComponent } from "./mini-map.component";
+import { MAIN_CANVAS } from "../workflow-editor.component";
import { WorkflowActionService } from "../../../service/workflow-graph/model/workflow-action.service";
+import { JointGraphWrapper } from "../../../service/workflow-graph/model/joint-graph-wrapper";
import { OperatorMetadataService } from "../../../service/operator-metadata/operator-metadata.service";
import { StubOperatorMetadataService } from "../../../service/operator-metadata/stub-operator-metadata.service";
import { JointUIService } from "../../../service/joint-ui/joint-ui.service";
+import { PanelService } from "../../../service/panel/panel.service";
import { UndoRedoService } from "../../../service/undo-redo/undo-redo.service";
import { WorkflowUtilService } from "../../../service/workflow-graph/util/workflow-util.service";
import { DragDropModule } from "@angular/cdk/drag-drop";
import { commonTestProviders } from "../../../../common/testing/test-utils";
+/**
+ * Stand-in for the main workflow-editor paper the mini-map mirrors.
+ *
+ * jsdom has no layout engine: `src/jsdom-svg-polyfill.ts` stubs `getScreenCTM`
+ * / `getCTM` to the identity matrix and `getBBox` to a zero rect, so a real
+ * jointjs paper reports zero-sized geometry and every navigator assertion would
+ * read "0px" — passing because there is no layout rather than because the
+ * formula is right. This stub reports explicit, distinguishable geometry
+ * instead, and records the calls the component makes against it.
+ */
+class StubPaper {
+ public readonly handlers: Record<string, () => void> = {};
+ public readonly pageToLocalPointArgs: { x: number; y: number }[] = [];
+ public readonly translateArgs: [number, number][] = [];
+ /** Local-coordinate point the next `pageToLocalPoint` call resolves to. */
+ public localPoint = { x: 0, y: 0 };
+ /** Current paper offset reported by the no-arg `translate()` getter. */
+ public offset = { tx: 0, ty: 0 };
+
+ constructor(
+ private readonly sx: number = 1,
+ private readonly sy: number = 1
+ ) {}
+
+ on(event: string, handler: () => void): void {
+ this.handlers[event] = handler;
+ }
+
+ scale(): { sx: number; sy: number } {
+ return { sx: this.sx, sy: this.sy };
+ }
+
+ pageToLocalPoint(point: { x: number; y: number }): { x: number; y: number } {
+ this.pageToLocalPointArgs.push(point);
+ return this.localPoint;
+ }
+
+ translate(tx?: number, ty?: number): { tx: number; ty: number } | void {
+ if (tx === undefined) return this.offset;
+ this.translateArgs.push([tx, ty as number]);
+ }
+}
+
+/**
+ * Regression coverage for the mini-map: the persisted show/hide flag, the
+ * navigator overlay geometry (which mirrors the main paper's viewport onto the
+ * mini-map), drag-to-pan, and the zoom/center toolbar buttons.
+ *
+ * Breakage this catches: dropping/renaming the "mini-map" localStorage key so
+ * the collapsed state no longer survives a reload; unsubscribing the mini-map
+ * from the main paper's translate/scale/resize events so the navigator freezes;
+ * sign or scale errors in the navigator-position and drag-to-pan formulas;
+ * losing the zoom-limit guards so the toolbar can push the zoom ratio past
+ * ZOOM_MINIMUM / ZOOM_MAXIMUM; and the panel service's close/reset streams no
+ * longer hiding/showing the mini-map.
+ */
describe("MiniMapComponent", () => {
let fixture: ComponentFixture<MiniMapComponent>;
+ let component: MiniMapComponent;
+ let workflowActionService: WorkflowActionService;
+ let panelService: PanelService;
+ let editorStub: HTMLDivElement | undefined;
+ let mainPaper$: ReplaySubject<joint.dia.Paper>;
+
+ /**
+ * `localStorage` is a jsdom global shared by every test in this file, and
+ * Angular's automatic fixture teardown runs `ngOnDestroy` — which writes
+ * `mini-map` — after each one. Seed and clear it explicitly so the persisted
+ * flag under test is the one this test wrote, not the previous test's.
+ */
+ beforeEach(() => {
+ localStorage.removeItem("mini-map");
+ });
+
+ afterEach(() => {
+ localStorage.removeItem("mini-map");
+ editorStub?.remove();
+ editorStub = undefined;
+ });
beforeEach(async () => {
await TestBed.configureTestingModule({
@@ -49,13 +131,285 @@ describe("MiniMapComponent", () => {
}).compileComponents();
});
+ // The fixture is created but NOT change-detected here: ngAfterViewInit reads
+ // the mini-map container's size and the persisted flag, so each test sets its
+ // own environment up before triggering it.
beforeEach(() => {
fixture = TestBed.createComponent(MiniMapComponent);
- TestBed.inject(WorkflowActionService);
- fixture.detectChanges();
+ component = fixture.componentInstance;
+ workflowActionService = TestBed.inject(WorkflowActionService);
+ panelService = TestBed.inject(PanelService);
+
+ // Own the paper stream rather than casting the wrapper's getter back to a Subject.
+ // `getMainJointPaperAttachedStream()` is declared `Observable<Paper>`, so calling
+ // `.next()` on its result only works because it happens to be a ReplaySubject today;
+ // switching it to `.asObservable()` would break the spec silently. Stubbing the
+ // getter depends on the declared type only. ReplaySubject(1) so a paper attached
+ // before the component subscribes in ngAfterViewInit is still delivered.
+ mainPaper$ = new ReplaySubject<joint.dia.Paper>(1);
+ vi.spyOn(workflowActionService.getJointGraphWrapper(), "getMainJointPaperAttachedStream").mockReturnValue(
+ mainPaper$.asObservable()
+ );
});
+ /** Gives the mini-map container a size, which jsdom otherwise reports as 0. */
+ function sizeMiniMapContainer(width: number, height: number): HTMLElement {
+ const map = fixture.nativeElement.querySelector("#mini-map") as HTMLElement;
+ Object.defineProperty(map, "offsetWidth", { value: width, configurable: true });
+ Object.defineProperty(map, "offsetHeight", { value: height, configurable: true });
+ return map;
+ }
+
+ /**
+ * The mini-map reads the main editor's element out of the document by id, so
+ * mount a stand-in with an explicit size and viewport rect.
+ */
+ function mountWorkflowEditorStub(width: number, height: number, left: number, top: number): HTMLDivElement {
+ const editor = document.createElement("div");
+ editor.id = "workflow-editor";
+ Object.defineProperty(editor, "offsetWidth", { value: width, configurable: true });
+ Object.defineProperty(editor, "offsetHeight", { value: height, configurable: true });
+ editor.getBoundingClientRect = () => ({ left, top, right: left + width, bottom: top + height }) as DOMRect;
+ document.body.appendChild(editor);
+ editorStub = editor;
+ return editor;
+ }
+
+ /** Publishes `paper` on the stream the mini-map subscribes to in ngAfterViewInit. */
+ function attachMainPaper(paper: StubPaper): void {
+ mainPaper$.next(paper as unknown as joint.dia.Paper);
+ }
+
it("should create", () => {
+ fixture.detectChanges();
expect(fixture.componentInstance).toBeTruthy();
});
+
+ describe("mini-map paper", () => {
+ it("fits the whole main canvas into the mini-map container", () => {
+ // 912 / (2688 - -960) == 0.25; the height (100) is deliberately different
+ // so a width/height mix-up in the scale formula cannot pass.
+ const map = sizeMiniMapContainer(912, 100);
+
+ fixture.detectChanges();
+
+ expect(MAIN_CANVAS.xMax - MAIN_CANVAS.xMin).toBe(3648);
+ expect(component.scale).toBe(0.25);
+ // The paper is sized to the container it renders into ...
+ expect(map.style.width).toBe("912px");
+ expect(map.style.height).toBe("100px");
+ // ... and shifted so the canvas' top-left corner (-960, -540) lands on the
+ // container's origin: 960 * 0.25 == 240, 540 * 0.25 == 135.
+ expect(map.querySelector("g.joint-layers")?.getAttribute("transform")).toBe("matrix(0.25,0,0,0.25,240,135)");
+ });
+ });
+
+ describe("persisted hidden state", () => {
+ it("starts visible when nothing has been persisted", () => {
+ fixture.detectChanges();
+ expect(component.hidden).toBe(false);
+ });
+
+ it("restores the collapsed state persisted by the previous session", () => {
+ localStorage.setItem("mini-map", "true");
+
+ // `hidden` is assigned inside ngAfterViewInit, i.e. after the template
+ // that reads it has been checked, so the dev-mode verification pass would
+ // report NG0100 for a state the component legitimately restores. Skip it.
+ fixture.detectChanges(false);
+
+ expect(component.hidden).toBe(true);
+ });
+
+ it("persists the collapsed state on destroy and restores it into a fresh instance", () => {
+ fixture.detectChanges();
+ component.hidden = true;
+
+ fixture.destroy();
+
+ // Round-trip through the real storage key: a rename on either the write
+ // or the read side breaks this.
+ expect(localStorage.getItem("mini-map")).toBe("true");
+ const reopened = TestBed.createComponent(MiniMapComponent);
+ reopened.detectChanges(false);
+ expect(reopened.componentInstance.hidden).toBe(true);
+ });
+
+ it("persists the collapsed state when the window unloads", () => {
+ fixture.detectChanges();
+ component.hidden = true;
+
+ // @HostListener("window:beforeunload") — the tab can close without Angular
+ // ever destroying the component, so the flag has to be written here too.
+ window.dispatchEvent(new Event("beforeunload"));
+
+ expect(localStorage.getItem("mini-map")).toBe("true");
+ });
+ });
+
+ describe("panel service integration", () => {
+ it("hides on closePanels and shows again on resetPanels", () => {
+ fixture.detectChanges();
+ expect(component.hidden).toBe(false);
+
+ panelService.closePanels();
+ expect(component.hidden).toBe(true);
+
+ panelService.resetPanels();
+ expect(component.hidden).toBe(false);
+ });
+ });
+
+ describe("navigator overlay", () => {
+ const MINI_MAP_SCALE = 0.25;
+
+ /**
+ * Wires a stub main paper to a change-detected component and pins an
+ * explicit mini-map scale (ngAfterViewInit computes 0 under jsdom, which
+ * would make every "0px" assertion vacuously true).
+ */
+ function attachStubbedViewport(): { paper: StubPaper; navigator: HTMLElement } {
+ mountWorkflowEditorStub(800, 600, 30, 40);
+ fixture.detectChanges();
+ component.scale = MINI_MAP_SCALE;
+
+ const navigator = document.getElementById("mini-map-navigator") as HTMLElement;
+ // cdkDrag leaves a transform behind after a drag; the component must clear
+ // it before writing left/top, or the two offsets would stack.
+ navigator.style.transform = "translate3d(11px, 13px, 0)";
+
+ // sx and sy differ so a width/height mix-up cannot pass.
+ const paper = new StubPaper(2, 4);
+ paper.localPoint = { x: -160, y: -140 };
+ attachMainPaper(paper);
+
+ return { paper, navigator };
+ }
+
+ it("positions and sizes the navigator from the main paper's viewport", () => {
+ const { paper, navigator } = attachStubbedViewport();
+
+ expect(component.paper).toBe(paper as unknown as joint.dia.Paper);
+ // The viewport origin is the editor's top-left corner in page coordinates.
+ expect(paper.pageToLocalPointArgs).toEqual([{ x: 30, y: 40 }]);
+ // (-160 - -960) * 0.25 and (-140 - -540) * 0.25
+ expect(navigator.style.left).toBe("200px");
+ expect(navigator.style.top).toBe("100px");
+ // (800 / 2) * 0.25 and (600 / 4) * 0.25
+ expect(navigator.style.width).toBe("100px");
+ expect(navigator.style.height).toBe("37.5px");
+ expect(navigator.style.transform).toBe("");
+ });
+
+ it("repositions the navigator when the main paper translates, scales or resizes", () => {
+ const { paper, navigator } = attachStubbedViewport();
+ expect(Object.keys(paper.handlers).sort()).toEqual(["resize", "scale", "translate"]);
+
+ const movements: [string, number, string][] = [
+ ["translate", -560, "100px"],
+ ["scale", -360, "150px"],
+ ["resize", -60, "225px"],
+ ];
+ for (const [event, localX, expectedLeft] of movements) {
+ paper.localPoint = { x: localX, y: -140 };
+ paper.handlers[event]();
+ expect(navigator.style.left).toBe(expectedLeft);
+ }
+ });
+
+ it("leaves the navigator alone while the user is dragging it", () => {
+ const { paper, navigator } = attachStubbedViewport();
+ expect(navigator.style.left).toBe("200px");
+
+ // The drag itself is already moving the navigator; echoing the paper's
+ // translate back onto it would fight the pointer.
+ component.dragging = true;
+ paper.localPoint = { x: -560, y: -140 };
+ paper.handlers["translate"]();
+
+ expect(navigator.style.left).toBe("200px");
+ });
+ });
+
+ describe("drag to pan", () => {
+ it("pans the main paper opposite the pointer, in main-canvas units", () => {
+ fixture.detectChanges();
+ const paper = new StubPaper();
+ paper.offset = { tx: 100, ty: 50 };
+ component.paper = paper as unknown as joint.dia.Paper;
+ component.scale = 0.25;
+
+ component.onDrag({ event: { movementX: 10, movementY: -20 } });
+
+ // A pointer delta on the mini-map is worth 1/scale as much on the main
+ // canvas, and the paper moves against the pointer.
+ expect(paper.translateArgs).toEqual([[100 - 40, 50 + 80]]);
+ });
+ });
+
+ describe("zoom buttons", () => {
+ let jointGraphWrapper: JointGraphWrapper;
+
+ beforeEach(() => {
+ fixture.detectChanges();
+ jointGraphWrapper = workflowActionService.getJointGraphWrapper();
+ });
+
+ it("zooms out by one click step", () => {
+ jointGraphWrapper.setZoomProperty(1);
+
+ component.onClickZoomOut();
+
+ expect(jointGraphWrapper.getZoomRatio()).toBeCloseTo(1 - JointGraphWrapper.ZOOM_CLICK_DIFF, 10);
+ expect(jointGraphWrapper.getZoomRatio()).toBeLessThan(1);
+ });
+
+ it("does not zoom out past the minimum ratio", () => {
+ jointGraphWrapper.setZoomProperty(JointGraphWrapper.ZOOM_MINIMUM);
+ const setZoomProperty = vi.spyOn(jointGraphWrapper, "setZoomProperty");
+
+ component.onClickZoomOut();
+
+ expect(setZoomProperty).not.toHaveBeenCalled();
+ expect(jointGraphWrapper.getZoomRatio()).toBe(JointGraphWrapper.ZOOM_MINIMUM);
+ });
+
+ it("zooms in by one click step", () => {
+ jointGraphWrapper.setZoomProperty(1);
+
+ component.onClickZoomIn();
+
+ expect(jointGraphWrapper.getZoomRatio()).toBeCloseTo(1 + JointGraphWrapper.ZOOM_CLICK_DIFF, 10);
+ expect(jointGraphWrapper.getZoomRatio()).toBeGreaterThan(1);
+ });
+
+ it("does not zoom in past the maximum ratio", () => {
+ jointGraphWrapper.setZoomProperty(JointGraphWrapper.ZOOM_MAXIMUM);
+ const setZoomProperty = vi.spyOn(jointGraphWrapper, "setZoomProperty");
+
+ component.onClickZoomIn();
+
+ expect(setZoomProperty).not.toHaveBeenCalled();
+ expect(jointGraphWrapper.getZoomRatio()).toBe(JointGraphWrapper.ZOOM_MAXIMUM);
+ });
+ });
+
+ describe("center button", () => {
+ it("broadcasts a center event and resets the navigator's drag offset", () => {
+ fixture.detectChanges();
+ const centerEvents: void[] = [];
+ workflowActionService
+ .getTexeraGraph()
+ .getCenterEventStream()
+ .subscribe(event => centerEvents.push(event));
+ const reset = vi.spyOn(component.navigatorDrag, "reset");
+
+ component.triggerCenter();
+
+ expect(centerEvents).toHaveLength(1);
+ // Without the reset the navigator keeps the cdkDrag transform from the
+ // previous drag and lands off-centre.
+ expect(reset).toHaveBeenCalledTimes(1);
+ });
+ });
});
diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
index c60b06d368..43347dfdf1 100644
--- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
+++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.spec.ts
@@ -26,6 +26,20 @@ import { NotebookMigrationService } from "../notebook-migration/notebook-migrati
import { GuiConfigService } from "src/app/common/service/gui-config.service";
import { firstValueFrom, of, throwError } from "rxjs";
+/**
+ * Regression coverage for the Jupyter notebook panel service: the per-workflow
+ * notebook fetch, the cell-to-operator highlight index, the iframe message
+ * bridge, and the feature-flag gate.
+ *
+ * Breakage this catches: reporting a notebook as present when Jupyter refused
+ * to load it (the toolbar would offer to expand a panel that has nothing in
+ * it); caching a failed Jupyter-origin lookup, which leaves the bridge dead for
+ * the rest of the session; dropping the guards that keep an unsaved workflow
+ * (no wid) or a workflow with no stored mapping from being looked up or posted
+ * to; leaving a previous cell's highlights on the canvas when an unmapped cell
+ * is clicked; and losing the feature-flag early-return in the window message
+ * listener, which is installed unconditionally in the constructor.
+ */
describe("JupyterPanelService", () => {
let service: JupyterPanelService;
let httpMock: HttpTestingController;
@@ -95,6 +109,9 @@ describe("JupyterPanelService", () => {
afterEach(() => {
httpMock.verify();
+ // Several specs below silence console.error/warn to keep the failure paths
+ // quiet; console is a shared global, so put the originals back.
+ vi.restoreAllMocks();
});
// Panel visibility
@@ -227,6 +244,46 @@ describe("JupyterPanelService", () => {
expect(await resultPromise).toBe(0);
});
+ // The fetch pipeline has two distinct ways to yield 0 — Jupyter refusing the
+ // notebook, and the request itself failing — and the second one is a
+ // catchError that swallows *any* throw inside the switchMap. Pinning them
+ // apart (via sendNotebookToJupyter and the console.error the catchError
+ // emits) is what keeps a broken happy path from masquerading as "send
+ // failed": the base mock has no sendNotebookToJupyter, so a spec that forgets
+ // to define it gets a TypeError converted into the very same 0.
+ it("returns 0 when Jupyter rejects the notebook, without going through the error path", async () => {
+ mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(0);
+ const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+ const mapping = { cell_to_operator: { cell1: ["A"] }, operator_to_cell: {} };
+ const notebook = { cells: [] };
+
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1));
+ httpMock
+ .expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
+ .flush({ exists: true, mapping, notebook });
+
+ expect(await resultPromise).toBe(0);
+ // The mapping is stored before the notebook is handed to Jupyter, ...
+ expect(mockNotebook.setMapping).toHaveBeenCalledWith("mapping_wid_1", mapping);
+ // ... and the 0 came from Jupyter's own answer, not from a thrown error.
+ expect(mockNotebook.sendNotebookToJupyter).toHaveBeenCalledWith(notebook);
+ expect(consoleError).not.toHaveBeenCalled();
+ });
+
+ it("returns 0 and logs when the fetch request fails", async () => {
+ mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(1);
+ const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+
+ const resultPromise = firstValueFrom((service as any).fetchNotebookAndMapping(1, 1));
+ httpMock
+ .expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
+ .flush("migration service down", { status: 500, statusText: "Server Error" });
+
+ expect(await resultPromise).toBe(0);
+ expect(consoleError).toHaveBeenCalled();
+ expect(mockNotebook.sendNotebookToJupyter).not.toHaveBeenCalled();
+ });
+
// jupyterNotebookExists$ starts false and flips true once init()'s fetch finds
// a notebook for the workflow; the toolbar's expand button binds to this.
it("sets jupyterNotebookExists$ true after a workflow's notebook is fetched", async () => {
@@ -244,6 +301,26 @@ describe("JupyterPanelService", () => {
expect(states.at(-1)).toBe(true); // true once the notebook is found
});
+ // The stored notebook exists but Jupyter refuses it: the toolbar must not
+ // advertise a notebook the panel cannot actually show.
+ it("leaves the panel closed when the stored notebook cannot be loaded into Jupyter", async () => {
+ mockNotebook.sendNotebookToJupyter = vi.fn().mockResolvedValue(0);
+ const exists: boolean[] = [];
+ const visible: boolean[] = [];
+ service.jupyterNotebookExists$.subscribe(v => exists.push(v));
+ service.jupyterNotebookPanelVisible$.subscribe(v => visible.push(v));
+
+ service.init();
+ httpMock
+ .expectOne(r => r.url.includes("/notebook-migration/fetch-notebook-and-mapping"))
+ .flush({ exists: true, mapping: { cell_to_operator: {}, operator_to_cell: {} }, notebook: {} });
+ await new Promise(resolve => setTimeout(resolve, 0));
+
+ expect(mockNotebook.sendNotebookToJupyter).toHaveBeenCalled();
+ expect(exists.at(-1)).toBe(false);
+ expect(visible.at(-1)).toBe(false);
+ });
+
// init(): subscribes to workflow changes, drops the stale mapping for the
// current workflow, and fetches the incoming workflow's notebook + mapping.
it("init subscribes, drops the stale mapping, and fetches for the new workflow", () => {
@@ -311,6 +388,20 @@ describe("JupyterPanelService", () => {
expect(mockWorkflow.highlightLinks).toHaveBeenCalledWith(true, "link1");
});
+ // A cell that isn't in the index still has to clear the previous cell's
+ // highlights, otherwise clicking an unmapped cell leaves the old selection on
+ // the canvas.
+ it("clears existing highlights and highlights nothing for an unmapped cell", () => {
+ (service as any).cellToHighlightMapping = { cell1: { components: ["op1"], edges: ["link1"] } };
+
+ (service as any).highlightFromCell("unmappedCell");
+
+ expect(mockWorkflow.unhighlightOperators).toHaveBeenCalledWith("A", "B");
+ expect(mockWorkflow.unhighlightLinks).toHaveBeenCalledWith("L1");
+ expect(mockWorkflow.highlightOperators).not.toHaveBeenCalled();
+ expect(mockWorkflow.highlightLinks).not.toHaveBeenCalled();
+ });
+
// handleNotebookMessage must only act on cellClicked messages that come from
// our own iframe (event.source) AND carry the Jupyter origin.
it("handleNotebookMessage highlights only for messages from the iframe at the Jupyter origin", async () => {
@@ -332,6 +423,27 @@ describe("JupyterPanelService", () => {
expect(highlightSpy).toHaveBeenCalledWith("c1");
});
+ // postMessage can deliver a payload-less event; destructuring it must not
+ // throw out of the (async, unawaited) listener.
+ it("handleNotebookMessage ignores a message that carries no payload", async () => {
+ const iframeWindow = {} as Window;
+ service.setIframeRef({ contentWindow: iframeWindow } as any);
+ const highlightSpy = vi.spyOn(service as any, "highlightFromCell").mockImplementation(() => {});
+
+ await (service as any).handleNotebookMessage({
+ source: iframeWindow,
+ origin: "http://jupyter",
+ data: undefined,
+ });
+
+ // The awaited call above is what proves the payload destructuring survived the
+ // missing data: drop the `event.data ?? {}` fallback and the async handler rejects,
+ // failing this test before either assertion runs. These two then pin that the
+ // guards still passed (source + origin are ours) and that no highlight was issued.
+ expect(mockNotebook.getJupyterURL).toHaveBeenCalled();
+ expect(highlightSpy).not.toHaveBeenCalled();
+ });
+
// A workflow with operators but no links is valid; precompute must still
// record each cell's components (with empty edges) so cell clicks highlight.
it("precomputes component mappings even when the graph has no links", () => {
@@ -376,6 +488,49 @@ describe("JupyterPanelService", () => {
});
});
+ // An unsaved workflow has no wid, so there is no mapping key to look up; the
+ // precompute must drop the stale index instead of querying "mapping_wid_undefined".
+ it("drops the highlight index without a lookup when the workflow has no wid", () => {
+ mockWorkflow.getWorkflow.mockReturnValue({ wid: undefined });
+ (service as any).cellToHighlightMapping = { stale: { components: ["X"], edges: [] } };
+ const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+ (service as any).precomputeHighlightMapping();
+
+ expect(mockNotebook.getMapping).not.toHaveBeenCalled();
+ expect((service as any).cellToHighlightMapping).toEqual({});
+ expect(consoleWarn).toHaveBeenCalled();
+ });
+
+ // The notebook was found but its mapping was never stored locally: leave the
+ // index empty rather than dereferencing the missing mapping.
+ it("drops the highlight index when no mapping is stored for the workflow", () => {
+ mockNotebook.getMapping.mockReturnValue(undefined);
+ (service as any).cellToHighlightMapping = { stale: { components: ["X"], edges: [] } };
+ const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+ (service as any).precomputeHighlightMapping();
+
+ expect(mockNotebook.getMapping).toHaveBeenCalledWith("mapping_wid_1");
+ expect((service as any).cellToHighlightMapping).toEqual({});
+ expect(consoleWarn).toHaveBeenCalled();
+ });
+
+ // A cell mapped to nothing must index as an empty component list; a raw
+ // undefined would blow up the later `.length` check in highlightFromCell.
+ it("indexes a cell with no mapped operators as an empty selection", () => {
+ mockNotebook.getMapping.mockReturnValue({
+ cell_to_operator: { emptyCell: null },
+ operator_to_cell: {},
+ });
+
+ (service as any).precomputeHighlightMapping();
+
+ expect((service as any).cellToHighlightMapping).toEqual({
+ emptyCell: { components: [], edges: [] },
+ });
+ });
+
// onWorkflowComponentClick
it("should postMessage when mapping exists", async () => {
const mockIframe = {
@@ -437,6 +592,73 @@ describe("JupyterPanelService", () => {
expect(mockNotebook.getJupyterURL).toHaveBeenCalledTimes(1);
});
+ it("does not postMessage for an unsaved workflow (undefined wid)", async () => {
+ const mockIframe = { contentWindow: { postMessage: vi.fn() } } as any;
+ service.setIframeRef(mockIframe);
+ mockWorkflow.getWorkflow.mockReturnValue({ wid: undefined });
+ const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+
+ await service.onWorkflowComponentClick("op1");
+
+ // Bailing before the lookup keeps a "mapping_wid_undefined" key from being read.
+ expect(mockNotebook.getMapping).not.toHaveBeenCalled();
+ expect(mockIframe.contentWindow.postMessage).not.toHaveBeenCalled();
+ expect(consoleError).toHaveBeenCalled();
+ });
+
+ it("does not postMessage when the workflow has no stored mapping", async () => {
+ const mockIframe = { contentWindow: { postMessage: vi.fn() } } as any;
+ service.setIframeRef(mockIframe);
+ mockNotebook.getMapping.mockReturnValue(undefined);
+ const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+
+ await service.onWorkflowComponentClick("op1");
+
+ expect(mockNotebook.getMapping).toHaveBeenCalledWith("mapping_wid_1");
+ expect(mockIframe.contentWindow.postMessage).not.toHaveBeenCalled();
+ expect(consoleError).toHaveBeenCalled();
+ });
+
+ // The Jupyter pod may not be reachable yet when the first click lands. A
+ // failed lookup must NOT be cached, or the panel stays dead for the rest of
+ // the session.
+ it("retries the origin lookup after an unavailable Jupyter URL", async () => {
+ const mockIframe = { contentWindow: { postMessage: vi.fn() } } as any;
+ service.setIframeRef(mockIframe);
+ mockNotebook.getMapping.mockReturnValue({
+ cell_to_operator: {},
+ operator_to_cell: { op1: ["cell1"] },
+ });
+ mockNotebook.getJupyterURL.mockResolvedValueOnce(null);
+
+ await service.onWorkflowComponentClick("op1");
+ expect(mockIframe.contentWindow.postMessage).not.toHaveBeenCalled();
+
+ await service.onWorkflowComponentClick("op1");
+
+ expect(mockNotebook.getJupyterURL).toHaveBeenCalledTimes(2);
+ expect(mockIframe.contentWindow.postMessage).toHaveBeenCalledWith(
+ { action: "triggerCellClick", operators: ["cell1"] },
+ "http://jupyter"
+ );
+ });
+
+ it("treats a malformed Jupyter URL as unavailable and keeps retrying", async () => {
+ const mockIframe = { contentWindow: { postMessage: vi.fn() } } as any;
+ service.setIframeRef(mockIframe);
+ mockNotebook.getMapping.mockReturnValue({
+ cell_to_operator: {},
+ operator_to_cell: { op1: ["cell1"] },
+ });
+ mockNotebook.getJupyterURL.mockResolvedValue("://not-a-url");
+
+ await service.onWorkflowComponentClick("op1");
+ await service.onWorkflowComponentClick("op1");
+
+ expect(mockIframe.contentWindow.postMessage).not.toHaveBeenCalled();
+ expect(mockNotebook.getJupyterURL).toHaveBeenCalledTimes(2);
+ });
+
// Feature flag gate (defence in depth). With the flag off, init must not
// subscribe to workflow changes, and onWorkflowComponentClick must not
// postMessage to the iframe. The window message listener is installed in
@@ -491,5 +713,22 @@ describe("JupyterPanelService", () => {
await service.onWorkflowComponentClick("cell1");
expect(mockIframe.contentWindow.postMessage).not.toHaveBeenCalled();
});
+
+ it("handleNotebookMessage ignores a cellClicked message from our own iframe", async () => {
+ const iframeWindow = {} as Window;
+ service.setIframeRef({ contentWindow: iframeWindow } as any);
+ const highlightSpy = vi.spyOn(service as any, "highlightFromCell").mockImplementation(() => {});
+
+ await (service as any).handleNotebookMessage({
+ source: iframeWindow,
+ origin: "http://jupyter",
+ data: { action: "cellClicked", cellUUID: "c1" },
+ });
+
+ expect(highlightSpy).not.toHaveBeenCalled();
+ // The message carried our own source and origin, so it was the flag check
+ // that stopped it — the origin was never even resolved.
+ expect(mockNotebook.getJupyterURL).not.toHaveBeenCalled();
+ });
});
});
diff --git a/frontend/src/app/workspace/service/preset/preset.service.spec.ts b/frontend/src/app/workspace/service/preset/preset.service.spec.ts
index b71c976515..51a24132a2 100644
--- a/frontend/src/app/workspace/service/preset/preset.service.spec.ts
+++ b/frontend/src/app/workspace/service/preset/preset.service.spec.ts
@@ -42,6 +42,17 @@ if (!ajvInstance.getKeyword("enable-presets")) {
ajvInstance.addKeyword({ keyword: "enable-presets", schemaType: "boolean" });
}
+/**
+ * Regression coverage for PresetService: the dictionary-backed read/write of
+ * operator presets, the toast the user sees for each write, preset validation
+ * against the operator's 'enable-presets' schema, and the static schema helpers.
+ *
+ * Breakage this catches: losing the empty-list default so the very first preset
+ * saved for an operator type parses `null` and throws; routing a save/delete
+ * toast to the wrong NzMessageService severity, or dropping the caller's
+ * message so the user sees the generic default; and accepting an operator
+ * schema that has no `properties` at all instead of rejecting it.
+ */
describe("PresetService", () => {
let userConfigStub: {
fetchKey: ReturnType<typeof vi.fn>;
@@ -163,6 +174,37 @@ describe("PresetService", () => {
expect(messageStub.error).not.toHaveBeenCalled();
});
+ it("routes an explicitly passed message to the NzMessageService method for its severity", () => {
+ const severities = ["error", "info", "success", "warning"] as const;
+
+ for (const severity of severities) {
+ presetService.savePresets(
+ presetType,
+ presetTarget,
+ [{ presetProperty: "v1" }],
+ `saved as ${severity}`,
+ severity
+ );
+ }
+
+ for (const severity of severities) {
+ // exactly one toast per severity: a mis-wired case would show up here as
+ // a missing call on one method and a doubled call on another.
+ expect(messageStub[severity]).toHaveBeenCalledTimes(1);
+ expect(messageStub[severity]).toHaveBeenCalledWith(`saved as ${severity}`);
+ }
+ });
+
+ it("deletePreset reports the caller's message as an error toast", () => {
+ userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }])));
+
+ // PresetWrapperComponent.deletePreset passes exactly this shape.
+ presetService.deletePreset(presetType, presetTarget, { presetProperty: "v1" }, "Deleted preset: v1", "error");
+
+ expect(messageStub.error).toHaveBeenCalledWith("Deleted preset: v1");
+ expect(messageStub.success).not.toHaveBeenCalled();
+ });
+
it("createPreset appends to existing presets and writes back", () => {
const existing: Preset[] = [{ presetProperty: "v1" }];
userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify(existing)));
@@ -175,6 +217,16 @@ describe("PresetService", () => {
);
});
+ it("createPreset stores the first preset when the dictionary has no entry yet", () => {
+ // The very first save for an operator type: fetchKey resolves to null, and
+ // the missing entry has to read as an empty list rather than being parsed.
+ userConfigStub.fetchKey.mockReturnValue(of(null));
+
+ presetService.createPreset(presetType, presetTarget, { presetProperty: "v1" });
+
+ expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify([{ presetProperty: "v1" }]));
+ });
+
it("createPreset does not write the preset back when it already exists", async () => {
userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }])));
@@ -394,6 +446,14 @@ describe("PresetService", () => {
).toThrow();
});
+ it("getOperatorPresetSchema throws when the operator schema omits properties entirely", () => {
+ // Distinct from the empty-`properties` case above, which fails the later
+ // "no preset properties" check instead.
+ expect(() => PresetService.getOperatorPresetSchema(<CustomJSONSchema7>{ type: "object" })).toThrow(
+ /has no properties$/
+ );
+ });
+
it("getOperatorPresetSchema throws when no property is preset-enabled", () => {
expect(() =>
PresetService.getOperatorPresetSchema(<CustomJSONSchema7>{