Copilot commented on code in PR #6773:
URL: https://github.com/apache/texera/pull/6773#discussion_r3627710760
##########
frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts:
##########
@@ -1035,4 +1035,333 @@ describe("HuggingFaceComponent (TestBed)", () => {
expect(component.loading).toBe(false);
});
});
+
+ // ── Poll-path resolution (second instance waits on an in-flight fetch) ──
+ //
+ // The poll uses setInterval created inside ngOnInit (via detectChanges),
which
+ // is a real timer here, so these use a real delay rather than
fakeAsync/tick.
+
+ describe("poll-path resolution", () => {
+ /** Wait past one 200ms poll tick using real timers. */
+ const waitForPoll = () => new Promise<void>(resolve => setTimeout(resolve,
300));
Review Comment:
These tests use real timers (setTimeout + a 300ms wall-clock delay) to wait
for the 200ms polling intervals. This makes the suite slower and can be flaky
under CI load. The same spec file already tests polling using
`fakeAsync`/`tick`/`discardPeriodicTasks` (e.g., the existing "polling" and
"cleanup" sections), so these cases should be convertible to `fakeAsync` as
well for deterministic timing.
##########
frontend/src/app/workspace/component/hugging-face/hugging-face.component.spec.ts:
##########
@@ -1035,4 +1035,333 @@ describe("HuggingFaceComponent (TestBed)", () => {
expect(component.loading).toBe(false);
});
});
+
+ // ── Poll-path resolution (second instance waits on an in-flight fetch) ──
+ //
+ // The poll uses setInterval created inside ngOnInit (via detectChanges),
which
+ // is a real timer here, so these use a real delay rather than
fakeAsync/tick.
+
+ describe("poll-path resolution", () => {
+ /** Wait past one 200ms poll tick using real timers. */
+ const waitForPoll = () => new Promise<void>(resolve => setTimeout(resolve,
300));
+
+ it("resolves both the task and model poll callbacks when the in-flight
fetch succeeds", async () => {
+ // comp1 leaves BOTH tasks and models in flight.
+ const { field: field1 } = buildFieldWithFormGroup("text-generation");
+ component.field = field1;
+ fixture.detectChanges();
+ flushIconRequests();
+
+ // comp2 finds tasks + models in flight → enters BOTH poll paths.
+ const fixture2 = TestBed.createComponent(HuggingFaceComponent);
+ const component2 = fixture2.componentInstance;
+ const { field: field2 } = buildFieldWithFormGroup("text-generation");
+ component2.field = field2;
+ fixture2.detectChanges();
+ http.match(req => req.url.startsWith("assets/")).forEach(req =>
req.flush("<svg></svg>"));
+
+ expect(component2.tasksLoading).toBe(true);
+ expect(component2.loading).toBe(true);
+
+ // comp1's fetches complete → module-level caches are populated.
+ http.expectOne(`${API}/huggingface/tasks`).flush(buildTaskResponse());
+ http.expectOne(req =>
req.url.startsWith(`${API}/huggingface/models`)).flush(buildModels(7));
+
+ // Let the 200ms poll intervals observe the now-populated caches.
+ await waitForPoll();
+ flushIconRequests();
+
+ expect(component2.tasksLoading).toBe(false);
+ expect(component2.taskOptions).toEqual(buildTaskResponse());
+ expect((component2 as any).taskPollInterval).toBeNull();
+
+ expect(component2.loading).toBe(false);
+ expect(component2.pagedModels.length).toBe(7);
+ expect((component2 as any).modelPollInterval).toBeNull();
+
+ fixture2.destroy();
+ });
+
+ it("resolves both poll callbacks to the error state when the in-flight
fetch fails", async () => {
+ const { field: field1 } = buildFieldWithFormGroup("text-generation");
+ component.field = field1;
+ fixture.detectChanges();
+ flushIconRequests();
+
+ const fixture2 = TestBed.createComponent(HuggingFaceComponent);
+ const component2 = fixture2.componentInstance;
+ const { field: field2 } = buildFieldWithFormGroup("text-generation");
+ component2.field = field2;
+ fixture2.detectChanges();
+ http.match(req => req.url.startsWith("assets/")).forEach(req =>
req.flush("<svg></svg>"));
+
+ expect(component2.tasksLoading).toBe(true);
+ expect(component2.loading).toBe(true);
+
+ // comp1's fetches fail → error caches are populated.
+ http.expectOne(`${API}/huggingface/tasks`).error(new
ProgressEvent("error"));
+ http.expectOne(req =>
req.url.startsWith(`${API}/huggingface/models`)).error(new
ProgressEvent("error"));
+
+ await waitForPoll();
+ flushIconRequests();
+
+ // Task poll falls back to the static list and surfaces the cached error.
+ expect(component2.tasksLoading).toBe(false);
+ expect(component2.taskOptions).toEqual(STATIC_TASK_OPTIONS);
+ expect(component2.tasksError).toBeTruthy();
+ expect((component2 as any).taskPollInterval).toBeNull();
+
+ // Model poll surfaces the cached error message.
+ expect(component2.loading).toBe(false);
+ expect(component2.errorMessage).toBeTruthy();
+ expect(component2.pagedModels.length).toBe(0);
+ expect((component2 as any).modelPollInterval).toBeNull();
+
+ fixture2.destroy();
+ });
+
+ it("stops polling and falls back when the in-flight fetch is invalidated
before it resolves", async () => {
+ // comp1 leaves tasks + models in flight.
+ const { field: field1 } = buildFieldWithFormGroup("text-generation");
+ component.field = field1;
+ fixture.detectChanges();
+ flushIconRequests();
+
+ // comp2 enters both poll paths.
+ const fixture2 = TestBed.createComponent(HuggingFaceComponent);
+ const component2 = fixture2.componentInstance;
+ const { field: field2 } = buildFieldWithFormGroup("text-generation");
+ component2.field = field2;
+ fixture2.detectChanges();
+ http.match(req => req.url.startsWith("assets/")).forEach(req =>
req.flush("<svg></svg>"));
+
+ expect(component2.tasksLoading).toBe(true);
+ expect(component2.loading).toBe(true);
+
+ // Invalidate while comp1's fetches are still in flight: this
unsubscribes the
+ // in-flight requests WITHOUT populating any cache, so the poll
callbacks must
+ // detect the canceled fetch and fall back rather than wait forever.
+ invalidateHuggingFaceModelCache();
+
+ await waitForPoll();
+ flushIconRequests();
+
+ // Task poll detects the canceled fetch → stops and falls back to the
static list.
+ expect(component2.tasksLoading).toBe(false);
+ expect(component2.taskOptions).toEqual(STATIC_TASK_OPTIONS);
+ expect((component2 as any).taskPollInterval).toBeNull();
+
+ // Model poll detects the canceled fetch → stops without an error.
+ expect(component2.loading).toBe(false);
+ expect((component2 as any).modelPollInterval).toBeNull();
+
+ fixture2.destroy();
+ // Drain the now-canceled comp1 requests so the shared afterEach
verify() passes.
+ http.match(() => true);
Review Comment:
`http.match(() => true)` is overly broad and can accidentally consume
unrelated pending requests, which can mask regressions. Since this is meant to
drain the canceled HuggingFace requests, match only those endpoints (e.g.,
`/huggingface/`).
--
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]
