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-6785-10e568b54e02b79e3fdf774d96377fef3d25d24e in repository https://gitbox.apache.org/repos/asf/texera.git
commit 3114a4d46b4bc00baad58a945c52bc4419093abb Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Jul 22 17:04:15 2026 -0700 test(frontend): extend VersionsListComponent unit test coverage (#6785) ### What changes were proposed in this PR? Extends `versions-list.component.spec.ts` (11 tests), covering the version load/select/revert/display flows and their error/guard branches. Coverage 46% -> high. No existing tests modified. ### Any related issues, documentation, discussions? Closes #6779. ### How was this PR tested? `ng test --include='**/versions-list.component.spec.ts'` -> 11/11 passing. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../versions-list/versions-list.component.spec.ts | 162 ++++++++++++++++++++- 1 file changed, 161 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts index a449218b8f..7517e1547a 100644 --- a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts +++ b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts @@ -19,6 +19,7 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { WorkflowActionService } from "../../../service/workflow-graph/model/workflow-action.service"; +import { DEFAULT_WORKFLOW } from "../../../service/workflow-graph/model/workflow-action.service"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { FormsModule, ReactiveFormsModule } from "@angular/forms"; import { FormlyModule } from "@ngx-formly/core"; @@ -27,11 +28,16 @@ import { HttpClientTestingModule } from "@angular/common/http/testing"; import { VersionsListComponent } from "./versions-list.component"; import { RouterTestingModule } from "@angular/router/testing"; import { commonTestProviders } from "../../../../common/testing/test-utils"; +import { WorkflowVersionService } from "../../../../dashboard/service/user/workflow-version/workflow-version.service"; +import { WorkflowVersionEntry } from "../../../../dashboard/type/workflow-version-entry"; +import { Workflow } from "../../../../common/type/workflow"; +import { of } from "rxjs"; describe("VersionsListComponent", () => { let component: VersionsListComponent; let fixture: ComponentFixture<VersionsListComponent>; let workflowActionService: WorkflowActionService; + let workflowVersionService: WorkflowVersionService; beforeEach(async () => { await TestBed.configureTestingModule({ @@ -50,10 +56,164 @@ describe("VersionsListComponent", () => { fixture = TestBed.createComponent(VersionsListComponent); component = fixture.componentInstance; workflowActionService = TestBed.inject(WorkflowActionService); - fixture.detectChanges(); + workflowVersionService = TestBed.inject(WorkflowVersionService); + // Intentionally do NOT call fixture.detectChanges() here: the ngOnInit specs configure + // their spies and then invoke ngOnInit() themselves, so this lets them observe the + // first/only invocation. Tests that need the rendered template call detectChanges() locally. + }); + + afterEach(() => { + fixture.destroy(); + vi.restoreAllMocks(); }); it("should create", () => { expect(component).toBeTruthy(); }); + + // Helper: build a collapsable-entry list directly on the component. + const makeEntry = (vId: number, importance: boolean, expand = false) => ({ + vId, + creationTime: vId * 1000, + content: `content-${vId}`, + importance, + expand, + }); + + describe("getDisplayedVersionId", () => { + it("should compute the descending displayed id as count - index", () => { + expect(component.getDisplayedVersionId(0, 5)).toBe(5); + expect(component.getDisplayedVersionId(2, 5)).toBe(3); + expect(component.getDisplayedVersionId(4, 5)).toBe(1); + }); + }); + + describe("collapse", () => { + it("should do nothing when versionsList is undefined", () => { + component.versionsList = undefined; + // must not throw when there is no list to walk + expect(() => component.collapse(0, true)).not.toThrow(); + expect(component.versionsList).toBeUndefined(); + }); + + it("should expand the trailing non-important entries when $event is true", () => { + component.versionsList = [ + makeEntry(5, true), + makeEntry(4, false), + makeEntry(3, false), + makeEntry(2, true), + makeEntry(1, false), + ]; + + component.collapse(0, true); + + // entries 1 and 2 (non-important) are expanded, then the walk stops at the + // next important entry (index 3); index 3 and 4 are left untouched. + expect(component.versionsList[1].expand).toBe(true); + expect(component.versionsList[2].expand).toBe(true); + expect(component.versionsList[3].expand).toBe(false); + expect(component.versionsList[4].expand).toBe(false); + }); + + it("should collapse the trailing non-important entries when $event is false", () => { + component.versionsList = [ + makeEntry(5, true), + makeEntry(4, false, true), + makeEntry(3, false, true), + makeEntry(2, true, true), + makeEntry(1, false, true), + ]; + + component.collapse(0, false); + + expect(component.versionsList[1].expand).toBe(false); + expect(component.versionsList[2].expand).toBe(false); + // stops at the important entry, so its expand state is preserved + expect(component.versionsList[3].expand).toBe(true); + expect(component.versionsList[4].expand).toBe(true); + }); + + it("should stop immediately when the next entry is important", () => { + component.versionsList = [makeEntry(3, false), makeEntry(2, true), makeEntry(1, false)]; + + component.collapse(0, true); + + // the very next entry (index 1) is important, so nothing is changed + expect(component.versionsList[1].expand).toBe(false); + expect(component.versionsList[2].expand).toBe(false); + }); + }); + + describe("ngOnInit", () => { + it("should unhighlight the currently highlighted elements", () => { + const wrapper = workflowActionService.getJointGraphWrapper(); + const highlights = { operators: ["op-1"], groups: [], links: [], commentBoxes: [], ports: [] }; + vi.spyOn(wrapper, "getCurrentHighlights").mockReturnValue(highlights as any); + const unhighlightSpy = vi.spyOn(wrapper, "unhighlightElements").mockImplementation(() => {}); + + component.ngOnInit(); + + expect(unhighlightSpy).toHaveBeenCalledWith(highlights); + }); + + it("should not retrieve versions when the route has no workflow id", () => { + (component.route.snapshot.params as any).id = undefined; + const retrieveSpy = vi.spyOn(workflowVersionService, "retrieveVersionsOfWorkflow"); + component.versionsList = undefined; + + component.ngOnInit(); + + expect(retrieveSpy).not.toHaveBeenCalled(); + expect(component.versionsList).toBeUndefined(); + }); + + it("should load and map the retrieved versions with expand defaulting to false", () => { + const entries: WorkflowVersionEntry[] = [ + { vId: 10, creationTime: 1000, content: "c10", importance: true }, + { vId: 9, creationTime: 900, content: "c9", importance: false }, + ]; + (component.route.snapshot.params as any).id = 42; + const retrieveSpy = vi.spyOn(workflowVersionService, "retrieveVersionsOfWorkflow").mockReturnValue(of(entries)); + + component.ngOnInit(); + + expect(retrieveSpy).toHaveBeenCalledWith(42); + expect(component.versionsList).toEqual([ + { vId: 10, creationTime: 1000, content: "c10", importance: true, expand: false }, + { vId: 9, creationTime: 900, content: "c9", importance: false, expand: false }, + ]); + }); + }); + + describe("getVersion", () => { + const mockWorkflow = { content: {} } as unknown as Workflow; + + it("should select the row, fetch the version and display it", () => { + const retrieveSpy = vi + .spyOn(workflowVersionService, "retrieveWorkflowByVersion") + .mockReturnValue(of(mockWorkflow)); + const displaySpy = vi.spyOn(workflowVersionService, "displayParticularVersion").mockImplementation(() => {}); + + component.getVersion(7, 3, 2); + + // selected row is recorded immediately (synchronously) + expect(component.selectedRowIndex).toBe(2); + // wid comes from the workflow metadata (default is 0) + expect(retrieveSpy).toHaveBeenCalledWith(0, 7); + expect(displaySpy).toHaveBeenCalledWith(mockWorkflow, 7, 3); + }); + + it("should use the current workflow metadata wid when fetching the version", () => { + workflowActionService.setWorkflowMetadata({ ...DEFAULT_WORKFLOW, wid: 99 }); + const retrieveSpy = vi + .spyOn(workflowVersionService, "retrieveWorkflowByVersion") + .mockReturnValue(of(mockWorkflow)); + vi.spyOn(workflowVersionService, "displayParticularVersion").mockImplementation(() => {}); + + component.getVersion(4, 1, 0); + + expect(retrieveSpy).toHaveBeenCalledWith(99, 4); + expect(component.selectedRowIndex).toBe(0); + }); + }); });
