Copilot commented on code in PR #7334:
URL: https://github.com/apache/texera/pull/7334#discussion_r3720165449
##########
frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts:
##########
@@ -377,4 +387,116 @@ describe("UserVenvComponent", () => {
expect(component.trackByVeid(1, { name: "", newPackages: []
})).toBeUndefined();
});
});
+
+ // The class is well covered above; these exercise the template itself — the
list
+ // branches and the nz-modal body/footer, which render into the CDK overlay.
+ describe("template rendering", () => {
+ type Draft = NonNullable<UserVenvComponent["currentDraft"]>;
+
+ // nz-modal renders into the overlay attached to ApplicationRef, so tick()
after
+ // detectChanges to flush its embedded view.
+ const flushOverlay = (): void => {
+ fixture.detectChanges();
+ TestBed.inject(ApplicationRef).tick();
+ };
+ const overlay = (): HTMLElement =>
document.querySelector(".cdk-overlay-container") as HTMLElement;
Review Comment:
This helper unsafely casts the result of `document.querySelector(...)` to
`HTMLElement`. If the overlay container isn’t present (or is removed between
tests), this becomes `null` at runtime and will fail with a cryptic error
later. Prefer using Angular CDK’s `OverlayContainer` test utility (or at
minimum assert non-null before casting) and consider cleaning the overlay
container in `afterEach` to avoid cross-test contamination.
##########
frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts:
##########
@@ -377,4 +387,116 @@ describe("UserVenvComponent", () => {
expect(component.trackByVeid(1, { name: "", newPackages: []
})).toBeUndefined();
});
});
+
+ // The class is well covered above; these exercise the template itself — the
list
+ // branches and the nz-modal body/footer, which render into the CDK overlay.
+ describe("template rendering", () => {
+ type Draft = NonNullable<UserVenvComponent["currentDraft"]>;
+
+ // nz-modal renders into the overlay attached to ApplicationRef, so tick()
after
+ // detectChanges to flush its embedded view.
+ const flushOverlay = (): void => {
+ fixture.detectChanges();
+ TestBed.inject(ApplicationRef).tick();
+ };
+ const overlay = (): HTMLElement =>
document.querySelector(".cdk-overlay-container") as HTMLElement;
+
+ const openModalWith = (draft: Draft): HTMLElement => {
+ component.currentDraft = draft;
+ component.pveModalVisible = true;
+ flushOverlay();
+ return overlay();
+ };
+
+ const seedList = (records: UserPveRecord[]): void => {
+ pveServiceSpy.listUserPves.mockReturnValue(of(records));
+ fixture.detectChanges();
+ };
+
+ it("shows the empty-state message and no list when there are no
environments", () => {
+ seedList([]);
+ const host = fixture.nativeElement as HTMLElement;
+
expect(host.querySelector(".python-env-page-empty")?.textContent).toContain("No
environments yet");
+ expect(host.querySelector("ul.python-env-page-list")).toBeNull();
+ });
+
+ it("opens an empty draft modal from the Create button", () => {
+ fixture.detectChanges();
+
fixture.debugElement.query(By.css(".create-btn")).triggerEventHandler("click",
{});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft).toEqual({ name: "", newPackages: [] });
+ });
+
+ it("renders a row per environment (with the unnamed fallback) and opens
the row on click", () => {
+ seedList([
+ { veid: 1, name: "envA", packages: {} },
+ { veid: 2, name: "", packages: {} },
+ ] as UserPveRecord[]);
+
+ const rows =
fixture.debugElement.queryAll(By.css("li.python-env-page-item"));
+ expect(rows.length).toBe(2);
+ expect((fixture.nativeElement as
HTMLElement).textContent).toContain("(unnamed)");
+
+ rows[0].triggerEventHandler("click", {});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft?.name).toBe("envA");
+ });
+
+ it("fires confirmDeletePve from the row delete icon and stops row-open
propagation", () => {
+ seedList([{ veid: 3, name: "envDel", packages: {} }] as UserPveRecord[]);
+ const stopPropagation = vi.fn();
+
fixture.debugElement.query(By.css(".python-env-delete-icon")).triggerEventHandler("click",
{ stopPropagation });
+ expect(stopPropagation).toHaveBeenCalled();
+ expect(confirmSpy).toHaveBeenCalledTimes(1);
+ expect(component.pveModalVisible).toBe(false);
+ });
+
+ it("renders the modal form, package header, one row per package, and the
footer when open", () => {
+ fixture.detectChanges();
+ const o = openModalWith({
+ name: "envForm",
+ newPackages: [
+ { name: "numpy", versionOp: "==", version: "1.2" },
+ { name: "pandas", versionOp: ">=", version: "2.0" },
+ ],
+ });
+
+ expect(o.querySelector(".ve-form")).not.toBeNull();
+ // header row (*ngIf newPackages.length > 0) + one row per package
+ expect(o.querySelectorAll(".package-row").length).toBe(3);
+ expect(o.querySelector(".add-btn button")).not.toBeNull();
+ expect(o.querySelectorAll(".footer-all button").length).toBe(2);
+ });
+
+ it("drives the modal package controls and the Save footer button through
the DOM", () => {
+ fixture.detectChanges();
+ const o = openModalWith({ name: "envDrive", newPackages: [{ name: "x",
versionOp: "==", version: "1" }] });
+
+ (o.querySelector(".add-btn button") as HTMLButtonElement).click();
+ flushOverlay();
+ expect(component.currentDraft?.newPackages.length).toBe(2);
+
+ (o.querySelector(".package-row .user-package-inputs button") as
HTMLButtonElement).click();
+ expect(component.currentDraft?.newPackages[0].deleteToggle).toBe(true);
+
+ (o.querySelectorAll(".footer-all button")[1] as
HTMLButtonElement).click();
+ expect(pveServiceSpy.savePve).toHaveBeenCalledWith("envDrive", {});
+ });
+
+ it("closes the modal from the footer Close button", () => {
+ fixture.detectChanges();
+ const o = openModalWith({ name: "envClose", newPackages: [] });
+ (o.querySelectorAll(".footer-all button")[0] as
HTMLButtonElement).click();
Review Comment:
Index-based selection of footer buttons (`[1]` for Save, `[0]` for Close) is
brittle and can break if button order changes (or if another button is added).
Use a more stable selector (e.g., `data-testid`, an `aria-label`, or matching
by button text/content) so the tests fail only when behavior changes, not when
markup is rearranged.
##########
frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts:
##########
@@ -377,4 +387,116 @@ describe("UserVenvComponent", () => {
expect(component.trackByVeid(1, { name: "", newPackages: []
})).toBeUndefined();
});
});
+
+ // The class is well covered above; these exercise the template itself — the
list
+ // branches and the nz-modal body/footer, which render into the CDK overlay.
+ describe("template rendering", () => {
+ type Draft = NonNullable<UserVenvComponent["currentDraft"]>;
+
+ // nz-modal renders into the overlay attached to ApplicationRef, so tick()
after
+ // detectChanges to flush its embedded view.
+ const flushOverlay = (): void => {
+ fixture.detectChanges();
+ TestBed.inject(ApplicationRef).tick();
+ };
+ const overlay = (): HTMLElement =>
document.querySelector(".cdk-overlay-container") as HTMLElement;
+
+ const openModalWith = (draft: Draft): HTMLElement => {
+ component.currentDraft = draft;
+ component.pveModalVisible = true;
+ flushOverlay();
+ return overlay();
+ };
+
+ const seedList = (records: UserPveRecord[]): void => {
+ pveServiceSpy.listUserPves.mockReturnValue(of(records));
+ fixture.detectChanges();
+ };
+
+ it("shows the empty-state message and no list when there are no
environments", () => {
+ seedList([]);
+ const host = fixture.nativeElement as HTMLElement;
+
expect(host.querySelector(".python-env-page-empty")?.textContent).toContain("No
environments yet");
+ expect(host.querySelector("ul.python-env-page-list")).toBeNull();
+ });
+
+ it("opens an empty draft modal from the Create button", () => {
+ fixture.detectChanges();
+
fixture.debugElement.query(By.css(".create-btn")).triggerEventHandler("click",
{});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft).toEqual({ name: "", newPackages: [] });
+ });
+
+ it("renders a row per environment (with the unnamed fallback) and opens
the row on click", () => {
+ seedList([
+ { veid: 1, name: "envA", packages: {} },
+ { veid: 2, name: "", packages: {} },
+ ] as UserPveRecord[]);
+
+ const rows =
fixture.debugElement.queryAll(By.css("li.python-env-page-item"));
+ expect(rows.length).toBe(2);
+ expect((fixture.nativeElement as
HTMLElement).textContent).toContain("(unnamed)");
+
+ rows[0].triggerEventHandler("click", {});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft?.name).toBe("envA");
+ });
+
+ it("fires confirmDeletePve from the row delete icon and stops row-open
propagation", () => {
+ seedList([{ veid: 3, name: "envDel", packages: {} }] as UserPveRecord[]);
+ const stopPropagation = vi.fn();
+
fixture.debugElement.query(By.css(".python-env-delete-icon")).triggerEventHandler("click",
{ stopPropagation });
+ expect(stopPropagation).toHaveBeenCalled();
+ expect(confirmSpy).toHaveBeenCalledTimes(1);
+ expect(component.pveModalVisible).toBe(false);
+ });
+
+ it("renders the modal form, package header, one row per package, and the
footer when open", () => {
+ fixture.detectChanges();
+ const o = openModalWith({
+ name: "envForm",
+ newPackages: [
+ { name: "numpy", versionOp: "==", version: "1.2" },
+ { name: "pandas", versionOp: ">=", version: "2.0" },
+ ],
+ });
+
+ expect(o.querySelector(".ve-form")).not.toBeNull();
+ // header row (*ngIf newPackages.length > 0) + one row per package
+ expect(o.querySelectorAll(".package-row").length).toBe(3);
+ expect(o.querySelector(".add-btn button")).not.toBeNull();
+ expect(o.querySelectorAll(".footer-all button").length).toBe(2);
+ });
+
+ it("drives the modal package controls and the Save footer button through
the DOM", () => {
+ fixture.detectChanges();
+ const o = openModalWith({ name: "envDrive", newPackages: [{ name: "x",
versionOp: "==", version: "1" }] });
+
+ (o.querySelector(".add-btn button") as HTMLButtonElement).click();
+ flushOverlay();
+ expect(component.currentDraft?.newPackages.length).toBe(2);
+
+ (o.querySelector(".package-row .user-package-inputs button") as
HTMLButtonElement).click();
+ expect(component.currentDraft?.newPackages[0].deleteToggle).toBe(true);
+
+ (o.querySelectorAll(".footer-all button")[1] as
HTMLButtonElement).click();
Review Comment:
Index-based selection of footer buttons (`[1]` for Save, `[0]` for Close) is
brittle and can break if button order changes (or if another button is added).
Use a more stable selector (e.g., `data-testid`, an `aria-label`, or matching
by button text/content) so the tests fail only when behavior changes, not when
markup is rearranged.
##########
frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts:
##########
@@ -377,4 +387,116 @@ describe("UserVenvComponent", () => {
expect(component.trackByVeid(1, { name: "", newPackages: []
})).toBeUndefined();
});
});
+
+ // The class is well covered above; these exercise the template itself — the
list
+ // branches and the nz-modal body/footer, which render into the CDK overlay.
+ describe("template rendering", () => {
+ type Draft = NonNullable<UserVenvComponent["currentDraft"]>;
+
+ // nz-modal renders into the overlay attached to ApplicationRef, so tick()
after
+ // detectChanges to flush its embedded view.
+ const flushOverlay = (): void => {
+ fixture.detectChanges();
+ TestBed.inject(ApplicationRef).tick();
+ };
+ const overlay = (): HTMLElement =>
document.querySelector(".cdk-overlay-container") as HTMLElement;
+
+ const openModalWith = (draft: Draft): HTMLElement => {
+ component.currentDraft = draft;
+ component.pveModalVisible = true;
+ flushOverlay();
+ return overlay();
+ };
+
+ const seedList = (records: UserPveRecord[]): void => {
+ pveServiceSpy.listUserPves.mockReturnValue(of(records));
+ fixture.detectChanges();
+ };
+
+ it("shows the empty-state message and no list when there are no
environments", () => {
+ seedList([]);
+ const host = fixture.nativeElement as HTMLElement;
+
expect(host.querySelector(".python-env-page-empty")?.textContent).toContain("No
environments yet");
+ expect(host.querySelector("ul.python-env-page-list")).toBeNull();
+ });
+
+ it("opens an empty draft modal from the Create button", () => {
+ fixture.detectChanges();
+
fixture.debugElement.query(By.css(".create-btn")).triggerEventHandler("click",
{});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft).toEqual({ name: "", newPackages: [] });
+ });
+
+ it("renders a row per environment (with the unnamed fallback) and opens
the row on click", () => {
+ seedList([
+ { veid: 1, name: "envA", packages: {} },
+ { veid: 2, name: "", packages: {} },
+ ] as UserPveRecord[]);
+
+ const rows =
fixture.debugElement.queryAll(By.css("li.python-env-page-item"));
+ expect(rows.length).toBe(2);
+ expect((fixture.nativeElement as
HTMLElement).textContent).toContain("(unnamed)");
+
+ rows[0].triggerEventHandler("click", {});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft?.name).toBe("envA");
+ });
+
+ it("fires confirmDeletePve from the row delete icon and stops row-open
propagation", () => {
+ seedList([{ veid: 3, name: "envDel", packages: {} }] as UserPveRecord[]);
+ const stopPropagation = vi.fn();
+
fixture.debugElement.query(By.css(".python-env-delete-icon")).triggerEventHandler("click",
{ stopPropagation });
+ expect(stopPropagation).toHaveBeenCalled();
+ expect(confirmSpy).toHaveBeenCalledTimes(1);
+ expect(component.pveModalVisible).toBe(false);
+ });
+
+ it("renders the modal form, package header, one row per package, and the
footer when open", () => {
+ fixture.detectChanges();
+ const o = openModalWith({
+ name: "envForm",
+ newPackages: [
+ { name: "numpy", versionOp: "==", version: "1.2" },
+ { name: "pandas", versionOp: ">=", version: "2.0" },
+ ],
+ });
+
+ expect(o.querySelector(".ve-form")).not.toBeNull();
+ // header row (*ngIf newPackages.length > 0) + one row per package
+ expect(o.querySelectorAll(".package-row").length).toBe(3);
+ expect(o.querySelector(".add-btn button")).not.toBeNull();
+ expect(o.querySelectorAll(".footer-all button").length).toBe(2);
+ });
+
+ it("drives the modal package controls and the Save footer button through
the DOM", () => {
+ fixture.detectChanges();
+ const o = openModalWith({ name: "envDrive", newPackages: [{ name: "x",
versionOp: "==", version: "1" }] });
+
+ (o.querySelector(".add-btn button") as HTMLButtonElement).click();
Review Comment:
These assertions rely on `querySelector(...) as HTMLButtonElement` and will
throw if the selector doesn’t match (e.g., template refactor), producing less
actionable failures. Prefer asserting the element exists before interacting (or
use `fixture.debugElement.query(...)` with expectations) so failures clearly
indicate 'element not found' vs. a null dereference.
##########
frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts:
##########
@@ -377,4 +387,116 @@ describe("UserVenvComponent", () => {
expect(component.trackByVeid(1, { name: "", newPackages: []
})).toBeUndefined();
});
});
+
+ // The class is well covered above; these exercise the template itself — the
list
+ // branches and the nz-modal body/footer, which render into the CDK overlay.
+ describe("template rendering", () => {
+ type Draft = NonNullable<UserVenvComponent["currentDraft"]>;
+
+ // nz-modal renders into the overlay attached to ApplicationRef, so tick()
after
+ // detectChanges to flush its embedded view.
+ const flushOverlay = (): void => {
+ fixture.detectChanges();
+ TestBed.inject(ApplicationRef).tick();
+ };
+ const overlay = (): HTMLElement =>
document.querySelector(".cdk-overlay-container") as HTMLElement;
+
+ const openModalWith = (draft: Draft): HTMLElement => {
+ component.currentDraft = draft;
+ component.pveModalVisible = true;
+ flushOverlay();
+ return overlay();
+ };
+
+ const seedList = (records: UserPveRecord[]): void => {
+ pveServiceSpy.listUserPves.mockReturnValue(of(records));
+ fixture.detectChanges();
+ };
+
+ it("shows the empty-state message and no list when there are no
environments", () => {
+ seedList([]);
+ const host = fixture.nativeElement as HTMLElement;
+
expect(host.querySelector(".python-env-page-empty")?.textContent).toContain("No
environments yet");
+ expect(host.querySelector("ul.python-env-page-list")).toBeNull();
+ });
+
+ it("opens an empty draft modal from the Create button", () => {
+ fixture.detectChanges();
+
fixture.debugElement.query(By.css(".create-btn")).triggerEventHandler("click",
{});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft).toEqual({ name: "", newPackages: [] });
+ });
+
+ it("renders a row per environment (with the unnamed fallback) and opens
the row on click", () => {
+ seedList([
+ { veid: 1, name: "envA", packages: {} },
+ { veid: 2, name: "", packages: {} },
+ ] as UserPveRecord[]);
+
+ const rows =
fixture.debugElement.queryAll(By.css("li.python-env-page-item"));
+ expect(rows.length).toBe(2);
+ expect((fixture.nativeElement as
HTMLElement).textContent).toContain("(unnamed)");
+
+ rows[0].triggerEventHandler("click", {});
+ expect(component.pveModalVisible).toBe(true);
+ expect(component.currentDraft?.name).toBe("envA");
+ });
+
+ it("fires confirmDeletePve from the row delete icon and stops row-open
propagation", () => {
+ seedList([{ veid: 3, name: "envDel", packages: {} }] as UserPveRecord[]);
+ const stopPropagation = vi.fn();
+
fixture.debugElement.query(By.css(".python-env-delete-icon")).triggerEventHandler("click",
{ stopPropagation });
+ expect(stopPropagation).toHaveBeenCalled();
+ expect(confirmSpy).toHaveBeenCalledTimes(1);
+ expect(component.pveModalVisible).toBe(false);
+ });
+
+ it("renders the modal form, package header, one row per package, and the
footer when open", () => {
+ fixture.detectChanges();
+ const o = openModalWith({
+ name: "envForm",
+ newPackages: [
+ { name: "numpy", versionOp: "==", version: "1.2" },
+ { name: "pandas", versionOp: ">=", version: "2.0" },
+ ],
+ });
+
+ expect(o.querySelector(".ve-form")).not.toBeNull();
+ // header row (*ngIf newPackages.length > 0) + one row per package
+ expect(o.querySelectorAll(".package-row").length).toBe(3);
+ expect(o.querySelector(".add-btn button")).not.toBeNull();
+ expect(o.querySelectorAll(".footer-all button").length).toBe(2);
+ });
+
+ it("drives the modal package controls and the Save footer button through
the DOM", () => {
+ fixture.detectChanges();
+ const o = openModalWith({ name: "envDrive", newPackages: [{ name: "x",
versionOp: "==", version: "1" }] });
+
+ (o.querySelector(".add-btn button") as HTMLButtonElement).click();
+ flushOverlay();
+ expect(component.currentDraft?.newPackages.length).toBe(2);
+
+ (o.querySelector(".package-row .user-package-inputs button") as
HTMLButtonElement).click();
Review Comment:
These assertions rely on `querySelector(...) as HTMLButtonElement` and will
throw if the selector doesn’t match (e.g., template refactor), producing less
actionable failures. Prefer asserting the element exists before interacting (or
use `fixture.debugElement.query(...)` with expectations) so failures clearly
indicate 'element not found' vs. a null dereference.
--
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]
