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-7541-dcecfa69fd9a94319f931221e7f99420e9d5970b in repository https://gitbox.apache.org/repos/asf/texera.git
commit af38ca99c13c0be4fbe69cc168c82d69aae36c56 Author: Meng Wang <[email protected]> AuthorDate: Mon Aug 10 23:42:26 2026 -0700 test(frontend): cover the report-generation image pipeline and shared-editing branches (#7541) ### What changes were proposed in this PR? 18 new tests across two units. | file | statements | branches | functions | | --- | --- | --- | --- | | `report-generation.service.ts` | 126/129 (was 98/129) | 25/25 | 34/35 | | `shared-editing.interface.ts` | 115/115 | 79/80 | 6/6 | **ReportGenerationService** — `fetchImageAsBase64` and the loop that feeds it were entirely unhit. Covered: an image whose bytes convert (its `href` is rewritten to the base64 result), one whose `FileReader` fails, one whose request fails, and one with no source at all (skipped without a request). `XMLHttpRequest` is replaced with a fake that settles synchronously and `FileReader` with one that fires on a microtask, so nothing depends on the network or on real timing; both globals are restored afterwards. Also covered while the report was in view: the two `|| "Unknown error"` fallbacks, the outer `catch` that reports a failure to build the report at all, and the visualization snapshot that has no wrapper `div` to resize. That takes branch coverage of the file to 25/25. **shared-editing.interface.ts** — the file is `createYTypeFromObject` / `updateYTypeFromObject` over Yjs types, so the tests drive real `Y.Doc`s in memory. Covered: the `typeof` arms no caller passes (function, symbol, bigint), the boxed-String arms of both functions, the no-op when a string is already up to date, an `undefined` array entry becoming `null`, an array element whose kind changes being replaced rather than merged, an array whose additions sit before its removals, and the `return false` for a type the dispatch has no strategy for. No production code was changed. ### Coverage that is not reachable - `report-generation.service.ts:93-95` — the html2canvas success callback. jsdom has no canvas, and reaching it means standing in a CSS engine, a 2D context, `toDataURL` and the image loader; that is a simulated renderer rather than a test, and it would break on an html2canvas or jsdom upgrade. Left alone deliberately. - `shared-editing.interface.ts:223` — the `_.isEqual` guard's false arm, i.e. an equal pair at the same offset *inside* an unmatched segment. A script replicating the LCS walk and segment construction found none across all 1,185,921 array pairs of length 2-6 over a 3-value alphabet, so the alignment appears to preclude it. Note that html2canvas still runs for real in the image tests and jsdom cannot render it, so those tests emit `Not implemented` notices on stderr. They are jsdom's, not failures — the tests assert on what the inlining step did, not on the render. ### One defect worth recording `createYTypeFromObject(new String("x"))` returns an **empty** `Y.Text`: `new Y.Text(...)` only accepts a primitive, so the boxed value is dropped. The update path does not share the bug — `Y.Text.insert` coerces — so the same input round-trips correctly through `updateYTypeFromObject`. The test asserts the real behaviour with a comment saying what to flip when the branch unwraps the box. ### Any related issues, documentation, discussions? Closes #7538. Note: the issue describes this file as a `switch` over shared-editing events with awareness state; the file is actually the two YType conversion functions above, with an LCS-based array diff. The tests follow the code. ### How was this PR tested? `ng test --watch=false` over the two specs — 52 passed (34 existing + 18 new), run 3x for determinism. Coverage (`--coverage`) gives the table above. The failure path was verified by breaking one assertion in each spec (red, non-zero exit) and restoring them; eslint and prettier are clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../report-generation.service.spec.ts | 186 +++++++++++++++++++++ .../types/shared-editing.interface.spec.ts | 117 +++++++++++++ 2 files changed, 303 insertions(+) diff --git a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts index f6d33c55c9..21617223c8 100644 --- a/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts +++ b/frontend/src/app/workspace/service/report-generation/report-generation.service.spec.ts @@ -175,6 +175,50 @@ describe("ReportGenerationService", () => { expect(html).toContain("No results found for operator"); }); + it("falls back to a generic reason when the page failure carries no message", async () => { + deps.workflowResultService.getPaginatedResultService.mockReturnValue({ + selectPage: vi.fn().mockReturnValue(throwError(() => new Error(""))), + }); + + await expect(htmlFor("op-1")).rejects.toBeInstanceOf(Error); + expect(deps.notificationService.error).toHaveBeenCalledWith( + "Error processing results for operator op-1: Unknown error" + ); + }); + + it("still renders a visualization snapshot that has no wrapper div to resize", async () => { + deps.workflowResultService.getResultService.mockReturnValue({ + getCurrentResultSnapshot: () => [{ "html-content": "<p>plain</p>" }], + }); + + const html = await htmlFor("op-1"); + + expect(html).toContain("plain"); + }); + + it("notifies and fails when building the report throws outright", async () => { + const failure = new Error("result service unavailable"); + deps.workflowResultService.getResultService.mockImplementation(() => { + throw failure; + }); + + await expect(htmlFor("op-1")).rejects.toBe(failure); + expect(deps.notificationService.error).toHaveBeenCalledWith( + "Unexpected error in retrieveOperatorInfoReport for operator op-1: result service unavailable" + ); + }); + + it("falls back to a generic reason when that failure carries no message", async () => { + deps.workflowResultService.getResultService.mockImplementation(() => { + throw new Error(""); + }); + + await expect(htmlFor("op-1")).rejects.toBeInstanceOf(Error); + expect(deps.notificationService.error).toHaveBeenCalledWith( + "Unexpected error in retrieveOperatorInfoReport for operator op-1: Unknown error" + ); + }); + it("embeds the operator's own definition in the collapsible details block", async () => { deps.workflowActionService.getWorkflowContent.mockReturnValue({ operators: [ @@ -291,5 +335,147 @@ describe("ReportGenerationService", () => { "Workflow editor element not found" ); }); + + /** + * Before the editor can be rendered, every <image> in it is refetched and inlined as + * base64 so the snapshot does not depend on URLs the renderer cannot resolve. Both async + * sources are replaced with fakes that settle synchronously (XHR) or on a microtask + * (FileReader), so nothing here depends on the network or on real timing. + * + * The html2canvas render that follows is left alone — it needs a real canvas — so these + * assert on what the inlining step did, not on the observable's outcome. + */ + describe("inlining the editor's images", () => { + const XLINK_HREF = "xlink:href"; + const BASE64 = "data:image/png;base64,AAAA"; + + let realXhr: typeof globalThis.XMLHttpRequest; + let realFileReader: typeof globalThis.FileReader; + let editor: HTMLElement; + let xhrOutcome: "load" | "error"; + let readerOutcome: "loadend" | "error"; + let sentUrls: string[]; + + class FakeXhr { + public response: unknown = "blob-stand-in"; + public responseType = ""; + public onload: (() => void) | null = null; + public onerror: (() => void) | null = null; + private url = ""; + open(_method: string, url: string): void { + this.url = url; + } + send(): void { + sentUrls.push(this.url); + if (xhrOutcome === "load") { + this.onload?.(); + } else { + this.onerror?.(); + } + } + } + + class FakeFileReader { + public result: string | null = null; + public onloadend: (() => void) | null = null; + public onerror: (() => void) | null = null; + readAsDataURL(): void { + queueMicrotask(() => { + if (readerOutcome === "loadend") { + this.result = BASE64; + this.onloadend?.(); + } else { + this.onerror?.(); + } + }); + } + } + + /** Adds an SVG <image> to the editor, optionally with a source attribute. */ + function addImage(src?: string): SVGElement { + const image = document.createElementNS("http://www.w3.org/2000/svg";, "image"); + if (src !== undefined) { + image.setAttribute(XLINK_HREF, src); + } + editor.appendChild(image); + return image; + } + + /** Runs the snapshot and resolves once it settles, whichever way html2canvas goes. */ + function runSnapshot(): Promise<void> { + return new Promise<void>(resolve => { + service.generateWorkflowSnapshot("myflow").subscribe({ + next: () => resolve(), + error: () => resolve(), + }); + }); + } + + beforeEach(() => { + sentUrls = []; + xhrOutcome = "load"; + readerOutcome = "loadend"; + realXhr = globalThis.XMLHttpRequest; + realFileReader = globalThis.FileReader; + (globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = FakeXhr; + (globalThis as unknown as { FileReader: unknown }).FileReader = FakeFileReader; + editor = document.createElement("div"); + editor.id = "workflow-editor"; + document.body.appendChild(editor); + }); + + afterEach(() => { + (globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = realXhr; + (globalThis as unknown as { FileReader: unknown }).FileReader = realFileReader; + // Two of these tests spy on console.error; without this the spy would outlive them. + vi.restoreAllMocks(); + editor.remove(); + }); + + it("rewrites an image's source to the fetched base64 data", async () => { + const image = addImage("/assets/icon.png"); + + await runSnapshot(); + + expect(sentUrls).toEqual(["/assets/icon.png"]); + expect(image.getAttribute("href")).toBe(BASE64); + }); + + it("leaves an image with no source alone and fetches nothing for it", async () => { + const image = addImage(); + + await runSnapshot(); + + expect(sentUrls).toEqual([]); + expect(image.getAttribute("href")).toBeNull(); + }); + + it("reports an image whose bytes cannot be converted, and leaves its source alone", async () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + readerOutcome = "error"; + const image = addImage("/assets/icon.png"); + + await runSnapshot(); + + expect(consoleSpy).toHaveBeenCalledWith( + "Failed to load image: /assets/icon.png", + "Failed to convert image to Base64" + ); + expect(image.getAttribute("href")).toBeNull(); + }); + + it("reports an image that cannot be fetched at all", async () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + xhrOutcome = "error"; + addImage("/assets/missing.png"); + + await runSnapshot(); + + expect(consoleSpy).toHaveBeenCalledWith( + "Failed to load image: /assets/missing.png", + "Failed to load image from /assets/missing.png" + ); + }); + }); }); }); diff --git a/frontend/src/app/workspace/types/shared-editing.interface.spec.ts b/frontend/src/app/workspace/types/shared-editing.interface.spec.ts index 47e20c0931..40141d0006 100644 --- a/frontend/src/app/workspace/types/shared-editing.interface.spec.ts +++ b/frontend/src/app/workspace/types/shared-editing.interface.spec.ts @@ -200,3 +200,120 @@ describe("updateYTypeFromObject", () => { }); }); }); + +/** + * The blocks above drive the shapes production actually stores. These cover the arms that + * only a boxed String, a `typeof` the switch lists but no caller passes, or a particular + * array alignment can reach. + */ +describe("createYTypeFromObject edge kinds", () => { + it("returns values of the non-storable primitive kinds unchanged", () => { + const fn = () => 0; + const sym = Symbol("s"); + const big = BigInt(9); + + expect(createYTypeFromObject(fn as any)).toBe(fn); + expect(createYTypeFromObject(sym as any)).toBe(sym); + expect(createYTypeFromObject(big as any)).toBe(big); + }); + + it("converts a boxed String object into a Y.Text, losing its content", () => { + // `typeof` a boxed String is "object", so it reaches the constructor-name check rather + // than the "string" case above it. + const yText = createYTypeFromObject(new String("boxed") as unknown as object) as unknown as Y.Text; + + expect(yText).toBeInstanceOf(Y.Text); + // Characterizing a defect, not an intent: Y.Text's constructor only accepts a primitive, + // so the boxed value is dropped and the text comes out empty. Change this to "boxed" + // when the branch unwraps the box (`String(obj)`) before constructing the Y.Text. + expect(yText.toJSON()).toBe(""); + }); +}); + +describe("updateYTypeFromObject edge kinds", () => { + it("returns false for the non-storable primitive kinds", () => { + const doc = new Y.Doc(); + const yText = attach(doc, "t", createYTypeFromObject("hi" as unknown as object)); + + expect(updateYTypeFromObject(yText, (() => 0) as any)).toBe(false); + expect(updateYTypeFromObject(yText, Symbol("s") as any)).toBe(false); + expect(updateYTypeFromObject(yText, BigInt(9) as any)).toBe(false); + }); + + it("leaves a Y.Text untouched when the new string is identical", () => { + const doc = new Y.Doc(); + const yText = attach(doc, "t", createYTypeFromObject("same" as unknown as object)) as unknown as Y.Text; + const before = Y.encodeStateAsUpdate(doc); + + expect(updateYTypeFromObject(yText as unknown as YType<object>, "same" as any)).toBe(true); + + expect(yText.toJSON()).toBe("same"); + // An in-place delete+insert would have produced new document state. + expect(Y.encodeStateAsUpdate(doc)).toEqual(before); + }); + + it("updates a Y.Text from a boxed String", () => { + const doc = new Y.Doc(); + const yText = attach(doc, "t", createYTypeFromObject("old" as unknown as object)) as unknown as Y.Text; + + // Unlike the constructor, `insert` coerces the boxed value, so this arm does carry the + // content through. + expect(updateYTypeFromObject(yText as unknown as YType<object>, new String("new") as any)).toBe(true); + + expect(yText.toJSON()).toBe("new"); + }); + + it("stores an undefined array entry as null", () => { + const doc = new Y.Doc(); + const yArray = attach(doc, "a", createYTypeFromObject(["keep"])) as unknown as Y.Array<any>; + + expect(updateYTypeFromObject(yArray as unknown as YType<object>, ["keep", undefined] as any)).toBe(true); + + expect(yArray.toJSON()).toEqual(["keep", null]); + }); + + it("replaces an array element outright when its kind changes", () => { + const doc = new Y.Doc(); + const yArray = attach(doc, "a", createYTypeFromObject([{ a: 1 }, "tail"])) as unknown as Y.Array<any>; + + // A number cannot be updated into the Y.Map that held the object, so the element is + // deleted and re-inserted rather than mutated. + expect(updateYTypeFromObject(yArray as unknown as YType<object>, [7, "tail"] as any)).toBe(true); + + expect(yArray.toJSON()).toEqual([7, "tail"]); + }); + + it("aligns an array whose additions sit before its removals", () => { + const doc = new Y.Doc(); + const yArray = attach(doc, "a", createYTypeFromObject(["a", "b", "c"])) as unknown as Y.Array<any>; + + // The longest common subsequence is "a","c"; reaching it forces the walk through both + // of its advance arms. + expect(updateYTypeFromObject(yArray as unknown as YType<object>, ["a", "x", "y", "c"] as any)).toBe(true); + + expect(yArray.toJSON()).toEqual(["a", "x", "y", "c"]); + }); +}); + +describe("updateYTypeFromObject unsupported kinds", () => { + it("returns false for a matching type it has no merge strategy for", () => { + // Both sides report the same constructor, so the type-mismatch guard lets them through, + // but the dispatch below only knows String, Array and Object. + const oldStandIn = { toJSON: () => new Date(0) } as unknown as YType<object>; + + expect(updateYTypeFromObject(oldStandIn, new Date(1) as unknown as object)).toBe(false); + }); +}); + +describe("updateYTypeFromObject identical boxed strings", () => { + it("writes nothing when the new value is the very String object already stored", () => { + // The String branch is only reachable for a boxed String, and a boxed String never + // compares equal to the primitive a Y.Text reports — so the only way to reach the + // "already up to date" arm is for both sides to be the same object. A stand-in whose + // toJSON returns that object is what makes the comparison meet. + const boxed = new String("same"); + const oldStandIn = { toJSON: () => boxed } as unknown as YType<object>; + + expect(updateYTypeFromObject(oldStandIn, boxed as unknown as object)).toBe(true); + }); +});
