Copilot commented on code in PR #6435: URL: https://github.com/apache/texera/pull/6435#discussion_r3583866467
########## frontend/src/app/common/util/computing-unit.util.spec.ts: ########## @@ -0,0 +1,241 @@ +/** + * 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 { DashboardWorkflowComputingUnit } from "../type/workflow-computing-unit"; +import { + parseResourceUnit, + parseResourceNumber, + cpuResourceConversion, + memoryResourceConversion, + cpuPercentage, + memoryPercentage, + validateName, + getComputingUnitBadgeColor, + getComputingUnitStatusTooltip, + getComputingUnitCpuStatus, + getComputingUnitMemoryStatus, + getComputingUnitCpuLimitUnit, + isComputingUnitShmTooLarge, + getJvmMemorySliderConfig, + buildLocalComputingUnitUri, +} from "./computing-unit.util"; + +describe("parseResourceUnit", () => { + it("should extract the unit from a resource string", () => { + expect(parseResourceUnit("500Mi")).toBe("Mi"); + expect(parseResourceUnit("2Gi")).toBe("Gi"); + expect(parseResourceUnit("1000m")).toBe("m"); + }); + + it("should return an empty string for a unitless number", () => { + expect(parseResourceUnit("2")).toBe(""); + expect(parseResourceUnit("1.5")).toBe(""); + }); + + it('should return "NaN" for empty or "NaN" input', () => { + expect(parseResourceUnit("")).toBe("NaN"); + expect(parseResourceUnit("NaN")).toBe("NaN"); + }); + + it("should return an empty string for a value that does not match the pattern", () => { + expect(parseResourceUnit("abc")).toBe(""); + }); +}); + +describe("parseResourceNumber", () => { + it("should extract the numeric value from a resource string", () => { + expect(parseResourceNumber("2Gi")).toBe(2); + expect(parseResourceNumber("500Mi")).toBe(500); + }); + + it("should handle decimal values", () => { + expect(parseResourceNumber("1.5Gi")).toBe(1.5); + }); + + it("should return 0 for empty, invalid, or NaN input", () => { + expect(parseResourceNumber("")).toBe(0); + expect(parseResourceNumber("NaN")).toBe(0); + expect(parseResourceNumber("abc")).toBe(0); + }); +}); + +describe("cpuResourceConversion", () => { + it("should convert millicores to cores with 4 decimals", () => { + expect(cpuResourceConversion("1000m", "")).toBe("1.0000"); + expect(cpuResourceConversion("500m", "")).toBe("0.5000"); + }); + + it("should convert cores to millicores with 2 decimals", () => { + expect(cpuResourceConversion("2", "m")).toBe("2000.00"); + }); + + it("should round to whole numbers for smaller units", () => { + expect(cpuResourceConversion("1", "u")).toBe("1000000"); + }); +}); + +describe("memoryResourceConversion", () => { + it("should convert between memory units with 4 decimals", () => { + expect(memoryResourceConversion("1Gi", "Gi")).toBe("1.0000"); + expect(memoryResourceConversion("1024Mi", "Gi")).toBe("1.0000"); + expect(memoryResourceConversion("512Mi", "Gi")).toBe("0.5000"); + }); +}); + +describe("cpuPercentage", () => { + it('should return 0 when usage or limit is "N/A"', () => { + expect(cpuPercentage("N/A", "1000m")).toBe(0); + expect(cpuPercentage("500m", "N/A")).toBe(0); + }); + + it("should return 0 when the limit is not positive", () => { + expect(cpuPercentage("500m", "0")).toBe(0); + }); + + it("should compute the usage percentage", () => { + expect(cpuPercentage("500m", "1000m")).toBe(50); + }); + + it("should clamp the percentage at 100", () => { + expect(cpuPercentage("2000m", "1000m")).toBe(100); + }); +}); + +describe("memoryPercentage", () => { + it('should return 0 when usage or limit is "N/A"', () => { + expect(memoryPercentage("N/A", "1Gi")).toBe(0); + expect(memoryPercentage("512Mi", "N/A")).toBe(0); + }); + + it("should compute the usage percentage", () => { + expect(memoryPercentage("512Mi", "1Gi")).toBe(50); + }); + + it("should clamp the percentage at 100", () => { + expect(memoryPercentage("2Gi", "1Gi")).toBe(100); + }); +}); Review Comment: `memoryPercentage` includes a `limitValue <= 0` guard, and the PR description claims this behavior is covered, but the spec currently doesn’t test the non-positive-limit case. Adding a regression-style test will better pin the contract and align with the stated coverage. ########## frontend/src/app/common/util/computing-unit.util.spec.ts: ########## @@ -0,0 +1,241 @@ +/** + * 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 { DashboardWorkflowComputingUnit } from "../type/workflow-computing-unit"; Review Comment: This import is only used as a type (in `as unknown as DashboardWorkflowComputingUnit` casts). Use a type-only import to avoid introducing an unnecessary runtime dependency during tests (and to match the codebase’s existing pattern of `import type` in spec files). -- 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]
