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-7688-fbee5c7259538369c2cc6d0e654d48db25b8c9a2 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 83e6ef3ecdcc5c6affefb4f1dfc5a17f6d24b5e8 Author: Meng Wang <[email protected]> AuthorDate: Sat Aug 15 05:00:52 2026 +0000 test(frontend): cover the missing branches in the dataset modal, UDF debug, hub detail and dataset page (#7688) ### What changes were proposed in this PR? Takes the missing side of each conditional in the four files, so their partial branches clear. No production code was changed. **`DatasetSelectionModalComponent`** (+4) — no dataset selected, so no version list is fetched; a dataset selected in non-file mode, where the versions load but none is auto-selected; a dataset with no version, so the file tree is not fetched; and a file click in non-file mode, which leaves the path alone. **`UdfDebugService`** (+12) — a condition set on a line that has no breakpoint; clearing a breakpoint that lost its id; console events from another operator, with no messages, and not from the debugger; a status update that is not `Uninitialized`; stepping and deletion messages that carry no line number; a deletion for a line with no debug state; creation messages missing the id, and missing both id and line; a stepping message on an existing breakpoint; and the two `markContinue` combinations the existing test did not reach. **`HubWorkflowDetailComponent`** (+8) — a workflow with a description and one with an empty description, asserting the placeholder reaches the description child; `postUnlike` reporting failure; the refreshed counts carrying no `like` on both the like and the unlike path; and the two `wid` re-checks inside the like/unlike handlers. The two like/unlike responses are asynchronous in production, so a `Subject` stands in for the pending request and the id is cleared between issuing the call and the response arriving — `of(...)` resolves too early to reach those `return`s. **`UserDatasetComponent`** (+9) — both view-child accessors read before the view is initialized, so each `throw` runs, and read after assignment; the filter component reporting a change; and the search de-duplication guard driven one condition at a time: unchanged, forced, a changed sort method, a filter added, and a filter replaced by another (same length, different contents, which only the element-wise comparison distinguishes). Two spots named by the issue are not reachable and are left uncovered: - `UserDatasetComponent`'s `if (!this.searchResultsComponent) throw new Error("searchResultsComponent is undefined.")` is dead. The accessor above it either returns a truthy component or throws, so the negation is never true — the "before it is initialized" error is what actually fires, and that is the one the tests assert. - `hub-workflow-detail.component.ts:65` is the class declaration line, which the source map attributes TypeScript's emitted decorator helper to. It was already uncovered before this change (baseline unhit lines were `65,239,257`; 239 and 257 are the two `return`s this PR covers). ### Any related issues, documentation, discussions? Closes #7686 ### How was this PR tested? Extended unit tests, run locally in `frontend/` (all green; the failure paths were verified by breaking assertions in each file and confirming the suites go red and exit non-zero): ``` ng test --watch=false --include .../dataset-selection-modal.component.spec.ts # 11 passed ng test --watch=false --include .../udf-debug.service.spec.ts # 33 passed ng test --watch=false --include .../hub-workflow-detail.component.spec.ts # 42 passed ng test --watch=false --include .../user-dataset.component.spec.ts # 35 passed prettier --write <specs> # clean eslint <specs> # clean ``` The coverage report was re-run over the four specs to confirm the partials cleared: `dataset-selection-modal.component.ts` and `udf-debug.service.ts` reach 100% of statements with no partial or never-taken branch left; `hub-workflow-detail.component.ts` and `user-dataset.component.ts` are left only with the two unreachable spots described above. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../user-dataset/user-dataset.component.spec.ts | 94 ++++++++++++ .../detail/hub-workflow-detail.component.spec.ts | 90 ++++++++++- .../dataset-selection-modal.component.spec.ts | 44 ++++++ .../operator-debug/udf-debug.service.spec.ts | 169 ++++++++++++++++++++- 4 files changed, 395 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset.component.spec.ts index 1ccd5918fb..698cb1f8f9 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset.component.spec.ts @@ -381,6 +381,100 @@ describe("UserDatasetComponent", () => { expect(makeFreshComponent().viewType).toBe("card"); }); }); + + describe("view child accessors", () => { + // The outer beforeEach assigns both view children, so these build a component that + // has never had a view attached. + const componentWithNoView = () => + new UserDatasetComponent( + modalServiceMock as any, + { + userChanged: () => new Subject<User | undefined>().asObservable(), + isLogin: () => true, + getCurrentUser: () => ({ uid: 42 }) as User, + } as any, + routerMock as any, + searchServiceMock as any, + datasetServiceMock as any, + messageMock as any + ); + + it("rejects reading searchResultsComponent before the view is initialized", () => { + expect(() => componentWithNoView().searchResultsComponent).toThrowError( + "Property cannot be accessed before it is initialized." + ); + }); + + it("rejects reading filters before the view is initialized", () => { + expect(() => componentWithNoView().filters).toThrowError("Property cannot be accessed before it is initialized."); + }); + + it("returns the view children once they are assigned", () => { + expect(component.searchResultsComponent).toBe(searchResultsStub); + expect(component.filters).toBe(filtersStub); + }); + + it("re-runs the search when the filter component reports a change", () => { + const search = vi.spyOn(component, "search").mockResolvedValue(undefined); + + // the setter is what wires this subscription up + filtersStub.masterFilterListChange.next(); + + expect(search).toHaveBeenCalled(); + }); + }); + + describe("search de-duplication", () => { + it("skips a repeat search when the filters and sort are unchanged", async () => { + filtersStub.masterFilterList = ["a"]; + await component.search(); + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(1); + + await component.search(); + + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(1); + }); + + it("runs the search again when it is forced", async () => { + filtersStub.masterFilterList = ["a"]; + await component.search(); + + await component.search(true); + + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(2); + }); + + it("runs the search again when the sort method changed", async () => { + filtersStub.masterFilterList = ["a"]; + await component.search(); + + component.sortMethod = SortMethod.NameAsc; + await component.search(); + + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(2); + }); + + it("runs the search again when a filter was added", async () => { + await component.search(); + + filtersStub.masterFilterList = ["a"]; + await component.search(); + + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(2); + }); + + it("runs the search again when a filter was replaced by another", async () => { + filtersStub.masterFilterList = ["a"]; + await component.search(); + + // same length, different contents: only the element-wise comparison can tell + // these two lists apart + filtersStub.masterFilterList = ["b"]; + await component.search(); + + expect(searchResultsStub.loadMore).toHaveBeenCalledTimes(2); + }); + }); }); /** * The existing suite constructs the component directly, so its template has never been rendered. diff --git a/frontend/src/app/hub/component/workflow/detail/hub-workflow-detail.component.spec.ts b/frontend/src/app/hub/component/workflow/detail/hub-workflow-detail.component.spec.ts index 2e45eaa8e0..31c3c8e0a4 100644 --- a/frontend/src/app/hub/component/workflow/detail/hub-workflow-detail.component.spec.ts +++ b/frontend/src/app/hub/component/workflow/detail/hub-workflow-detail.component.spec.ts @@ -23,7 +23,8 @@ import { ActivatedRoute, Router } from "@angular/router"; import { NzIconModule } from "ng-zorro-antd/icon"; import { NZ_MODAL_DATA } from "ng-zorro-antd/modal"; import { ArrowLeftOutline, EyeOutline, LikeOutline, UserOutline } from "@ant-design/icons-angular/icons"; -import { config, of, throwError } from "rxjs"; +import { By } from "@angular/platform-browser"; +import { config, of, Subject, throwError } from "rxjs"; import { vi } from "vitest"; import { HubWorkflowDetailComponent, THROTTLE_TIME_MS } from "./hub-workflow-detail.component"; @@ -280,6 +281,24 @@ describe("HubWorkflowDetailComponent", () => { build({ modalData: { wid: 1 }, userOverride: undefined }); expect(hubServiceMock.isLiked).not.toHaveBeenCalled(); }); + + it("assigns the fetched description and passes it to the description child", () => { + workflowPersistServiceMock.getWorkflowDescription.mockReturnValue(of("a real description")); + build({ modalData: { wid: 1 } }); + expect(component.workflowDescription).toBe("a real description"); + expect( + fixture.debugElement.query(By.directive(StubMarkdownDescriptionComponent)).componentInstance.description + ).toBe("a real description"); + }); + + it("substitutes a placeholder when the workflow has no description", () => { + workflowPersistServiceMock.getWorkflowDescription.mockReturnValue(of("")); + build({ modalData: { wid: 1 } }); + expect(component.workflowDescription).toBe("No description available"); + expect( + fixture.debugElement.query(By.directive(StubMarkdownDescriptionComponent)).componentInstance.description + ).toBe("No description available"); + }); }); describe("ngAfterViewInit / loadWorkflowWithId", () => { @@ -421,6 +440,75 @@ describe("HubWorkflowDetailComponent", () => { expect(component.isLiked).toBe(false); expect(hubServiceMock.getCounts).not.toHaveBeenCalled(); }); + + it("does not flip isLiked when postUnlike returns false", () => { + hubServiceMock.postUnlike.mockReturnValue(of(false)); + build({ modalData: { wid: 1 } }); + component.isLiked = true; + hubServiceMock.getCounts.mockClear(); + component.toggleLike(); + expect(component.isLiked).toBe(true); + expect(hubServiceMock.getCounts).not.toHaveBeenCalled(); + }); + + it("defaults likeCount to 0 when the counts refreshed after a like carry none", () => { + hubServiceMock.getCounts + .mockReturnValueOnce(of([{ entityId: 1, entityType: EntityType.Workflow, counts: { like: 4, clone: 0 } }])) + .mockReturnValueOnce(of([{ entityId: 1, entityType: EntityType.Workflow, counts: {} }])); + build({ modalData: { wid: 1 } }); + expect(component.likeCount).toBe(4); + + component.isLiked = false; + component.toggleLike(); + + expect(component.likeCount).toBe(0); + }); + + it("defaults likeCount to 0 when the counts refreshed after an unlike carry none", () => { + hubServiceMock.getCounts + .mockReturnValueOnce(of([{ entityId: 1, entityType: EntityType.Workflow, counts: { like: 4, clone: 0 } }])) + .mockReturnValueOnce(of([{ entityId: 1, entityType: EntityType.Workflow, counts: {} }])); + build({ modalData: { wid: 1 } }); + expect(component.likeCount).toBe(4); + + component.isLiked = true; + component.toggleLike(); + + expect(component.likeCount).toBe(0); + }); + + // The like/unlike responses are asynchronous in production, so `wid` is re-checked + // inside each handler. A subject stands in for the pending request so the id can be + // cleared between issuing the call and the response arriving. + it("skips the like refresh when wid disappears before the response", () => { + const pending = new Subject<boolean>(); + hubServiceMock.postLike.mockReturnValue(pending); + build({ modalData: { wid: 1 } }); + component.isLiked = false; + component.toggleLike(); + hubServiceMock.getCounts.mockClear(); + + component.wid = undefined; + pending.next(true); + + expect(component.isLiked).toBe(true); + expect(hubServiceMock.getCounts).not.toHaveBeenCalled(); + }); + + it("skips the unlike refresh when wid disappears before the response", () => { + const pending = new Subject<boolean>(); + hubServiceMock.postUnlike.mockReturnValue(pending); + build({ modalData: { wid: 1 } }); + component.isLiked = true; + component.toggleLike(); + hubServiceMock.getCounts.mockClear(); + + component.wid = undefined; + pending.next(true); + + expect(component.isLiked).toBe(false); + expect(hubServiceMock.getCounts).not.toHaveBeenCalled(); + }); }); describe("formatCount", () => { diff --git a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts index ddd595a10f..1eb0d1e646 100644 --- a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts +++ b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts @@ -171,4 +171,48 @@ describe("DatasetSelectionModalComponent", () => { expect(modalRef.close).toHaveBeenCalledWith("/some/path"); }); + + // Both handlers guard on a dataset (and a version) having been chosen, and behave + // differently in the two modes; the cases above only drive the file-mode arms. + it("onDatasetChange does nothing while no dataset is selected", () => { + build(); + datasetService.retrieveDatasetVersionList.mockClear(); + component.selectedDataset = undefined; + + component.onDatasetChange(); + + expect(datasetService.retrieveDatasetVersionList).not.toHaveBeenCalled(); + expect(component.fileTree).toEqual([]); + }); + + it("onDatasetChange loads the versions but auto-selects none in non-file mode", () => { + build(); // modalData.fileMode is false by default + component.selectedDataset = dataset; + + component.onDatasetChange(); + + expect(datasetService.retrieveDatasetVersionList).toHaveBeenCalledWith(dataset.dataset.did); + expect(component.datasetVersions).toEqual([version]); + expect(component.selectedVersion).toBeUndefined(); + }); + + it("onVersionChange does nothing while no version is selected", () => { + build(); + component.selectedDataset = dataset; + component.selectedVersion = undefined; + datasetService.retrieveDatasetVersionFileTree.mockClear(); + + component.onVersionChange(); + + expect(datasetService.retrieveDatasetVersionFileTree).not.toHaveBeenCalled(); + }); + + it("onFileSelected ignores the node in non-file mode", () => { + build(); // fileMode false + component.selectedPath = "/kept"; + + component.onFileSelected(fileNode); + + expect(component.selectedPath).toBe("/kept"); + }); }); diff --git a/frontend/src/app/workspace/service/operator-debug/udf-debug.service.spec.ts b/frontend/src/app/workspace/service/operator-debug/udf-debug.service.spec.ts index 3ea9184bdd..5f22bd3dbd 100644 --- a/frontend/src/app/workspace/service/operator-debug/udf-debug.service.spec.ts +++ b/frontend/src/app/workspace/service/operator-debug/udf-debug.service.spec.ts @@ -30,7 +30,7 @@ import { mockPoint, mockPythonUDFPredicate } from "../workflow-graph/model/mock- import { OperatorMetadataService } from "../operator-metadata/operator-metadata.service"; import { StubOperatorMetadataService } from "../operator-metadata/stub-operator-metadata.service"; import * as Y from "yjs"; -import { ConsoleUpdateEvent } from "../../types/workflow-common.interface"; +import { ConsoleMessage, ConsoleUpdateEvent } from "../../types/workflow-common.interface"; import { TexeraWebsocketEvent } from "../../types/workflow-websocket.interface"; import { commonTestProviders } from "../../../common/testing/test-utils"; import type { Mocked } from "vitest"; @@ -464,4 +464,171 @@ describe("UdfDebugServiceSpec", () => { expect(debugState.get("1")).toEqual({ breakpointId: 1, condition: "x > 5", hit: false }); expect(debugState.has("2")).toBe(false); }); + + // Builds the (Pdb) DEBUGGER console event shape that every handler below filters on. + function pdbEvent(title: string, overrides: Partial<ConsoleMessage> = {}): ConsoleUpdateEvent { + return { + operatorId: mockPythonUDFPredicate.operatorID, + messages: [ + { + workerId: stubWorker, + timestamp: { nanos: 0, seconds: 0 }, + title, + source: "(Pdb)", + msgType: { name: "DEBUGGER" }, + message: "", + ...overrides, + }, + ], + }; + } + + it("should not send a condition for a line that has no breakpoint", () => { + // The condition differs from the empty default, so the early return is passed and + // the `isDefined(breakpointInfo)` guard is the one that stops the update. + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + + service.doUpdateBreakpointCondition(mockPythonUDFPredicate.operatorID, 7, "x < 10"); + + expect(mockWorkflowWebsocketService.send).not.toHaveBeenCalled(); + expect(debugState.has("7")).toBe(false); + }); + + it("should clear a breakpoint that lost its id with an empty id", () => { + // State a hit breakpoint is left in once pdb deleted it: still present, so the + // command is `clear`, but with no id to clear by. The trailing space is pinned + // deliberately — it is the literal payload that goes over the websocket. + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + debugState.set("10", { breakpointId: undefined, condition: "", hit: true }); + + service.doModifyBreakpoint(mockPythonUDFPredicate.operatorID, 10); + + expect(mockWorkflowWebsocketService.send).toHaveBeenCalledWith("DebugCommandRequest", { + operatorId: mockPythonUDFPredicate.operatorID, + workerId: stubWorker, + cmd: "clear ", + }); + }); + + it("should ignore console events from another operator or with no messages", () => { + vi.spyOn(service as any, "markBreakpointAsHit"); + + consoleUpdateEventStream.next({ + ...pdbEvent("> /path/to/file.py(10)<module>()"), + operatorId: "some-other-operator", + }); + consoleUpdateEventStream.next({ operatorId: mockPythonUDFPredicate.operatorID, messages: [] }); + + expect(service["markBreakpointAsHit"]).not.toHaveBeenCalled(); + }); + + it("should ignore console messages that are not from the debugger", () => { + vi.spyOn(service as any, "markBreakpointAsHit"); + + consoleUpdateEventStream.next(pdbEvent("> /path/to/file.py(10)<module>()", { source: "stdout" })); + consoleUpdateEventStream.next(pdbEvent("> /path/to/file.py(10)<module>()", { msgType: { name: "PRINT" } })); + + expect(service["markBreakpointAsHit"]).not.toHaveBeenCalled(); + }); + + it("should keep the debug state on a status update that is not Uninitialized", () => { + const operatorId = mockPythonUDFPredicate.operatorID; + const debugState = service.getDebugState(operatorId); + debugState.set("10", { breakpointId: 1, condition: "", hit: false }); + + const running: OperatorStatistics = { + operatorState: OperatorState.Running, + aggregatedInputRowCount: 0, + aggregatedOutputRowCount: 0, + inputPortMetrics: {}, + outputPortMetrics: {}, + }; + statusUpdateStream.next({ [operatorId]: running }); + statusUpdateStream.next({ "some-other-operator": { ...running, operatorState: OperatorState.Uninitialized } }); + + expect(debugState.size).toBe(1); + }); + + it("should ignore a stepping message that carries no line number", () => { + vi.spyOn(service as any, "markBreakpointAsHit"); + // spied so the assertions below cannot be satisfied by the message never + // reaching the handler at all + vi.spyOn(service as any, "extractInfo"); + + consoleUpdateEventStream.next(pdbEvent("> <stdin> in the interactive shell")); + + expect(service["extractInfo"]).toHaveBeenCalledWith("> <stdin> in the interactive shell"); + expect(service["markBreakpointAsHit"]).not.toHaveBeenCalled(); + }); + + it("should ignore a deletion message that carries no line number", () => { + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + debugState.set("10", { breakpointId: 1, condition: "", hit: false }); + vi.spyOn(service, "doContinue"); + vi.spyOn(service as any, "extractInfo"); + + consoleUpdateEventStream.next(pdbEvent("Deleted all breakpoints")); + + expect(service["extractInfo"]).toHaveBeenCalledWith("Deleted all breakpoints"); + expect(debugState.has("10")).toBe(true); + expect(service.doContinue).not.toHaveBeenCalled(); + }); + + it("should ignore a deletion message for a line with no debug state", () => { + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + vi.spyOn(service, "doContinue"); + + consoleUpdateEventStream.next(pdbEvent("Deleted breakpoint 1 at /path/to/file.py:10")); + + expect(debugState.size).toBe(0); + // the handler returns before reaching the continue check + expect(service.doContinue).not.toHaveBeenCalled(); + }); + + it("should not record a breakpoint whose creation message has no id", () => { + // `Breakpoint <id> at <file>:<line>` is what carries an id; without one only the + // line is recovered, so the entry is skipped but the execution still continues. + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + vi.spyOn(service, "doContinue"); + + consoleUpdateEventStream.next(pdbEvent("Breakpoint at /path/to/file.py:10")); + + expect(debugState.has("10")).toBe(false); + expect(service.doContinue).toHaveBeenCalledWith(mockPythonUDFPredicate.operatorID, stubWorker); + }); + + it("should not record a breakpoint whose creation message has neither id nor line", () => { + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + vi.spyOn(service, "doContinue"); + + consoleUpdateEventStream.next(pdbEvent("Breakpoint already exists")); + + expect(debugState.size).toBe(0); + expect(service.doContinue).toHaveBeenCalledWith(mockPythonUDFPredicate.operatorID, stubWorker); + }); + + it("should mark an existing breakpoint as hit without discarding its condition", () => { + // The companion of the existing markBreakpointAsHit test, which starts from an + // empty state and therefore only creates the placeholder entry. + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + debugState.set("10", { breakpointId: 3, condition: "x > 5", hit: false }); + + consoleUpdateEventStream.next(pdbEvent("> /path/to/file.py(10)<module>()")); + + expect(debugState.get("10")).toEqual({ breakpointId: 3, condition: "x > 5", hit: true }); + }); + + it("should keep a hit breakpoint that still has an id when continuing", () => { + // The two combinations the existing markContinue test does not reach: a hit + // breakpoint with an id is reset rather than dropped, and an idle temporary one + // is dropped even though it was never hit. + const debugState = service.getDebugState(mockPythonUDFPredicate.operatorID); + debugState.set("1", { breakpointId: 1, condition: "x > 5", hit: true }); + debugState.set("2", { breakpointId: undefined, condition: "", hit: false }); + + service["markContinue"](mockPythonUDFPredicate.operatorID); + + expect(debugState.get("1")).toEqual({ breakpointId: 1, condition: "x > 5", hit: false }); + expect(debugState.has("2")).toBe(false); + }); });
