Copilot commented on code in PR #8101:
URL: https://github.com/apache/texera/pull/8101#discussion_r3886256892
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts:
##########
@@ -839,6 +922,160 @@ describe("PowerButtonComponent", () => {
expect(deleteSpy).toHaveBeenCalled();
expect(installSpy).toHaveBeenCalled();
});
+
+ it("refuses to create a second environment under a name another card
already uses", () => {
+ setPve();
+ // The nameless card is scanned first, so the duplicate check has to
survive it before it
+ // reaches the card that actually clashes.
+ const card = component.pves[0];
+ component.pves = [{ ...card, name: undefined }, { ...card }, { ...card,
name: " envone " }] as any;
+
+ component.createVirtualEnvironment(2);
+
+ expect(errorSpy).toHaveBeenCalledWith("An environment with this name
already exists.");
+ expect(sockets).toHaveLength(0);
+ expect(component.pves[2].isInstalling).toBeFalsy();
+ });
+
+ it("starts the log from empty when the card has produced no output yet",
() => {
+ setPve();
+ component.createVirtualEnvironment(0);
+ // runPveWebSocket seeds the card with its banner; clearing it models a
card whose log
+ // was reset while the socket was still attached, and exercises the
handler's fallback.
+ (component.pves[0] as { pipOutput?: string }).pipOutput = undefined;
+
+ sockets[0].say("collecting numpy");
+
+ expect(component.pves[0].pipOutput).toBe("collecting numpy\n");
+ });
+
+ it("records a dropped connection even when the card has produced no output
yet", () => {
+ setPve();
+ component.createVirtualEnvironment(0);
+ (component.pves[0] as { pipOutput?: string }).pipOutput = undefined;
+
+ sockets[0].onerror?.();
+
+ expect(component.pves[0].pipOutput).toBe("\n[WebSocket error]\n");
+ expect(component.pves[0].isInstalling).toBe(false);
+ });
+
+ describe("installUserPackages", () => {
+ it("refuses a package that is missing its operator or its version", ()
=> {
+ setPve({ newPackages: [{ name: "numpy", versionOp: undefined, version:
"1.26.0" }] });
+ (component as any).installUserPackages(0);
+ expect(errorSpy).toHaveBeenLastCalledWith("Please specify an operator
and version for each package.");
+
+ setPve({ newPackages: [{ name: "numpy", versionOp: "==", version: "
" }] });
+ (component as any).installUserPackages(0);
+ expect(errorSpy).toHaveBeenLastCalledWith("Please specify an operator
and version for each package.");
+
+ expect(sockets).toHaveLength(0);
+ });
+
+ it("skips a package already present as a system package or in the
environment", () => {
+ vi.spyOn(TestBed.inject(WorkflowPveService),
"getUserPackages").mockReturnValue(of([]));
+ // The system list is matched case-insensitively, so "NumPy" must
block "numpy".
+ component.systemPackages = [{ name: "NumPy", version: "1.26.0" }];
+ setPve({
+ userPackages: [{ name: "pandas", versionOp: "==", version: "2.0.0"
}],
+ newPackages: [
+ { name: " ", versionOp: "==", version: "1.0.0" },
+ { name: "numpy", versionOp: "==", version: "1.26.0" },
+ { name: "Pandas", versionOp: "==", version: "2.0.0" },
+ ],
+ });
+
+ (component as any).installUserPackages(0);
+
+ expect(errorSpy).toHaveBeenCalledWith("Skipped numpy: already
installed as a system package.");
+ expect(errorSpy).toHaveBeenCalledWith("Skipped Pandas: already
installed in this environment.");
+ // Nothing survives the filters, so no socket is opened and the draft
rows are cleared.
+ expect(sockets).toHaveLength(0);
+ expect(component.pves[0].newPackages).toEqual([]);
+ expect(component.pves[0].isInstalling).toBe(false);
+ });
+
+ it("has nothing to install when the card carries no draft rows at all",
() => {
+ const refreshSpy = vi.spyOn(TestBed.inject(WorkflowPveService),
"getUserPackages").mockReturnValue(of([]));
+ setPve({ newPackages: undefined });
+
+ (component as any).installUserPackages(0);
+
+ expect(sockets).toHaveLength(0);
+ expect(refreshSpy).toHaveBeenCalledWith(1, "envone");
+ });
+
+ it("opens an install socket for the surviving packages and reloads the
list when it finishes", () => {
+ const urlSpy = vi.spyOn(TestBed.inject(WorkflowPveService),
"getPveWebSocketUrl");
+ vi.spyOn(TestBed.inject(WorkflowPveService),
"getUserPackages").mockReturnValue(of(["numpy==1.26.0"]));
+ component.systemPackages = [];
+ setPve({ newPackages: [{ name: " numpy ", versionOp: "==", version:
" 1.26.0 " }] });
+
+ (component as any).installUserPackages(0);
+
+ expect(urlSpy).toHaveBeenCalledWith(1, "envone", "install",
["numpy==1.26.0"]);
+ expect(sockets).toHaveLength(1);
+
+ sockets[0].say("__DONE__");
+
+ expect(component.pves[0].newPackages).toEqual([]);
+ expect(component.pves[0].userPackages).toEqual([{ name: "numpy",
versionOp: "==", version: "1.26.0" }]);
+ });
+
+ it("logs and keeps the current list when reloading the packages fails",
() => {
+ const consoleSpy = vi.spyOn(console, "error").mockImplementation(() =>
{});
+ vi.spyOn(TestBed.inject(WorkflowPveService),
"getUserPackages").mockReturnValue(
+ throwError(() => new Error("nope"))
+ );
+ setPve({ userPackages: [{ name: "numpy", versionOp: "==", version:
"1.26.0" }] });
+
+ (component as any).refreshUserPackages(0);
+
+ expect(consoleSpy).toHaveBeenCalledWith("Failed to refresh user
packages", expect.any(Error));
+ expect(component.pves[0].userPackages).toEqual([{ name: "numpy",
versionOp: "==", version: "1.26.0" }]);
+ });
Review Comment:
`console.error` is spied but never restored in this test. Since `console` is
global and this spec file doesn’t have a blanket `afterEach(() =>
vi.restoreAllMocks())`, the mocked `console.error` can leak into later tests
and hide failures or break assertions that rely on the real implementation.
Wrap the body in a try/finally (or manually restore at the end) to guarantee
cleanup even if an assertion throws.
--
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]
