aglinxinyuan commented on code in PR #6738:
URL: https://github.com/apache/texera/pull/6738#discussion_r3627632748


##########
frontend/src/app/common/util/computing-unit.util.spec.ts:
##########
@@ -36,6 +39,36 @@ import {
   buildLocalComputingUnitUri,
 } from "./computing-unit.util";
 
+function makeUnit(overrides: Partial<DashboardWorkflowComputingUnit> = {}): 
DashboardWorkflowComputingUnit {
+  return {
+    computingUnit: {
+      cuid: 1,
+      uid: 1,
+      name: "unit-a",
+      creationTime: 1700000000000,
+      terminateTime: undefined,
+      type: "kubernetes",
+      uri: "http://localhost/wsapi";,
+      resource: {
+        cpuLimit: "1000m",
+        memoryLimit: "2Gi",
+        gpuLimit: "1",
+        jvmMemorySize: "2G",
+        shmSize: "64Mi",
+        nodeAddresses: [],
+      },
+      ...(overrides.computingUnit ?? {}),
+    },
+    status: "Running",
+    metrics: { cpuUsage: "N/A", memoryUsage: "N/A" },
+    isOwner: true,
+    accessPrivilege: "READ",
+    ownerGoogleAvatar: "",
+    ownerName: "owner",
+    ...overrides,

Review Comment:
   Fixed — makeUnit now spreads ...overrides first and sets the merged nested 
computingUnit last, so a computingUnit override merges into the defaults 
instead of clobbering them.



##########
frontend/src/app/common/util/computing-unit.util.spec.ts:
##########
@@ -239,3 +272,127 @@ describe("buildLocalComputingUnitUri", () => {
     );
   });
 });
