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/23 07:51:00 UTC

(texera) branch gh-readonly-queue/main/pr-7886-a33ece9ab11bf47f4bab0fcb1cf3f4aee5ea2989 created (now 37ccb73151)

This is an automated email from the ASF dual-hosted git repository.

github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7886-a33ece9ab11bf47f4bab0fcb1cf3f4aee5ea2989
in repository https://gitbox.apache.org/repos/asf/texera.git


      at 37ccb73151 fix(test, frontend): stop the snapshot image tests waiting on html2canvas (#7886)

This branch includes the following new commits:

     new 37ccb73151 fix(test, frontend): stop the snapshot image tests waiting on html2canvas (#7886)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(texera) 01/01: fix(test, frontend): stop the snapshot image tests waiting on html2canvas (#7886)

Posted by gi...@apache.org.
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-7886-a33ece9ab11bf47f4bab0fcb1cf3f4aee5ea2989
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 37ccb73151361a16c39d763b6617d1bd4da563b8
Author: Meng Wang <me...@uci.edu>
AuthorDate: Sun Aug 23 07:50:50 2026 +0000

    fix(test, frontend): stop the snapshot image tests waiting on html2canvas (#7886)
    
    ### What changes were proposed in this PR?
    
    `report-generation.service.spec.ts`'s four image tests waited on
    `generateWorkflowSnapshot`'s
    observable, which only settles once html2canvas finishes. Under jsdom
    that render takes an
    unbounded amount of time — on the macOS runner long enough that "reports
    an image that cannot
    be fetched at all" hit the 20s test timeout and turned the job red,
    while ubuntu and windows
    passed. It has been failing intermittently on macOS since #7541.
    
    The tests are about the inlining step that runs *before* the render, so
    they no longer wait for
    the render at all: `startSnapshot()` subscribes and returns, and each
    test waits for its own
    effect — the rewritten `href`, or the `console.error` the failure path
    logs. The "image with no
    source" case needs no wait, since the fake XHR issues its request
    synchronously. `afterEach`
    unsubscribes so nothing an in-flight render emits reaches a test that
    has already finished.
    
    Same assertions, same coverage: `report-generation.service.ts` stays at
    129/129 statements and
    25/25 branches. The spec file's tests went from 20.6s (timing out) to
    348ms.
    
    ### Any related issues, documentation, discussions?
    
    Closes #7887.
    
    The flake was introduced by #7541.
    
    ### How was this PR tested?
    
    `ng test --watch=false --include
    src/app/workspace/service/report-generation/report-generation.service.spec.ts`
    — 26 passed, run 3x. Coverage (`--coverage`) confirms the file is
    unchanged at 129/129
    statements. The failure path was verified by breaking the base64
    assertion (red, non-zero exit)
    and restoring it. `yarn --cwd frontend format:ci` is 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              | 58 ++++++++++++++--------
 1 file changed, 38 insertions(+), 20 deletions(-)

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 15c7155b5a..90926b7431 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
@@ -19,7 +19,7 @@
 
 import { TestBed } from "@angular/core/testing";
 import { HttpClient } from "@angular/common/http";
-import { firstValueFrom, of, Subject, throwError } from "rxjs";
+import { firstValueFrom, of, Subject, Subscription, throwError } from "rxjs";
 import { ReportGenerationService } from "./report-generation.service";
 import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service";
 import { WorkflowResultService } from "../workflow-result/workflow-result.service";
@@ -352,6 +352,7 @@ describe("ReportGenerationService", () => {
       let realXhr: typeof globalThis.XMLHttpRequest;
       let realFileReader: typeof globalThis.FileReader;
       let editor: HTMLElement;
+      let started: Subscription[];
       let xhrOutcome: "load" | "error";
       let readerOutcome: "loadend" | "error";
       let sentUrls: string[];
@@ -401,17 +402,25 @@ describe("ReportGenerationService", () => {
         return image;
       }
 
-      /** Runs the snapshot and resolves once it settles, whichever way html2canvas goes. */
-      function runSnapshot(): Promise<void> {
-        return new Promise<void>(resolve => {
+      /**
+       * Starts the snapshot and returns immediately. Waiting for the observable to settle
+       * would mean waiting for html2canvas, which under jsdom takes an unbounded amount of
+       * time — on a slow runner long enough to blow the test timeout. These tests are about
+       * the inlining step that runs first, so each waits for that step's own effect instead.
+       * The subscriber swallows both outcomes so the render's failure is not an unhandled
+       * error.
+       */
+      function startSnapshot(): void {
+        started.push(
           service.generateWorkflowSnapshot("myflow").subscribe({
-            next: () => resolve(),
-            error: () => resolve(),
-          });
-        });
+            next: () => {},
+            error: () => {},
+          })
+        );
       }
 
       beforeEach(() => {
+        started = [];
         sentUrls = [];
         xhrOutcome = "load";
         readerOutcome = "loadend";
@@ -427,6 +436,9 @@ describe("ReportGenerationService", () => {
       afterEach(() => {
         (globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = realXhr;
         (globalThis as unknown as { FileReader: unknown }).FileReader = realFileReader;
+        // The render this started cannot be cancelled, but nothing it emits should reach a
+        // test that has already finished.
+        started.forEach(subscription => subscription.unsubscribe());
         // Two of these tests spy on console.error; without this the spy would outlive them.
         vi.restoreAllMocks();
         editor.remove();
@@ -435,16 +447,18 @@ describe("ReportGenerationService", () => {
       it("rewrites an image's source to the fetched base64 data", async () => {
         const image = addImage("/assets/icon.png");
 
-        await runSnapshot();
+        startSnapshot();
 
+        await vi.waitFor(() => expect(image.getAttribute("href")).toBe(BASE64));
         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 () => {
+      it("leaves an image with no source alone and fetches nothing for it", () => {
         const image = addImage();
 
-        await runSnapshot();
+        // The request, if there were one, is issued synchronously by the fake XHR, so an
+        // empty list right after starting is already the answer.
+        startSnapshot();
 
         expect(sentUrls).toEqual([]);
         expect(image.getAttribute("href")).toBeNull();
@@ -455,11 +469,13 @@ describe("ReportGenerationService", () => {
         readerOutcome = "error";
         const image = addImage("/assets/icon.png");
 
-        await runSnapshot();
+        startSnapshot();
 
-        expect(consoleSpy).toHaveBeenCalledWith(
-          "Failed to load image: /assets/icon.png",
-          "Failed to convert image to Base64"
+        await vi.waitFor(() =>
+          expect(consoleSpy).toHaveBeenCalledWith(
+            "Failed to load image: /assets/icon.png",
+            "Failed to convert image to Base64"
+          )
         );
         expect(image.getAttribute("href")).toBeNull();
       });
@@ -469,11 +485,13 @@ describe("ReportGenerationService", () => {
         xhrOutcome = "error";
         addImage("/assets/missing.png");
 
-        await runSnapshot();
+        startSnapshot();
 
-        expect(consoleSpy).toHaveBeenCalledWith(
-          "Failed to load image: /assets/missing.png",
-          "Failed to load image from /assets/missing.png"
+        await vi.waitFor(() =>
+          expect(consoleSpy).toHaveBeenCalledWith(
+            "Failed to load image: /assets/missing.png",
+            "Failed to load image from /assets/missing.png"
+          )
         );
       });
     });