mengw15 commented on code in PR #7503:
URL: https://github.com/apache/texera/pull/7503#discussion_r3746769901
##########
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:
Good catch on the premise — `isLoading = true` is set immediately above the
id guard and nothing on that path clears it, so a component that gets a
`filePath` before its `did`/`dvid` spins forever. Rather than drop the
assertion I've labelled it: the test now says in a comment that it
characterizes a defect, not an intent, and says what to flip when it's fixed.
Deleting it would lose a real finding; leaving it unexplained was the actual
problem you spotted. Added to the defect list in the PR description.
##########
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:
Not applying this one. `ComponentFixture.destroy()` is idempotent —
`@angular/core/fesm2022/testing.mjs` guards the teardown with `if
(!this._isDestroyed) { this.componentRef.destroy(); this._isDestroyed = true;
}` — so the second call from `afterEach` is a no-op and there is no
double-destroy.
The `destroy()` inside the test is the behaviour under test (the name is
"…and stops once destroyed"): it asserts the poll stops after a real Angular
teardown, which is what happens in production. Calling `ngOnDestroy()` alone
would test less. The `afterEach` stays as the safety net for the tests that
don't destroy explicitly.
--
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]
