Copilot commented on code in PR #7503:
URL: https://github.com/apache/texera/pull/7503#discussion_r3746718590
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts:
##########
@@ -414,6 +416,181 @@ describe("UserDatasetFileRendererComponent", () => {
loadWith("notes.txt", blob);
expect(component.isFileTypePreviewUnsupported).toBe(true);
});
+
+ it("does not fetch when the dataset ids are missing", () => {
+ const datasetService = TestBed.inject(DatasetService);
+ const spy = vi.spyOn(datasetService, "retrieveDatasetVersionSingleFile");
+ // A supported, in-limit file, so the two pre-checks pass and the id
guard is the
+ // only thing left to stop the request.
+ component.did = undefined;
+ component.dvid = 2;
+ component.filePath = "notes.txt";
+ component.fileSize = 100;
+
+ component.reloadFileContent();
+
+ expect(spy).not.toHaveBeenCalled();
+ expect(component.isLoading).toBe(true);
Review Comment:
This test asserts `component.isLoading` stays `true` when `did` is missing.
In the component implementation, `isLoading` is set to `true` before the
`did/dvid/filePath` guard, so this assertion bakes in what looks like an
unintended “stuck loading” state. To keep the test focused on the guard
behavior (no fetch) and avoid encoding a likely bug/implementation detail, drop
the `isLoading` assertion here.
##########
frontend/src/app/dashboard/component/user/user-computing-unit/user-computing-unit.component.spec.ts:
##########
@@ -94,4 +96,109 @@ describe("UserComputingUnitComponent", () => {
modal.visibleChange.emit(false);
expect(component.addComputeUnitModalVisible).toBe(false);
});
+
+ describe("session, polling and termination", () => {
+ function makeUnit(cuid: number): DashboardWorkflowComputingUnit {
+ return {
+ computingUnit: {
+ cuid,
+ uid: 1,
+ name: `unit-${cuid}`,
+ creationTime: 0,
+ terminateTime: undefined,
+ type: "kubernetes",
+ uri: `uri-${cuid}`,
+ resource: {
+ cpuLimit: "1",
+ memoryLimit: "1Gi",
+ gpuLimit: "0",
+ jvmMemorySize: "1Gi",
+ shmSize: "64Mi",
+ nodeAddresses: [],
+ },
+ },
+ status: "Running",
+ metrics: { cpuUsage: "N/A", memoryUsage: "N/A" },
+ isOwner: true,
+ accessPrivilege: "WRITE",
+ ownerGoogleAvatar: "",
+ ownerName: "owner",
+ } as DashboardWorkflowComputingUnit;
+ }
+
+ afterEach(() => {
+ // ngOnInit starts a 1s interval; destroying the fixture unsubscribes it
so it
+ // cannot tick into a later test.
+ fixture.destroy();
+ });
+
+ it("follows the signed-in user when the session changes", () => {
+ fixture.detectChanges();
+ const stubUserService = TestBed.inject(UserService) as unknown as
StubUserService;
+
+ stubUserService.user = undefined;
+ stubUserService.userChangeSubject.next(undefined);
+
+ expect(component.isLogin).toBe(false);
+ expect(component.currentUid).toBeUndefined();
+ });
+
+ it("maps the fetched computing units into dashboard entries", () => {
+ const statusService = TestBed.inject(ComputingUnitStatusService);
+ vi.spyOn(statusService,
"getAllComputingUnits").mockReturnValue(of([makeUnit(7)]));
+
+ fixture.detectChanges();
+
+ expect(component.allComputingUnits.map(u =>
u.computingUnit.cuid)).toEqual([7]);
+ expect(component.entries.map(e => e.id)).toEqual([7]);
+ });
+
+ it("refreshes the list on every poll tick, and stops once destroyed",
fakeAsync(() => {
+ const statusService = TestBed.inject(ComputingUnitStatusService);
+ const refreshSpy = vi.spyOn(statusService,
"refreshComputingUnitList").mockImplementation(() => {});
+
+ // ngOnInit directly rather than through detectChanges: the fixture's
NgZone was
+ // created outside this fakeAsync zone, so a poll scheduled from there
would land
+ // on the real timer queue and tick() could not drive it.
+ component.ngOnInit();
+ expect(refreshSpy).not.toHaveBeenCalled();
+
+ tick(1000);
+ expect(refreshSpy).toHaveBeenCalledTimes(1);
+ tick(1000);
+ expect(refreshSpy).toHaveBeenCalledTimes(2);
+
+ fixture.destroy();
Review Comment:
`fixture.destroy()` is called inside this test and again in the nested
`afterEach`, which can lead to double-destroy behavior (and makes the test
harder to reason about). Since the component uses
`@UntilDestroy()`/`untilDestroyed(this)` for the poll subscription, you can
stop the interval by invoking the component’s `ngOnDestroy()` hook here and
leave `fixture.destroy()` to the `afterEach` cleanup.
--
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]
