This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 646e460838 fix(test, frontend): bound the workflow-snapshot render's
cost under jsdom (#7999)
646e460838 is described below
commit 646e460838c568dc421042ac8bcd95aa534aa118
Author: Neil Ketteringham <[email protected]>
AuthorDate: Thu Aug 27 20:04:39 2026 +0000
fix(test, frontend): bound the workflow-snapshot render's cost under jsdom
(#7999)
### What changes were proposed in this PR?
`report-generation.service.spec.ts` drives the real html2canvas, which
clones the whole document
rather than the element it is pointed at. The unit-test builder runs
spec files with
`isolate: false`, so one jsdom document is shared by every spec file in
a worker and the clone
drags in whatever DOM the files before it left behind.
On the macOS runner that clone was measured at 12–37s against a 20s test
timeout, failing
`fails when the editor cannot be rendered` while ubuntu and windows
passed on the same commit.
Two changes, both test-only:
- **`report-generation.service.spec.ts`** — the snapshot suite parks the
document's existing body
nodes for the duration of the file and restores them after, so the
render's cost depends only on
the DOM these tests build. It spans the file rather than each test
because the renders outlive
the tests that start them (visible in the failing log: clone #1 finished
during test #3).
- **`jsdom-svg-polyfill.ts`** — stops jsdom announcing
`getComputedStyle(elt, pseudoElt)` and
`scrollTo` on the virtual console, which vitest forwards to
`console.error` as a full stack trace
per call. The failing job carried 5,691 of them for six renders.
Dropping `pseudoElt` changes no
behaviour: jsdom complains and then ignores the argument, returning the
element's own declaration
either way. `scrollTo` has nothing to move — jsdom has no layout, and
the one html2canvas reaches
for belongs to the throwaway clone iframe, so it is neutered as each
`contentWindow` is handed out.
### Any related issues, documentation, discussions?
Closes #8001.
This continues #7886 (closing #7887), which stopped the four
image-inlining tests in this file
waiting on the render. The flake moved to the two tests that still await
one.
Scope note for reviewers: this bounds the render's cost, it does not
remove the dependency on a
real render. The two remaining tests still await html2canvas, so a
sufficiently loaded runner
could in principle still be slow — just not by the two orders of
magnitude measured here. Taking
#7886's logic to its conclusion (no test in this file waiting on a real
render) would need a
stub html2canvas that survives `isolate: false`, which is a larger
change to what those two tests
cover; happy to do that instead if reviewers prefer it.
Unrelated pre-existing noise left alone: a full run still emits ~827
jsdom
`HTMLCanvasElement.prototype.getContext` traces from
`JointUIService.getMeasureContext`
(`joint-ui.service.ts:219`). Silencing it would change what
`measureText` returns for the
joint-ui specs, so it is out of scope here.
### How was this PR tested?
`ng test --watch=false` on this branch (base `444fc58284`), four full
runs: 201 files,
5131 passed / 1 skipped each time, no `Not implemented:
window.getComputedStyle` traces.
Clone time across the file's six renders was 79–353 ms on three warm
runs; the test that was
timing out renders in 85–114 ms. The first run on a cold checkout was
slower — up to 3.9s for a
clone — which is a reminder that machine load, not document size, is
what is left.
The effect of the console noise was measured directly by seeding the
document with 500 nodes:
clone time across the six renders went 18.8s → 13.1s with the polyfill
change alone (~30%; the
share is larger where console forwarding is slower, as on CI).
`yarn --cwd frontend format:ci` is clean.
I cannot demonstrate the macOS leg green from here — that needs CI on
this branch.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5 [1M context])
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../report-generation.service.spec.ts | 32 +++++++++++++
frontend/src/jsdom-svg-polyfill.ts | 55 ++++++++++++++++++++++
2 files changed, 87 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 90926b7431..abcfbb24ba 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
@@ -330,6 +330,34 @@ describe("ReportGenerationService", () => {
});
describe("generateWorkflowSnapshot", () => {
+ /**
+ * html2canvas clones the whole document — from `documentElement`, not
from the element it is
+ * pointed at — and the unit-test builder runs spec files with `isolate:
false`, so one jsdom
+ * document is shared by every spec file in a worker and the renders below
drag in whatever DOM
+ * the files before this one left behind, in `<head>` as much as in the
body. That is what
+ * failed the macOS leg: a clone that costs ~60ms against this suite's own
DOM was measured at
+ * 12–37s there, past the 20s test timeout, while ubuntu and windows
passed. Park the foreign
+ * nodes of both for the duration of the suite and put them back after, so
the render's cost
+ * depends only on what these tests build. The renders started here
outlive the tests that
+ * start them, so this has to span the suite rather than each test.
+ */
+ let parkedNodes: [ParentNode, ChildNode][];
+
+ beforeAll(() => {
+ parkedNodes = [document.head, document.body].flatMap(parent =>
+ Array.from(parent.childNodes).map((node): [ParentNode, ChildNode] =>
[parent, node])
+ );
+ parkedNodes.forEach(([, node]) => node.remove());
+ });
+
+ afterAll(() => {
+ // html2canvas only detaches the iframe it clones into on the render's
success path, so each
+ // render these tests leave failing strands one in the body. Drop them
before the parked
+ // nodes go back, otherwise the next spec file's renders clone them.
+
document.body.querySelectorAll("iframe.html2canvas-container").forEach(node =>
node.remove());
+ parkedNodes.forEach(([parent, node]) => parent.appendChild(node));
+ });
+
it("fails when the editor is not on the page", async () => {
await
expect(firstValueFrom(service.generateWorkflowSnapshot("myflow"))).rejects.toBe(
"Workflow editor element not found"
@@ -431,6 +459,10 @@ describe("ReportGenerationService", () => {
editor = document.createElement("div");
editor.id = "workflow-editor";
document.body.appendChild(editor);
+ // The render these tests start is left to fail on its own, but jsdom
announces its
+ // missing 2D context on the virtual console, so an otherwise clean
run carries a stack
+ // trace per test. Hand back what jsdom hands back after complaining,
minus the complaint.
+ vi.spyOn(HTMLCanvasElement.prototype,
"getContext").mockReturnValue(null as never);
});
afterEach(() => {
diff --git a/frontend/src/jsdom-svg-polyfill.ts
b/frontend/src/jsdom-svg-polyfill.ts
index 1fe62e2147..7aef063d7d 100644
--- a/frontend/src/jsdom-svg-polyfill.ts
+++ b/frontend/src/jsdom-svg-polyfill.ts
@@ -192,6 +192,61 @@ G.ResizeObserver ??= class {
disconnect(): void {}
};
+// `window.getComputedStyle(elt, pseudoElt)` and `window.scrollTo` — jsdom
+// implements neither. Both route through its `notImplemented` helper, which
+// emits a `jsdomError` on the virtual console; vitest's jsdom environment
+// forwards that to `console.error`, one full stack trace per call. html2canvas
+// calls both on every render — a `:before` and an `:after` lookup for each
+// cloned node, plus one `scrollTo` per document clone — so the
+// report-generation spec, which drives the real renderer on purpose (`vi.mock`
+// can't reach it under the builder's `isolate: false`), buries the run in
+// traces while all of its tests pass.
+// Dropping `pseudoElt` changes no behaviour for the arguments jsdom only
+// complains about: it ignores them and returns the element's own declaration
+// either way. The exception is a shadow-DOM pseudo-element (`::part(…)` /
+// `::slotted(…)`), which jsdom rejects with a `TypeError` before it reaches
+// `notImplemented` — those are forwarded so the rejection still happens.
+// `scrollTo` has nothing to move — jsdom has no layout.
+// Both patches wrap a global that this setup file is re-evaluated against once
+// per spec file, so each marks what it installed and does nothing when it
finds
+// its own mark — otherwise the wrappers nest one layer deeper per spec file.
+// The mark rides on the installed function rather than on a `globalThis` flag
+// so that a jsdom window replaced mid-run still gets patched.
+const NOISE_PATCH_MARK = Symbol.for("texera.jsdomNotImplementedNoisePatched");
+const alreadyPatched = (fn: unknown): boolean => typeof fn === "function" &&
NOISE_PATCH_MARK in (fn as object);
+const markPatched = (fn: AnyFn): AnyFn => Object.assign(fn, {
[NOISE_PATCH_MARK]: true });
+
+const SHADOW_DOM_PSEUDO = /^::(?:part|slotted)\(/i;
+const jsdomGetComputedStyle = G.getComputedStyle as ((elt: Element,
pseudoElt?: string | null) => unknown) | undefined;
+if (typeof jsdomGetComputedStyle === "function" &&
!alreadyPatched(jsdomGetComputedStyle)) {
+ const withoutPseudoElement = markPatched(((elt: Element, pseudoElt?: string
| null) =>
+ pseudoElt !== undefined && pseudoElt !== null &&
SHADOW_DOM_PSEUDO.test(String(pseudoElt))
+ ? jsdomGetComputedStyle(elt, pseudoElt)
+ : jsdomGetComputedStyle(elt)) as AnyFn);
+ G.getComputedStyle = withoutPseudoElement;
+ if (G.window) G.window.getComputedStyle = withoutPseudoElement;
+}
+// The only `scrollTo` html2canvas aims at this window is guarded by a
+// scroll-offset check that cannot fire under jsdom — there is no layout, so
+// both offsets are 0 and the call is skipped. The one it does make belongs to
+// the throwaway iframe it clones the page into, and jsdom installs the method
+// on each window instance rather than on a shared prototype, so it has to be
+// neutered as each `contentWindow` is handed out.
+const inertScrollTo: AnyFn = () => undefined;
+const iframeProto = G.HTMLIFrameElement?.prototype;
+const contentWindow = iframeProto &&
Object.getOwnPropertyDescriptor(iframeProto, "contentWindow");
+if (contentWindow?.get && !alreadyPatched(contentWindow.get)) {
+ const getContentWindow = contentWindow.get;
+ Object.defineProperty(iframeProto, "contentWindow", {
+ ...contentWindow,
+ get: markPatched(function (this: unknown): unknown {
+ const frameWindow = getContentWindow.call(this) as Record<string,
unknown> | null;
+ if (frameWindow) frameWindow.scrollTo = inertScrollTo;
+ return frameWindow;
+ } as AnyFn),
+ });
+}
+
// `WebSocket` — y-websocket schedules a reconnect timer the moment a
// collaborative-editing service is constructed. When that timer fires AFTER
// vitest has begun tearing down the jsdom window, jsdom's WebSocket