+
+describe("cpuResourceConversion (fallback units)", () => {
+  it("should fall back to the millicore scale for an unknown source unit", () 
=> {
+    // "k" is not a known CPU unit, so cpuScales[from] is undefined and the 
"m" scale is used
+    expect(cpuResourceConversion("5k", "m")).toBe("5.00");
+  });
+
+  it("should fall back to the core scale for an unknown target unit", () => {
+    // "x" is not a known CPU unit, so cpuScales[to] is undefined and the "" 
(cores) scale is used
+    expect(cpuResourceConversion("2", "x")).toBe("2");
+  });
+});
+
+describe("memoryResourceConversion (edge branches)", () => {
+  it("should treat a unitless source value as bytes", () => {
+    expect(memoryResourceConversion("1073741824", "Gi")).toBe("1.0000");
+  });
+
+  it("should treat a unitless target unit as bytes", () => {
+    expect(memoryResourceConversion("1Gi", "")).toBe("1073741824.0000");
+  });
+
+  it("should fall back to a byte scale for an unknown source unit", () => {
+    // "Ti" is not a known memory unit, so memoryScales[from] is undefined and 
1 (bytes) is used
+    expect(memoryResourceConversion("5Ti", "")).toBe("5.0000");
+  });
+
+  it("should fall back to a byte scale for an unknown target unit", () => {
+    // "Ti" is not a known memory unit, so memoryScales[to] is undefined and 1 
(bytes) is used
+    expect(memoryResourceConversion("1Gi", "Ti")).toBe("1073741824.0000");
+  });
+});
+
+describe("memoryPercentage (limit guard)", () => {
+  it("should return 0 when the limit is not positive", () => {
+    expect(memoryPercentage("512Mi", "0")).toBe(0);
+  });
+});
+
+describe("isComputingUnitShmTooLarge (non-Gi selected memory)", () => {
+  it("should treat a Mi selected memory as-is when comparing", () => {
+    expect(isComputingUnitShmTooLarge("2048Mi", 3, "Gi")).toBe(true);
+    expect(isComputingUnitShmTooLarge("2048Mi", 1024, "Mi")).toBe(false);
+  });
+});
+
+describe("getJvmMemorySliderConfig (memory unit handling)", () => {
+  it("should convert a Mi memory value to whole GiB", () => {
+    // 2048Mi -> 2Gi -> small path (<=3), defaultValue 2 (not 1)
+    const config = getJvmMemorySliderConfig("2048Mi");
+    expect(config.showJvmMemorySlider).toBe(false);
+    expect(config.jvmMemorySteps).toEqual([1, 2]);
+    expect(config.selectedJvmMemorySize).toBe("2G");
+    expect(config.jvmMemorySliderValue).toBe(1);
+  });
+
+  it("should floor sub-GiB Mi values up to a minimum of 1 GiB", () => {
+    // 512Mi -> floor(512/1024)=0 -> Math.max(1, 0) = 1
+    const config = getJvmMemorySliderConfig("512Mi");
+    expect(config.jvmMemorySteps).toEqual([1]);
+    expect(config.selectedJvmMemorySize).toBe("1G");
+  });
+
+  it("should build slider steps from a larger Mi value", () => {
+    // 8192Mi -> 8Gi -> slider path
+    const config = getJvmMemorySliderConfig("8192Mi");
+    expect(config.showJvmMemorySlider).toBe(true);
+    expect(config.jvmMemorySteps).toEqual([2, 4, 8]);
+    expect(config.selectedJvmMemorySize).toBe("2G");
+  });
+
+  it("should default to 1 GiB for a value with an unrecognized memory unit", 
() => {
+    // "Ki" is neither Gi nor Mi, so memoryToGb falls back to 1
+    const config = getJvmMemorySliderConfig("500Ki");
+    expect(config.jvmMemorySteps).toEqual([1]);
+    expect(config.selectedJvmMemorySize).toBe("1G");
+  });
+});
+
+describe("ComputingUnitMetadataComponent", () => {
+  let fixture: ComponentFixture<ComputingUnitMetadataComponent> | undefined;
+
+  function render(unit: DashboardWorkflowComputingUnit): 
ComputingUnitMetadataComponent {
+    TestBed.configureTestingModule({
+      declarations: [ComputingUnitMetadataComponent],
+      providers: [{ provide: NZ_MODAL_DATA, useValue: unit }],
+    });
+    fixture = TestBed.createComponent(ComputingUnitMetadataComponent);
+    fixture.detectChanges();
+    return fixture.componentInstance;
+  }
+
+  afterEach(() => {
+    fixture?.destroy();
+    fixture = undefined;
+    TestBed.resetTestingModule();
+  });
+
+  it("should expose the injected unit and a formatted creation time", () => {
+    const unit = makeUnit();
+    const instance = render(unit);
+    expect(instance.unit).toBe(unit);
+    expect(instance.createdAt).toBe(new 
Date(unit.computingUnit.creationTime).toLocaleString());
+  });
+
+  it("should render owner access and the gpu limit when present", () => {
+    const unit = makeUnit({ isOwner: true });
+    unit.computingUnit.resource.gpuLimit = "2";
+    render(unit);
+    const text = (fixture as 
ComponentFixture<ComputingUnitMetadataComponent>).nativeElement.textContent as 
string;
+    expect(text).toContain("Owner");
+    expect(text).toContain("2");
+    expect(text).not.toContain("None");
+  });
+
+  it('should render the access privilege and "None" when there is no gpu 
limit', () => {
+    const unit = makeUnit({ isOwner: false, accessPrivilege: "WRITE" });
+    unit.computingUnit.resource.gpuLimit = "";
+    render(unit);
+    const text = (fixture as 
ComponentFixture<ComputingUnitMetadataComponent>).nativeElement.textContent as 
string;
+    expect(text).toContain("WRITE");
+    expect(text).toContain("None");
+  });

Review Comment:
   Fixed — the render tests now assert the exact 'Access' and 'GPU Limit' 
table-cell text via a row/cell lookup helper, rather than broad textContent 
substring matches.



-- 
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]

Reply via email to