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-7701-363537e0251182472f540079d262599f6cbb5240 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 9d58135088c9a7bb074cb62e27cb97bfbd977fb2 Author: Meng Wang <[email protected]> AuthorDate: Mon Aug 17 02:25:26 2026 +0000 test(frontend): cover the remaining lines and branches in the coeditor icon, venv page, files uploader and preset wrapper (#7701) ### What changes were proposed in this PR? Covers the remaining lines and untaken branches in the four files. All four now report full line and branch coverage. No production code was changed. Two of the paths in the issue have moved: `CoeditorUserIconComponent` lives under `workspace/component/menu/`, and `PresetWrapperComponent` under `common/formly/`. **`CoeditorUserIconComponent`** (+5) — the shadowing menu had no test at all beyond "should create". Both arms of the compound guard now render: shadowing off, shadowing on for *another* co-editor (the second half flipped on its own), and shadowing on for this one. Each variant is then clicked, asserting the presence service receives `shadowCoeditor` / `stopShadowing`. **`UserVenvComponent`** (+5) — the `"(unnamed)"` fallback on both the confirm dialog and the delete notification, a record with no `packages`, a stored version that is nullish rather than empty (only nullish reaches `?? ""`), and a draft row whose version is null. **`FilesUploaderComponent`** (+5) — the existing suite constructs the component with `new`, so the template had never rendered (0% on the .html). A second block mounts it for real and drives the banner `*ngIf` through all four flag combinations, its message, its close handler, the drop-zone button and the drop handler. **`PresetWrapperComponent`** (+6) — a form control holding a value and holding `null`, `setupFieldConfig`, the `applyPresetStream` predicate with a matching and a non-matching event plus the `basePreset` assignment behind it, and the dropdown's own `nzVisibleChange` output. Two notes on how the DOM is driven here: - `nz-dropdown-menu` keeps its content in an ng-template that mounts into a CDK overlay only when the dropdown opens, which jsdom does not drive. Rather than assert on the bound data, the tests instantiate that template directly (`viewContainerRef.createEmbeddedView(templateRef)`), which puts the rows in the fixture's DOM so the `*ngFor`, the interpolations and the click handlers all really run. This replaces the data-only assertions the preset spec had for the same reason. - `PresetWrapperComponent`'s "does not refresh while the dropdown is closed" test never awaited the handler's `debounceTime(0)`, so it passed because the callback had not run yet rather than because the menu was closed — it could not fail. It now awaits the tick and also asserts the search term is still tracked. `ngx-file-drop` hands its `openFileSelector` to the content template by reference, so a spy installed after render is not seen; that test asserts the effect (the hidden file input is clicked) instead. ### Any related issues, documentation, discussions? Closes #7700 ### How was this PR tested? Extended unit tests, run locally in `frontend/` (all green; the failure paths were verified by breaking one assertion per file and confirming all four suites go red and the run exits non-zero): ``` ng test --watch=false --include <the four specs> # Test Files 4 passed (4) | Tests 111 passed (111) prettier --write <specs> # clean eslint <specs> # clean ``` The coverage report was re-run over the four specs to confirm the gaps cleared — `coeditor-user-icon.component.{ts,html}`, `user-venv.component.ts`, `files-uploader.component.{ts,html}` and `preset-wrapper.component.{ts,html}` each report no unhit line and no partial branch. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../preset-wrapper.component.spec.ts | 149 ++++++++++++++++++--- .../files-uploader.component.spec.ts | 105 ++++++++++++++- .../user/user-venv/user-venv.component.spec.ts | 51 +++++++ .../coeditor-user-icon.component.spec.ts | 68 ++++++++++ 4 files changed, 354 insertions(+), 19 deletions(-) diff --git a/frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts b/frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts index c074ce1a90..29d887f282 100644 --- a/frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts +++ b/frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts @@ -18,6 +18,8 @@ */ import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { NzDropdownDirective, NzDropdownMenuComponent } from "ng-zorro-antd/dropdown"; import { FormControl } from "@angular/forms"; import { FormlyFieldConfig } from "@ngx-formly/core"; import { NzMessageService } from "ng-zorro-antd/message"; @@ -135,6 +137,43 @@ describe("PresetWrapperComponent", () => { expect(presetServiceStub.getPresets).toHaveBeenCalledWith(presetKey.presetType, presetKey.saveTarget); expect(component.searchResults).toEqual([testPreset, otherPreset]); }); + + it("seeds the search term from the form control's value", () => { + formControl.setValue("seeded"); + component.field = buildField(); + + component.ngOnInit(); + + expect(component["searchTerm"]).toBe("seeded"); + }); + + it("seeds an empty search term when the form control holds null", () => { + formControl.setValue(null); + component.field = buildField(); + + component.ngOnInit(); + + expect(component["searchTerm"]).toBe(""); + }); + }); + + describe("setupFieldConfig", () => { + it("merges the preset wrappers and the preset key into the given config", () => { + const config: FormlyFieldConfig = { key: fieldKey, templateOptions: { label: "kept" } }; + + PresetWrapperComponent.setupFieldConfig(config, "operator", "MySQLSource", "MySQLSource-op-1"); + + expect(config.wrappers).toEqual(["form-field", "preset-wrapper"]); + expect(config.templateOptions?.presetKey).toEqual({ + presetType: "operator", + saveTarget: "MySQLSource", + applyTarget: "MySQLSource-op-1", + }); + // the browser's own autocomplete is turned off so it cannot cover the preset menu + expect(config.templateOptions?.attributes).toEqual({ autocomplete: "off" }); + // pre-existing options survive the merge + expect(config.templateOptions?.label).toBe("kept"); + }); }); describe("functional api", () => { @@ -268,13 +307,17 @@ describe("PresetWrapperComponent", () => { expect(component.searchResults).toEqual([]); }); - it("does not refresh searchResults from form value changes while the dropdown is closed", () => { + it("does not refresh searchResults from form value changes while the dropdown is closed", async () => { const baselineCalls = presetServiceStub.getPresets.mock.calls.length; component.presetMenuVisible = false; formControl.setValue("typing"); + // the handler is debounced(0); without this tick it would not have run at all and + // the assertion below would hold no matter what the menu state is + await new Promise(resolve => setTimeout(resolve, 0)); - // No additional getPresets call because the menu is closed. + // the term is still tracked, but the menu being closed suppresses the refetch + expect(component["searchTerm"]).toBe("typing"); expect(presetServiceStub.getPresets.mock.calls.length).toBe(baselineCalls); }); @@ -290,6 +333,44 @@ describe("PresetWrapperComponent", () => { expect(presetServiceStub.getPresets.mock.calls.length).toBe(baselineCalls + 1); }); + it("adopts the preset carried by a matching applyPresetStream event", () => { + presetServiceStub.applyPresetStream.next({ + type: presetKey.presetType, + target: presetKey.applyTarget, + preset: testPreset, + }); + + expect(component["basePreset"]).toBe(testPreset); + }); + + it("ignores applyPresetStream events for a different presetType or applyTarget", () => { + const before = component["basePreset"]; + + presetServiceStub.applyPresetStream.next({ + type: "someOtherType", + target: presetKey.applyTarget, + preset: testPreset, + }); + presetServiceStub.applyPresetStream.next({ + type: presetKey.presetType, + target: "someOtherTarget", + preset: otherPreset, + }); + + expect(component["basePreset"]).toBe(before); + }); + + it("clears the search term when the form value becomes null while the dropdown is open", async () => { + component.presetMenuVisible = true; + component["searchTerm"] = "typed"; + + formControl.setValue(null); + // the valueChanges handler is debounced(0) — wait one tick + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(component["searchTerm"]).toBe(""); + }); + it("stops responding to stream events after ngOnDestroy", () => { component.searchResults = []; component.ngOnDestroy(); @@ -360,6 +441,20 @@ describe("PresetWrapperComponent", () => { fixture.detectChanges(); }; + it("routes the dropdown's own visibility event into onDropdownVisibilityEvent", () => { + // the tests above call the handler directly; this drives it through the template + // binding, which is the wiring that would break if the output were renamed + initWith([testPreset]); + const handler = vi.spyOn(component, "onDropdownVisibilityEvent"); + + fixture.debugElement + .query(By.directive(NzDropdownDirective)) + .injector.get(NzDropdownDirective) + .nzVisibleChange.emit(true); + + expect(handler).toHaveBeenCalledWith(true); + }); + it("renders the save button and saves the preset when it is clicked", () => { initWith([]); @@ -372,39 +467,57 @@ describe("PresetWrapperComponent", () => { expect(savePreset).toHaveBeenCalled(); }); - it("feeds the dropdown *ngFor with one entry per preset, titled and described", () => { - // The rows live in an nz-dropdown-menu that only mounts into a CDK overlay on a - // real user open, which jsdom does not drive; assert the list the *ngFor is bound - // to and the interpolations it renders for each row instead. + /** + * The rows live in an nz-dropdown-menu, whose content is an ng-template that only + * mounts into a CDK overlay when the dropdown opens — jsdom never drives that. + * Instantiating the template directly puts the rows in the fixture's DOM so the + * *ngFor, the interpolations and the row click handlers all really run. + */ + const renderDropdownRows = (): HTMLElement[] => { + const menu = fixture.debugElement.query(By.directive(NzDropdownMenuComponent)) + .componentInstance as NzDropdownMenuComponent; + menu.viewContainerRef.createEmbeddedView(menu.templateRef); + fixture.detectChanges(); + return Array.from(fixture.nativeElement.querySelectorAll(".preset-dropdown-item")); + }; + + it("renders one dropdown row per preset, titled and described", () => { initWith([testPreset, otherPreset]); - expect(component.searchResults).toEqual([testPreset, otherPreset]); - // the title cell renders the preset's value under the field's own key, and the - // description cell joins the remaining values - expect(component.getEntryTitle(testPreset)).toBe(testPreset[fieldKey]); - expect(component.getEntryDescription(testPreset)).toBe("otherPresetValue"); + const rows = renderDropdownRows(); + + expect(rows).toHaveLength(2); + expect(rows[0].querySelector(".title")?.textContent).toBe(testPreset[fieldKey]); + expect(rows[0].querySelector(".description")?.textContent).toBe("otherPresetValue"); + expect(rows[1].querySelector(".title")?.textContent).toBe(otherPreset[fieldKey]); }); - it("binds an empty dropdown list when there are no presets", () => { + it("renders no dropdown rows when there are no presets", () => { initWith([]); - expect(component.searchResults).toEqual([]); + expect(renderDropdownRows()).toHaveLength(0); }); - it("applies the preset the row's (click) binding targets", () => { + it("applies the preset when its row is clicked", () => { initWith([testPreset]); - component.applyPreset(testPreset); + renderDropdownRows()[0].querySelector<HTMLElement>(".dropdown-entry")!.click(); - expect(presetServiceStub.applyPreset).toHaveBeenCalledWith(expect.anything(), expect.anything(), testPreset); + expect(presetServiceStub.applyPreset).toHaveBeenCalledWith( + presetKey.presetType, + presetKey.applyTarget, + testPreset + ); }); - it("deletes the preset the delete button's (click) binding targets", () => { + it("deletes the preset from its row's delete button without applying it", () => { initWith([testPreset]); - component.deletePreset(testPreset); + renderDropdownRows()[0].querySelector<HTMLElement>(".delete-button")!.click(); expect(presetServiceStub.deletePreset).toHaveBeenCalled(); + // the button stops propagation so the surrounding row does not also apply it + expect(presetServiceStub.applyPreset).not.toHaveBeenCalled(); }); }); }); diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts index e23668e997..f0f9eeed19 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.spec.ts @@ -19,8 +19,13 @@ import { of, Subject, throwError } from "rxjs"; import { OnDestroy } from "@angular/core"; -import { NgxFileDropEntry } from "ngx-file-drop"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { NoopAnimationsModule } from "@angular/platform-browser/animations"; +import { NgxFileDropComponent, NgxFileDropEntry } from "ngx-file-drop"; +import { NzAlertComponent } from "ng-zorro-antd/alert"; import { NzModalService } from "ng-zorro-antd/modal"; +import { commonTestProviders } from "../../../../common/testing/test-utils"; import { AdminSettingsService } from "../../../service/admin/settings/admin-settings.service"; import { DatasetService } from "../../../service/user/dataset/dataset.service"; import { NotificationService } from "../../../../common/service/notification/notification.service"; @@ -502,3 +507,101 @@ describe("FilesUploaderComponent", () => { }); }); }); + +/** + * The suite above constructs the component directly, so its template has never been + * rendered — the banner's `*ngIf`, the banner bindings and the drop-zone button live + * only in the template. These mount it for real. + */ +describe("FilesUploaderComponent rendered", () => { + let fixture: ComponentFixture<FilesUploaderComponent>; + let component: FilesUploaderComponent; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [FilesUploaderComponent, NoopAnimationsModule], + providers: [ + { provide: NotificationService, useValue: { error: vi.fn() } }, + { provide: AdminSettingsService, useValue: { getPublicSetting: vi.fn().mockReturnValue(of("20")) } }, + { + provide: DatasetService, + useValue: { + listMultipartUploads: vi.fn().mockReturnValue(of([])), + findExistingUploadFiles: vi.fn().mockReturnValue(of([])), + }, + }, + { provide: NzModalService, useValue: { create: vi.fn() } }, + ...commonTestProviders, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(FilesUploaderComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + const alert = (): HTMLElement | null => (fixture.nativeElement as HTMLElement).querySelector("nz-alert"); + + it("hides the banner until the alert is enabled and the upload has finished", () => { + expect(alert()).toBeNull(); + + component.showUploadAlert = true; + fixture.detectChanges(); + expect(alert()).toBeNull(); + + component.showUploadAlert = false; + component.fileUploadingFinished = true; + fixture.detectChanges(); + expect(alert()).toBeNull(); + }); + + it("renders the banner message once both flags are set", () => { + component.showUploadAlert = true; + component.fileUploadingFinished = true; + component.fileUploadBannerType = "error"; + component.fileUploadBannerMessage = "Upload failed. Please retry."; + fixture.detectChanges(); + + const banner = alert(); + expect(banner).not.toBeNull(); + expect(banner!.textContent).toContain("Upload failed. Please retry."); + }); + + it("clears the banner when its close control fires", () => { + component.showUploadAlert = true; + component.fileUploadingFinished = true; + component.fileUploadBannerMessage = "done"; + fixture.detectChanges(); + + fixture.debugElement.query(By.directive(NzAlertComponent)).componentInstance.nzOnClose.emit(); + fixture.detectChanges(); + + expect(component.fileUploadingFinished).toBe(false); + expect(alert()).toBeNull(); + }); + + it("opens the file selector from the drop-zone button", () => { + // ngx-file-drop hands its `openFileSelector` to the content template by reference, + // so spying on the component's property after render would not be seen. Assert its + // effect instead: it clicks the hidden file input. + const host = fixture.nativeElement as HTMLElement; + const fileInput: HTMLInputElement = host.querySelector("input.ngx-file-drop__file-input")!; + expect(fileInput).not.toBeNull(); + const openDialog = vi.spyOn(fileInput, "click").mockImplementation(() => {}); + + const button: HTMLButtonElement = host.querySelector(".upload-file-button")!; + expect(button).not.toBeNull(); + button.click(); + + expect(openDialog).toHaveBeenCalled(); + }); + + it("routes a drop on the zone into fileDropped", () => { + const dropped = vi.spyOn(component, "fileDropped").mockImplementation(() => {}); + const entries = [droppedFile("a.csv", new File(["a"], "a.csv"))]; + + fixture.debugElement.query(By.directive(NgxFileDropComponent)).componentInstance.onFileDrop.emit(entries); + + expect(dropped).toHaveBeenCalledWith(entries); + }); +}); diff --git a/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts b/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts index cdc5ed6605..dde83e2726 100644 --- a/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-venv/user-venv.component.spec.ts @@ -147,6 +147,26 @@ describe("UserVenvComponent", () => { expect(notificationSpy.error).toHaveBeenCalledWith("Failed to fetch Python environments."); expect(component.pves).toEqual([]); }); + + it("treats a record with no packages as an empty package list", () => { + pveServiceSpy.listUserPves.mockReturnValue(of([{ veid: 1, name: "envA" } as UserPveRecord])); + + fixture.detectChanges(); + + expect(component.pves[0].newPackages).toEqual([]); + }); + + it("falls back to an empty version when the stored value is nullish", () => { + // distinct from the `empty: ""` case above: "" is not nullish, so only a null + // value reaches the `?? ""` arm + pveServiceSpy.listUserPves.mockReturnValue( + of([{ veid: 1, name: "envA", packages: { ghost: null } } as unknown as UserPveRecord]) + ); + + fixture.detectChanges(); + + expect(component.pves[0].newPackages).toEqual([{ name: "ghost", versionOp: "==", version: "" }]); + }); }); describe("modal open/close and package editing", () => { @@ -284,6 +304,19 @@ describe("UserVenvComponent", () => { expect(pveServiceSpy.listUserPves).toHaveBeenCalledTimes(1); // refresh after save }); + it("treats a row whose version is nullish as an empty version", () => { + component.currentDraft = { + name: "envNull", + newPackages: [{ name: "a", versionOp: ">=", version: null as unknown as string }], + }; + pveServiceSpy.savePve.mockReturnValue(of({ veid: 6 })); + pveServiceSpy.listUserPves.mockReturnValue(of([])); + + component.saveEnvironment(); + + expect(pveServiceSpy.savePve).toHaveBeenCalledWith("envNull", { a: "" }); + }); + it("updates an existing environment when the draft carries a veid", () => { component.pves = [{ veid: 7, name: "envU", newPackages: [] }]; component.currentDraft = { @@ -351,6 +384,14 @@ describe("UserVenvComponent", () => { component.confirmDeletePve(5); expect(confirmSpy).not.toHaveBeenCalled(); }); + + it("names an environment with a blank name as (unnamed) in the confirm title", () => { + component.pves = [{ veid: 3, name: "", newPackages: [] }]; + + component.confirmDeletePve(0); + + expect(capturedConfirmConfig?.nzTitle).toBe('Delete environment "(unnamed)"?'); + }); }); describe("deletePve", () => { @@ -387,6 +428,16 @@ describe("UserVenvComponent", () => { expect(consoleErrorSpy).toHaveBeenCalled(); expect(notificationSpy.error).toHaveBeenCalledWith("Failed to delete Python environment."); }); + + it("reports a blank-named environment as (unnamed) on success", () => { + component.pves = [{ veid: 9, name: "", newPackages: [] }]; + pveServiceSpy.deleteUserPve.mockReturnValue(of(undefined)); + pveServiceSpy.listUserPves.mockReturnValue(of([])); + + component.deletePve(0); + + expect(notificationSpy.success).toHaveBeenCalledWith('Deleted environment "(unnamed)".'); + }); }); describe("trackByVeid", () => { diff --git a/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.spec.ts b/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.spec.ts index 0f3f07170d..b52d6b9670 100644 --- a/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.spec.ts @@ -18,6 +18,7 @@ */ import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; import { CoeditorUserIconComponent } from "./coeditor-user-icon.component"; import { CoeditorPresenceService } from "../../../service/workflow-graph/model/coeditor-presence.service"; @@ -53,7 +54,74 @@ describe("CoeditorUserIconComponent", () => { fixture.detectChanges(); }); + /** + * The menu items live inside `<nz-dropdown-menu>`, whose content is an ng-template + * that only mounts into a CDK overlay when the dropdown opens — jsdom never drives + * that. Instantiating the template directly puts the items in the fixture's DOM, so + * both variants can be asserted and clicked without an overlay. + */ + function renderDropdownMenu(): HTMLElement[] { + const menu = fixture.debugElement.query(By.directive(NzDropdownMenuComponent)) + .componentInstance as NzDropdownMenuComponent; + menu.viewContainerRef.createEmbeddedView(menu.templateRef); + fixture.detectChanges(); + return Array.from(fixture.nativeElement.querySelectorAll("li[nz-menu-item]")); + } + it("should create", () => { expect(component).toBeTruthy(); }); + + it("offers to start shadowing while shadowing mode is off", () => { + component.coeditor = { ...component.coeditor, name: "alice", clientId: "c1" }; + const items = renderDropdownMenu(); + + expect(items).toHaveLength(1); + expect(items[0].textContent).toContain('Start "shadowing":'); + expect(items[0].textContent).toContain("alice"); + expect(items[0].textContent).toContain("c1"); + }); + + it("still offers to start shadowing while another co-editor is being shadowed", () => { + // second half of the guard false: shadowing is on, but for a different client + component.coeditor = { ...component.coeditor, clientId: "c1" }; + coeditorPresenceService.shadowingModeEnabled = true; + coeditorPresenceService.shadowingCoeditor = { ...component.coeditor, clientId: "c2" }; + + const items = renderDropdownMenu(); + + expect(items).toHaveLength(1); + expect(items[0].textContent).toContain('Start "shadowing":'); + }); + + it("offers to stop shadowing while this co-editor is the one being shadowed", () => { + component.coeditor = { ...component.coeditor, clientId: "c1" }; + coeditorPresenceService.shadowingModeEnabled = true; + coeditorPresenceService.shadowingCoeditor = component.coeditor; + + const items = renderDropdownMenu(); + + expect(items).toHaveLength(1); + expect(items[0].textContent).toContain("Stop Shadowing"); + }); + + it("shadows the co-editor when the start item is clicked", () => { + component.coeditor = { ...component.coeditor, clientId: "c1" }; + const shadow = vi.spyOn(coeditorPresenceService, "shadowCoeditor"); + + renderDropdownMenu()[0].click(); + + expect(shadow).toHaveBeenCalledWith(component.coeditor); + }); + + it("stops shadowing when the stop item is clicked", () => { + component.coeditor = { ...component.coeditor, clientId: "c1" }; + coeditorPresenceService.shadowingModeEnabled = true; + coeditorPresenceService.shadowingCoeditor = component.coeditor; + const stop = vi.spyOn(coeditorPresenceService, "stopShadowing"); + + renderDropdownMenu()[0].click(); + + expect(stop).toHaveBeenCalled(); + }); });
