aglinxinyuan commented on code in PR #6773:
URL: https://github.com/apache/texera/pull/6773#discussion_r3627951856
##########
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:
Addressed the flakiness concern: these polls use setInterval created during
detectChanges (a real timer not captured by fakeAsync), so rather than convert
to fakeAsync I replaced the fixed 300ms sleep with a condition-poll helper that
waits until both poll intervals have cleared (up to a 3s cap) — deterministic
regardless of CI load.
##########
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:
Fixed — narrowed the cleanup drain to http.match(req =>
req.url.includes("/huggingface/")) so it only consumes the canceled HuggingFace
requests, not unrelated pending ones.
--
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]
