This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7480-97cb8cf699e676e7d9ee1ca643da58d0b3bc8b84 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 306944a3ecc3466cc514cd0d8c1f8711d0f4dbe0 Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Aug 9 18:17:13 2026 -0700 test(frontend): drive the virtual-environment socket (#7480) ### What changes were proposed in this PR? The PVE block is the largest uncovered region in this component, and the existing suite stops exactly at its seams: it stubs `runPveWebSocket` and `deleteUserPackages` so it can assert name validation without opening a socket. These drive the other side of those seams. Adds 9 tests using a stand-in `WebSocket`, so `onmessage` and `onerror` can be fired by hand. Covered: the socket opening and locking the card, the name being trimmed before it reaches the URL, server lines appending to the pip output, the `__DONE__` sentinel closing the socket and clearing installing without printing itself, a dropped connection surfacing as output rather than a hang, a still-open socket being closed before another starts, and the create path chaining delete-then-install. **Verified by mutation**, all reverted (production diff empty): | Mutation | Result | |---|---| | name not trimmed for the socket URL | red | | previous socket left open | red | | card not locked while installing | red | | sentinel not recognised | red | | done leaves installing set | red | | done does not close the socket | red | | done skips the continuation | red | | error not surfaced in the output | red | | error leaves the card installing | red | The "done leaves installing set" mutation **survived a first pass**, and the reason is worth recording: the test let the real delete/install continuation run, which resets `isInstalling` downstream — so it was observing the continuation's state, not the sentinel branch's. Stubbing the continuation is what makes the test discriminate, and that is commented in the spec. No production file is touched. ### Any related issues, documentation, discussions? Closes #7477 ### How was this PR tested? ``` npx ng test --watch=false --include="**/computing-unit-selection.component.spec.ts" ``` ``` Test Files 1 passed (1) Tests 109 passed (109) ``` 9 new on top of the existing 100. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) Co-authored-by: Meng Wang <[email protected]> --- .../computing-unit-selection.component.spec.ts | 175 +++++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts index 481f595dea..4d6777e0ff 100644 --- a/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts +++ b/frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts @@ -666,6 +666,181 @@ describe("PowerButtonComponent", () => { }); }); + /** + * The PVE block is the largest uncovered region in the component, and the suite above stops + * exactly at its seams: it stubs out `runPveWebSocket` and `deleteUserPackages` so it can assert + * name validation without opening a socket. These drive the other side of those seams. + * + * The only environmental need is a stand-in for `WebSocket`, so `onmessage` / `onerror` can be + * fired by hand — jsdom has no WebSocket and the component never inspects anything but the + * handlers it assigns. + */ + describe("the virtual-environment socket", () => { + /** The sockets opened during a test, newest last. */ + let sockets: FakeSocket[]; + + class FakeSocket { + static opened: FakeSocket[] = []; + onmessage: ((e: { data: string }) => void) | null = null; + onerror: (() => void) | null = null; + close = vi.fn(); + constructor(public url: string) { + FakeSocket.opened.push(this); + } + /** Delivers one server line to the component. */ + say(data: string): void { + this.onmessage?.({ data }); + } + } + + /** Installs one unlocked environment and returns the component's view of it. */ + function setPve(over: Record<string, unknown> = {}): void { + component.pves = [ + { + name: "envone", + isLocked: false, + isInstalling: false, + userPackages: [], + newPackages: [], + deletingPackages: [], + pipOutput: "", + prettyPipOutput: "", + expanded: false, + ...over, + } as any, + ]; + } + + beforeEach(() => { + FakeSocket.opened = []; + sockets = FakeSocket.opened; + vi.stubGlobal("WebSocket", FakeSocket as unknown as typeof WebSocket); + vi.spyOn((component as any).notificationService, "error").mockImplementation(() => {}); + // The socket URL is built from the selected unit's cuid, so a unit has to be selected. + component.selectedComputingUnit = makeComputingUnit({ name: "unit" }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("opens a socket for the environment and locks the card while it runs", () => { + setPve(); + + component.createVirtualEnvironment(0); + + expect(sockets).toHaveLength(1); + expect(component.pves[0].isInstalling).toBe(true); + expect(component.pves[0].isLocked).toBe(true); + expect(component.pves[0].pipOutput).toContain("Creating virtual environment"); + }); + + it("trims the environment name before it reaches the socket URL", () => { + // The name is user input from an inline editor; an untrimmed name produces a URL for an + // environment the backend does not have. + const urlSpy = vi.spyOn(TestBed.inject(WorkflowPveService), "getPveWebSocketUrl"); + setPve({ name: " spaced " }); + + component.createVirtualEnvironment(0); + + expect(component.pves[0].name).toBe("spaced"); + expect(urlSpy).toHaveBeenCalledWith(expect.anything(), "spaced", "create", []); + }); + + it("appends each line the server sends to the pip output", () => { + setPve(); + component.createVirtualEnvironment(0); + + sockets[0].say("collecting numpy"); + sockets[0].say("installed"); + + expect(component.pves[0].pipOutput).toContain("collecting numpy"); + expect(component.pves[0].pipOutput).toContain("installed"); + // Still running: the card stays locked until the sentinel arrives. + expect(component.pves[0].isInstalling).toBe(true); + }); + + it("closes the socket and unlocks installing on the __DONE__ sentinel", () => { + // Without this the spinner never stops and the card is stuck mid-install. + // The continuation is stubbed out deliberately: letting the real delete/install chain run + // would reset isInstalling downstream, so the assertion would pass even if this branch + // never cleared it. + vi.spyOn(component as any, "deleteUserPackages").mockImplementation(() => {}); + setPve(); + component.createVirtualEnvironment(0); + + sockets[0].say("__DONE__"); + + expect(component.pves[0].isInstalling).toBe(false); + expect(component.pves[0].socket).toBeUndefined(); + expect(sockets[0].close).toHaveBeenCalled(); + }); + + it("does not print the sentinel as though it were output", () => { + setPve(); + component.createVirtualEnvironment(0); + + sockets[0].say("__DONE__"); + + expect(component.pves[0].pipOutput).not.toContain("__DONE__"); + }); + + it("reports a dropped connection in the output rather than hanging", () => { + setPve(); + component.createVirtualEnvironment(0); + + sockets[0].onerror?.(); + + expect(component.pves[0].pipOutput).toContain("[WebSocket error]"); + expect(component.pves[0].isInstalling).toBe(false); + expect(component.pves[0].socket).toBeUndefined(); + }); + + it("closes any socket still open on the card before starting another", () => { + // Re-running a create on a card that is already streaming would otherwise leave the first + // socket writing into the same pipOutput. + setPve(); + component.createVirtualEnvironment(0); + const first = sockets[0]; + + component.pves[0] = { ...component.pves[0], isLocked: false } as any; + component.createVirtualEnvironment(0); + + expect(first.close).toHaveBeenCalled(); + expect(sockets).toHaveLength(2); + }); + + it("reinstalls the recorded packages once the environment is created", () => { + // The create socket's completion chains delete-then-install, which is how a rebuilt + // environment gets its packages back. + const deleteSpy = vi + .spyOn(component as any, "deleteUserPackages") + .mockImplementation((...args: unknown[]) => (args[1] as (() => void) | undefined)?.()); + const installSpy = vi.spyOn(component as any, "installUserPackages").mockImplementation(() => {}); + setPve(); + + component.createVirtualEnvironment(0); + sockets[0].say("__DONE__"); + + expect(deleteSpy).toHaveBeenCalled(); + expect(installSpy).toHaveBeenCalled(); + }); + + it("skips creation entirely for a locked card and only syncs its packages", () => { + const deleteSpy = vi + .spyOn(component as any, "deleteUserPackages") + .mockImplementation((...args: unknown[]) => (args[1] as (() => void) | undefined)?.()); + const installSpy = vi.spyOn(component as any, "installUserPackages").mockImplementation(() => {}); + setPve({ isLocked: true }); + + component.createVirtualEnvironment(0); + + expect(sockets).toHaveLength(0); + expect(deleteSpy).toHaveBeenCalled(); + expect(installSpy).toHaveBeenCalled(); + }); + }); + describe("createVirtualEnvironment name validation", () => { const VALIDATION_MSG = "Environment name must contain only letters and numbers.";
