Copilot commented on code in PR #6082: URL: https://github.com/apache/texera/pull/6082#discussion_r3522428554
########## frontend/src/app/common/component/computing-unit-create-modal/computing-unit-create-modal.component.spec.ts: ########## @@ -0,0 +1,215 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { NzModalService } from "ng-zorro-antd/modal"; +import { of } from "rxjs"; +import type { Mocked } from "vitest"; +import { ComputingUnitCreateModalComponent } from "./computing-unit-create-modal.component"; +import { WorkflowComputingUnitManagingService } from "../../service/computing-unit/workflow-computing-unit/workflow-computing-unit-managing.service"; +import { ComputingUnitStatusService } from "../../service/computing-unit/computing-unit-status/computing-unit-status.service"; +import { MockComputingUnitStatusService } from "../../service/computing-unit/computing-unit-status/mock-computing-unit-status.service"; +import { NotificationService } from "../../service/notification/notification.service"; +import { DashboardWorkflowComputingUnit, WorkflowComputingUnitType } from "../../type/workflow-computing-unit"; +import { commonTestProviders } from "../../testing/test-utils"; +import { getJvmMemorySliderConfig } from "../../util/computing-unit.util"; + +describe("ComputingUnitCreateModalComponent", () => { + let component: ComputingUnitCreateModalComponent; + let fixture: ComponentFixture<ComputingUnitCreateModalComponent>; + let mockComputingUnitService: Mocked<WorkflowComputingUnitManagingService>; + let mockNotificationService: Mocked<NotificationService>; + + const createdUnit = { computingUnit: { cuid: 42 } } as unknown as DashboardWorkflowComputingUnit; + + beforeEach(async () => { + mockComputingUnitService = { + getComputingUnitTypes: vi.fn(), + getComputingUnitLimitOptions: vi.fn(), + createKubernetesBasedComputingUnit: vi.fn(), + createLocalComputingUnit: vi.fn(), + } as unknown as Mocked<WorkflowComputingUnitManagingService>; + mockComputingUnitService.getComputingUnitTypes.mockReturnValue(of({ typeOptions: [] })); + mockComputingUnitService.getComputingUnitLimitOptions.mockReturnValue( + of({ cpuLimitOptions: [], memoryLimitOptions: [], gpuLimitOptions: [] }) + ); + mockComputingUnitService.createKubernetesBasedComputingUnit.mockReturnValue(of(createdUnit)); + mockComputingUnitService.createLocalComputingUnit.mockReturnValue(of(createdUnit)); + + mockNotificationService = { + success: vi.fn(), + error: vi.fn(), + info: vi.fn(), + } as unknown as Mocked<NotificationService>; + + await TestBed.configureTestingModule({ + providers: [ + NzModalService, + { provide: WorkflowComputingUnitManagingService, useValue: mockComputingUnitService }, + { provide: NotificationService, useValue: mockNotificationService }, + { provide: ComputingUnitStatusService, useClass: MockComputingUnitStatusService }, + ...commonTestProviders, + ], Review Comment: This testbed provides `NzModalService` as a class provider but does not import `NzModalModule`. Because `ComputingUnitCreateModalComponent` injects `ComputingUnitActionsService` (which depends on `NzModalService`), Angular may try to instantiate the real Zorro modal service and fail due to missing overlay dependencies. Prefer a minimal stub for `NzModalService` here (the tests only exercise `create(...)`, which doesn’t use the modal service). ########## frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts: ########## @@ -207,45 +181,17 @@ export class ComputingUnitSelectionComponent implements OnInit { ) {} ngOnInit(): void { - // Fetch available computing unit types - this.localComputingUnitUri = `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ""}/wsapi`; - this.newComputingUnitName = "My Computing Unit"; - this.computingUnitService - .getComputingUnitTypes() - .pipe(untilDestroyed(this)) - .subscribe({ - next: ({ typeOptions }) => { - this.availableComputingUnitTypes = typeOptions; - // Set default selected type if available - if (typeOptions.includes("kubernetes")) { - this.selectedComputingUnitType = "kubernetes"; - } else if (typeOptions.length > 0) { - this.selectedComputingUnitType = typeOptions[0]; - } - }, - error: (err: unknown) => - this.notificationService.error(`Failed to fetch computing unit types: ${extractErrorMessage(err)}`), - }); - + // GPU options drive the GPU row in the metrics popover. The shared + // create modal fetches these options itself and owns the user-facing + // error toast for this endpoint. this.computingUnitService .getComputingUnitLimitOptions() .pipe(untilDestroyed(this)) .subscribe({ - next: ({ cpuLimitOptions, memoryLimitOptions, gpuLimitOptions }) => { - this.cpuOptions = cpuLimitOptions; - this.memoryOptions = memoryLimitOptions; + next: ({ gpuLimitOptions }) => { this.gpuOptions = gpuLimitOptions; - - // fallback defaults - this.selectedCpu = this.cpuOptions[0] ?? "1"; - this.selectedMemory = this.memoryOptions[0] ?? "1Gi"; - this.selectedGpu = this.gpuOptions[0] ?? "0"; - - // Initialize JVM memory slider based on selected memory - this.updateJvmMemorySlider(); }, - error: (err: unknown) => - this.notificationService.error(`Failed to fetch resource options: ${extractErrorMessage(err)}`), + error: () => {}, }); Review Comment: The `getComputingUnitLimitOptions()` subscription swallows errors (`error: () => {}`), but `gpuOptions` is used by `showGpuSelection()` to decide whether to display the GPU metric row. If this request fails, the UI can incorrectly hide the GPU metric even when `getGpuLimit()` indicates GPUs are allocated. Consider restoring an error handler and providing a safe fallback so GPU metrics don’t silently disappear. -- 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]
