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-8178-c9b75c048cb1af200c53e27fb9c8f428fdb01d8e in repository https://gitbox.apache.org/repos/asf/texera.git
commit d2eb275da60949ee249b12ff8d451a340f593887 Author: Xinyuan Lin <[email protected]> AuthorDate: Mon Aug 31 19:16:10 2026 +0000 test(frontend): close eight small service and component gaps (#8178) ### What changes were proposed in this PR? Eight small frontend gaps, each too small to be worth a PR alone. **+9 fully-covered lines and +4 branch arms.** | File | Codecov | Closed | |---|---|---| | `dashboard-entry.ts` | 117/119 → **119/119** | 205, 206 | | `settings.component.ts` | 27/28 → **28/28** | 78 | | `codearea-custom-template.component.ts` | 30/31 → **31/31** | 72 | | `result-panel-modal.component.html` | 23/24 → **24/24** | 56 | | `breakpoint-condition-input.component.html` | 8/9 → **9/9** | 27 (arms 0/2 → **2/2**) | | `hub-workflow-detail.component.html` | 45/46 → **46/46** | 27 | | `search.component.ts` | 54/56 → 55/56 | 119 | | `ui-udf-parameters-parser.service.ts` | 135/142 → 136/142 | 364 | Six of the eight reach 100%. ### Verification Measured against the **full** suite with no `--include` filter at all — 203 files / 5227 tests before, 203 / 5242 after, both green — because the `DA` line set for a file depends on which specs are in the run, so a filtered before/after pair can fabricate a delta. Parsed from `lcov.info` with Codecov's rule applied directly: a line counts only if its `DA` hit is non-zero **and** every `BRDA` arm on it is taken. Two reviewers returned ten findings; all repaired. **The repair pass moved coverage by exactly zero** — its three added tests are mutation-strength, not coverage. The builder's figures reproduced line for line. ### Two equivalent mutants, reported rather than papered over - `ui-udf-parameters-parser.service.ts:364` — `return parts.length ? parts : undefined` → `return parts;` survives the full suite. Both callers of `readMemberPath` treat `[]` and `undefined` identically, so it is not killable. No test was written to pretend otherwise. - `:349` — relaxing `parts?.length !== 2` to `parts.length < 2` survives for the same reason. Seven of the sixteen baseline gap lines across these files are given up rather than chased. Two files touched only by repaired specs are **not** claimed as gains: `result-panel-modal.component.ts` (46/47) and `hub-workflow-detail.component.ts` (98/99) are unchanged. `yarn format:ci` passes. `frontend/junit.xml` is regenerated by every run, is not gitignored, and is not committed. No production file is touched. ### Any related issues, documentation, discussions? Closes #8176 ### How was this PR tested? ``` npx ng test --watch=false --include="**/ui-udf-parameters-parser.service.spec.ts" --include="**/dashboard-entry.spec.ts" --include="**/search.component.spec.ts" --include="**/settings.component.spec.ts" --include="**/codearea-custom-template.component.spec.ts" --include="**/result-panel-modal.component.spec.ts" --include="**/breakpoint-condition-input.component.spec.ts" --include="**/hub-workflow-detail.component.spec.ts" ``` ``` Test Files 8 passed (8) ``` Re-run after rebasing onto current `main`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../component/user/search/search.component.spec.ts | 58 +++++++++- .../src/app/dashboard/type/dashboard-entry.spec.ts | 55 +++++++++ .../detail/hub-workflow-detail.component.spec.ts | 21 +++- .../breakpoint-condition-input.component.spec.ts | 31 +++++ .../codearea-custom-template.component.spec.ts | 126 +++++++++++++++++++-- .../left-panel/settings/settings.component.spec.ts | 20 ++++ .../result-panel-modal.component.spec.ts | 21 ++++ .../ui-udf-parameters-parser.service.spec.ts | 24 ++++ 8 files changed, 343 insertions(+), 13 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/search/search.component.spec.ts b/frontend/src/app/dashboard/component/user/search/search.component.spec.ts index 381ed6f52a..3eafea2569 100644 --- a/frontend/src/app/dashboard/component/user/search/search.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/search/search.component.spec.ts @@ -73,10 +73,10 @@ class MockSearchResultsComponent { // A plain filters double for the unit tests that drive component methods // directly (no rendering / no ViewChild). -function makeFiltersDouble(keywords: string[] = []) { +function makeFiltersDouble(keywords: string[] = [], masterFilterList: ReadonlyArray<string> = []) { return { masterFilterListChange: EMPTY, - masterFilterList: [] as ReadonlyArray<string>, + masterFilterList, getSearchKeywords: () => keywords, getSearchFilterParameters: () => ({}), } as unknown as FiltersComponent; @@ -216,6 +216,60 @@ describe("SearchComponent", () => { expect(results.reset).not.toHaveBeenCalled(); }); + // The duplicate-search guard above compares list *lengths* and then the terms themselves. With + // two empty lists the term comparison never runs at all, so these three drive it with non-empty + // lists: equal terms still short-circuit, differing terms do not, and a shorter incoming list + // does not sneak past `Array.every` (which is vacuously true on a shorter receiver). + // + // Every case below deliberately gives the double a *different* keyword list from its filter list. + // The guard reads `filters.masterFilterList`, and `filters.getSearchKeywords()` is right next to + // it in the same object; with the two set to equal arrays the receiver could be swapped for the + // keywords and nothing would notice. + it("skips a duplicate search when a non-empty filter list is unchanged", async () => { + component.filters = makeFiltersDouble(["kw"], ["alpha"]); + const results = makeSearchResultsDouble(); + component.searchResultsComponent = results as unknown as SearchResultsComponent; + component.masterFilterList = ["alpha"]; + component.lastSortMethod = component.sortMethod; + component.lastSelectedType = component.selectedType; + + await component.search(); + + expect(results.reset).not.toHaveBeenCalled(); + }); + + it("re-runs the search when a same-length filter list holds different terms", async () => { + component.filters = makeFiltersDouble(["kw"], ["alpha"]); + const results = makeSearchResultsDouble(); + component.searchResultsComponent = results as unknown as SearchResultsComponent; + component.masterFilterList = ["beta"]; + component.lastSortMethod = component.sortMethod; + component.lastSelectedType = component.selectedType; + + await component.search(); + + expect(results.reset).toHaveBeenCalledTimes(1); + expect(component.masterFilterList).toEqual(["alpha"]); + }); + + it("re-runs the search when the new filter list is a prefix of the last one", async () => { + // The element-wise comparison alone cannot see this: `["alpha"].every((v, i) => v === ["alpha", + // "beta"][i])` is true, so without the length conjunct every narrowing of the filter box - + // including clearing it - would be dismissed as "same list" and the panel would keep showing + // the previous query's results. + component.filters = makeFiltersDouble(["kw"], ["alpha"]); + const results = makeSearchResultsDouble(); + component.searchResultsComponent = results as unknown as SearchResultsComponent; + component.masterFilterList = ["alpha", "beta"]; + component.lastSortMethod = component.sortMethod; + component.lastSelectedType = component.selectedType; + + await component.search(); + + expect(results.reset).toHaveBeenCalledTimes(1); + expect(component.masterFilterList).toEqual(["alpha"]); + }); + it("throws when the results component is missing", async () => { component.filters = makeFiltersDouble(["x"]); component.searchResultsComponent = undefined; diff --git a/frontend/src/app/dashboard/type/dashboard-entry.spec.ts b/frontend/src/app/dashboard/type/dashboard-entry.spec.ts index 9fa2c62595..76ad406673 100644 --- a/frontend/src/app/dashboard/type/dashboard-entry.spec.ts +++ b/frontend/src/app/dashboard/type/dashboard-entry.spec.ts @@ -22,6 +22,7 @@ import { EntityType } from "../../hub/service/hub.service"; import { DashboardWorkflow } from "./dashboard-workflow.interface"; import { DashboardFile } from "./dashboard-file.interface"; import { DashboardDataset } from "./dashboard-dataset.interface"; +import { DashboardModel } from "./dashboard-model.interface"; import { DashboardWorkflowComputingUnit } from "../../common/type/workflow-computing-unit"; import { ExecutionMode } from "../../common/type/workflow"; @@ -92,6 +93,28 @@ function makeDataset(): DashboardDataset { }; } +function makeModel(): DashboardModel { + return { + isOwner: false, + ownerEmail: "[email protected]", + model: { + mid: 606, + ownerUid: 60, + name: "My Model", + repositoryName: "my-model", + isPublic: true, + isDownloadable: false, + description: "A sample model", + creationTime: 1700000006000, + coverImage: "http://example.com/model-cover.png";, + framework: "pytorch", + format: "safetensors", + }, + accessPrivilege: "WRITE", + size: 8765, + }; +} + function makeComputingUnit(): DashboardWorkflowComputingUnit { return { computingUnit: { @@ -181,6 +204,32 @@ describe("DashboardEntry", () => { expect(entry.coverImageUrl).toBe("http://example.com/dataset-cover.png";); }); + it("maps a DashboardModel to the Model entity and copies model fields", () => { + const entry = new DashboardEntry(makeModel()); + + expect(entry.type).toBe(EntityType.Model); + expect(entry.id).toBe(606); + expect(entry.name).toBe("My Model"); + expect(entry.description).toBe("A sample model"); + expect(entry.creationTime).toBe(1700000006000); + expect(entry.lastModifiedTime).toBe(1700000006000); + expect(entry.accessLevel).toBe("WRITE"); + expect(entry.ownerEmail).toBe("[email protected]"); + expect(entry.ownerId).toBe(60); + expect(entry.size).toBe(8765); + expect(entry.coverImageUrl).toBe("http://example.com/model-cover.png";); + // The model branch has no hub-side counters to copy, so it hard-codes placeholders. They are + // read straight by the card templates, so leaving them unasserted would let any of the seven + // be changed to a fabricated value with nothing failing. + expect(entry.ownerName).toBe(""); + expect(entry.ownerAvatar).toBe(""); + expect(entry.viewCount).toBe(0); + expect(entry.cloneCount).toBe(0); + expect(entry.likeCount).toBe(0); + expect(entry.isLiked).toBe(false); + expect(entry.accessibleUserIds).toEqual([]); + }); + it("maps a DashboardWorkflowComputingUnit to the ComputingUnit entity and copies computing-unit fields", () => { const entry = new DashboardEntry(makeComputingUnit()); @@ -273,6 +322,12 @@ describe("DashboardEntry", () => { expect(() => new DashboardEntry(makeWorkflow()).dataset).toThrowError("Value is not of type DashboardDataset"); }); + it("model getter returns the value for a model entry and throws for others", () => { + const modelValue = makeModel(); + expect(new DashboardEntry(modelValue).model).toBe(modelValue); + expect(() => new DashboardEntry(makeWorkflow()).model).toThrowError("Value is not of type DashboardModel"); + }); + it("computingUnit getter returns the value for a computing-unit entry and throws for others", () => { const computingUnitValue = makeComputingUnit(); expect(new DashboardEntry(computingUnitValue).computingUnit).toBe(computingUnitValue); 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 31c3c8e0a4..25e91bbc82 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 @@ -550,8 +550,11 @@ describe("HubWorkflowDetailComponent", () => { */ describe("HubWorkflowDetailComponent rendered with its real children", () => { let fixture: ComponentFixture<HubWorkflowDetailComponent>; + // goBack() chains .catch() onto the navigation result, so this has to be a real promise. + let renderedRouter: { navigateByUrl: ReturnType<typeof vi.fn>; navigate: ReturnType<typeof vi.fn> }; function render(opts: { isHub: boolean }): void { + renderedRouter = { navigateByUrl: vi.fn().mockResolvedValue(true), navigate: vi.fn().mockResolvedValue(true) }; TestBed.resetTestingModule(); TestBed.configureTestingModule({ imports: [ @@ -572,7 +575,7 @@ describe("HubWorkflowDetailComponent rendered with its real children", () => { provide: ActivatedRoute, useValue: { snapshot: { params: opts.isHub ? { id: "5" } : {} } }, }, - { provide: Router, useValue: { navigateByUrl: vi.fn(), navigate: vi.fn() } }, + { provide: Router, useValue: renderedRouter }, { provide: HubService, useValue: { @@ -621,6 +624,22 @@ describe("HubWorkflowDetailComponent rendered with its real children", () => { expect((fixture.nativeElement as HTMLElement).querySelector(".go-back-button")).not.toBeNull(); }); + it("navigates back to the hub listing when the back button is clicked", () => { + // The goBack() unit test above calls the method directly, so nothing pinned the button's + // (click) binding: drop it from the template and that test still passes while the arrow + // becomes inert. + render({ isHub: true }); + + (fixture.nativeElement as HTMLElement).querySelector<HTMLButtonElement>(".go-back-button")!.click(); + + // Asserted as a literal, not as HUB_WORKFLOW_RESULT: the component navigates with that same + // symbol, so a symbolic assertion moves with it and cannot see the destination change. The + // route table in app-routing.module.ts spells the segments out as literals and does not import + // the constant, so the two really can drift apart into a navigation to a dead route. + expect(renderedRouter.navigateByUrl).toHaveBeenCalledWith("/hub/workflow/result"); + expect(HUB_WORKFLOW_RESULT).toBe("/hub/workflow/result"); + }); + it("hides the back button when the wid arrived as modal data", () => { // The converse. Without it the `*ngIf` could be replaced by a constant and the positive case // above would still pass. diff --git a/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts index 18f16f05a4..b96eac284d 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/breakpoint-condition-input/breakpoint-condition-input.component.spec.ts @@ -178,6 +178,37 @@ describe("BreakpointConditionInputComponent", () => { }); }); + describe("the condition textarea", () => { + const textarea = (): HTMLTextAreaElement => fixture.nativeElement.querySelector("textarea.condition-textarea"); + + it("carries the current condition into the textarea", async () => { + component.condition = "x > 1"; + fixture.detectChanges(); + await fixture.whenStable(); + + expect(textarea().value).toBe("x > 1"); + }); + + it("saves what the user types, not what the class was holding", async () => { + // Every other test in this file writes `condition` from the class side, so the *inbound* + // half of the two-way binding is all that is pinned: downgrade the template to a one-way + // [ngModel] and the whole suite stays green while the popup silently discards every + // keystroke. Drive a real input event and then let the save path read it back. + component.condition = "stale"; + fixture.detectChanges(); + await fixture.whenStable(); + + textarea().value = " y != 2 "; + textarea().dispatchEvent(new Event("input")); + + expect(component.condition).toBe(" y != 2 "); + + component.handleEvent(new KeyboardEvent("keydown", { key: "Enter" })); + + expect(mockUdfDebugService.doUpdateBreakpointCondition).toHaveBeenCalledWith("test-operator", 1, "y != 2"); + }); + }); + describe("visibility", () => { it("is visible only while a line is targeted", () => { // The template keys its *ngIf on this, so an inverted getter leaves the popup stuck open. diff --git a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts index 6b1a81cc21..7bf63d610e 100644 --- a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts +++ b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts @@ -27,7 +27,7 @@ import { FormControl } from "@angular/forms"; import { commonTestProviders } from "../../../common/testing/test-utils"; import { CodeEditorService } from "../../service/code-editor/code-editor.service"; import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; -import { Subject } from "rxjs"; +import { config as rxjsConfig, Subject, take } from "rxjs"; describe("CodeareaCustomTemplateComponent", () => { let component: CodeareaCustomTemplateComponent; @@ -78,6 +78,21 @@ describe("CodeareaCustomTemplateComponent", () => { return TestBed.inject(WorkflowActionService).getJointGraphWrapper().getCurrentHighlightedOperatorIDs()[0]; } + // The shared flag as the service currently publishes it for one operator. `getEditorState` + // hands back a BehaviorSubject as an observable, so the current value arrives synchronously on + // subscribe; `take(1)` consumes that one value and completes, instead of leaving a live + // subscription on a service-owned subject behind for the rest of the test. Returns + // `boolean | undefined` on purpose: a service that stopped replaying a current value would + // yield `undefined` here and fail the caller's assertion rather than quietly reading stale. + function publishedEditorState(operatorId: string): boolean | undefined { + let published: boolean | undefined; + codeEditorService + .getEditorState(operatorId) + .pipe(take(1)) + .subscribe(v => (published = v)); + return published; + } + beforeEach(() => { const workflowActionService = TestBed.inject(WorkflowActionService); vi.spyOn(workflowActionService.getJointGraphWrapper(), "getCurrentHighlightedOperatorIDs").mockReturnValue([ @@ -107,9 +122,7 @@ describe("CodeareaCustomTemplateComponent", () => { // detached from the operator property it is meant to edit. expect(component.componentRef!.instance.formControl).toBe(component.field.formControl); - let published: boolean | undefined; - codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); - expect(published).toBe(true); + expect(publishedEditorState(highlightedOperatorId())).toBe(true); }); it("clears the flag again when the editor component is destroyed", () => { @@ -117,9 +130,7 @@ describe("CodeareaCustomTemplateComponent", () => { component.componentRef!.destroy(); expect(component.isEditorOpen).toBe(false); - let published: boolean | undefined; - codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); - expect(published).toBe(false); + expect(publishedEditorState(highlightedOperatorId())).toBe(false); }); it("opens the editor when a co-editor opens one", () => { @@ -147,6 +158,103 @@ describe("CodeareaCustomTemplateComponent", () => { expect(remoteComponent.componentRef).toBeDefined(); }); + it("tears this client's editor down when any co-editor closes one, whichever operator it names", () => { + // The mirror image of the open case above, and the half that was missing: a co-editor + // closing the dialog has to close it here too, or this client keeps typing into an editor + // the other side has already dismissed. + const closed = new Subject<{ operatorId: string }>(); + vi.spyOn(TestBed.inject(CoeditorPresenceService), "getCoeditorClosedCodeEditorSubject").mockReturnValue( + closed.asObservable() + ); + + const remoteFixture = TestBed.createComponent(CodeareaCustomTemplateComponent); + const remoteComponent = remoteFixture.componentInstance; + remoteComponent.field = { props: {}, formControl: new FormControl() } as any; + remoteFixture.detectChanges(); + remoteComponent.openEditor(); + expect(remoteComponent.isEditorOpen).toBe(true); + + // The subscriber binds the payload to `_` and discards it, so a deliberately foreign operator + // id is the honest fixture: naming this panel's own operator would advertise a targeting + // filter the component does not have, and would leave a later filter free to be added without + // any test noticing. + const destroySpy = vi.spyOn(remoteComponent.componentRef!, "destroy"); + closed.next({ operatorId: "a-completely-unrelated-operator" }); + + // The spy is what proves teardown. The shared flag below is reachable *without* any teardown: + // a subscriber that merely published `false` for this operator would satisfy it through + // ngOnInit's getEditorState subscription while the dialog stayed on screen. vi.spyOn calls + // through, so the real destroy still runs and the flag assertions still describe the result. + expect(destroySpy).toHaveBeenCalledTimes(1); + expect(remoteComponent.isEditorOpen).toBe(false); + expect(publishedEditorState(highlightedOperatorId())).toBe(false); + }); + + it("stays quiet when a co-editor closes an editor this panel never opened", async () => { + // componentRef is still undefined here; the optional call is what keeps a close broadcast + // from throwing in every panel that happens to be mounted but closed. A close broadcast + // reaches EVERY mounted codearea, so most recipients are in exactly this state. + const closed = new Subject<{ operatorId: string }>(); + vi.spyOn(TestBed.inject(CoeditorPresenceService), "getCoeditorClosedCodeEditorSubject").mockReturnValue( + closed.asObservable() + ); + + const remoteFixture = TestBed.createComponent(CodeareaCustomTemplateComponent); + const remoteComponent = remoteFixture.componentInstance; + remoteComponent.field = { props: {}, formControl: new FormControl() } as any; + remoteFixture.detectChanges(); + + // A throw inside a subscriber does NOT propagate out of Subject.next() - RxJS swallows it and + // reports it on its unhandled-error channel one macrotask later. So `expect(...).not.toThrow()` + // would pass even with the optional call removed; watch that channel instead. + const unhandled = vi.fn(); + const previousHandler = rxjsConfig.onUnhandledError; + // Two restores, for two different exits. The `finally` below is the normal one, and it puts + // the handler back before the assertions so anything RxJS reports afterwards is reported, + // not swallowed. It is skipped, though, if the awaited flush never resolves and Vitest kills + // this test on its 20s timeout, and a handler left installed silently eats every later + // unhandled error. That blast radius is not limited to this file: the Angular unit-test + // builder defaults `isolate: false`, so one forked worker keeps a single module registry + // across all the spec FILES it runs, and this `rxjs` `config` is therefore the same object + // every one of them sees. (Measured with three probe specs pinned to one worker: same pid, + // and each file read the mutation the previous file had left behind.) `onTestFinished` runs + // however the test ends - verified against a never-resolving await, where a `finally` + // provably cannot - so it closes that exit; re-assigning the same value after the `finally` + // already ran is a no-op. Deliberately not hoisted into beforeEach/afterEach: that would + // install this swallowing handler for all eight tests. + // + // Files sharing a worker do run one at a time, so the reverse hazard a reviewer raised - a + // CONCURRENT spec file having its unhandled error captured by this `unhandled` spy - cannot + // happen. Sequential execution is the reason, not per-file isolation, which does not exist. + onTestFinished(() => { + rxjsConfig.onUnhandledError = previousHandler; + }); + rxjsConfig.onUnhandledError = unhandled; + try { + closed.next({ operatorId: "a-completely-unrelated-operator" }); + // A real macrotask, not fake timers. `next()` schedules RxJS's report timer synchronously, + // before this line creates its own, and Node fires equal-delay timers in insertion order - + // so the report always lands before the await resumes. That order is load-bearing, not + // just tidy: `reportUnhandledError` reads `config.onUnhandledError` INSIDE its timer + // callback, so were the two to swap, the `finally` would already have restored and the + // error would escape as a genuine unhandled throw instead of reaching the spy. Fake timers + // would buy nothing here (~1ms of a 167ms file) and would cost the flush being a + // whole-timer-API swap while an Angular fixture is live, draining whatever else the + // fixture had queued - and any leaked fake-timer state would follow the shared worker into + // later spec files, the same way a leaked handler would. + await new Promise(resolve => setTimeout(resolve, 0)); + } finally { + rxjsConfig.onUnhandledError = previousHandler; + } + + expect(unhandled).not.toHaveBeenCalled(); + // Trivially satisfied against pristine production - nothing in this test can set the flag - + // but not decorative: this is the assertion that catches the open and close subscriber bodies + // being swapped, in which case a close broadcast would OPEN an editor in a panel that had + // none. Measured: with this line the swap fails 3 tests here, without it only 2. + expect(remoteComponent.isEditorOpen).toBe(false); + }); + it("persists the open flag on destroy so a reopened panel restores it", () => { component.openEditor(); @@ -154,9 +262,7 @@ describe("CodeareaCustomTemplateComponent", () => { // ngOnDestroy writes the CURRENT flag rather than a hardcoded false, so a component torn // down with its editor still up comes back open. - let published: boolean | undefined; - codeEditorService.getEditorState(highlightedOperatorId()).subscribe(v => (published = v)); - expect(published).toBe(true); + expect(publishedEditorState(highlightedOperatorId())).toBe(true); }); it("tracks an external state change through ngOnInit's subscription", () => { diff --git a/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts b/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts index 7d73319ab9..ab13c540f3 100644 --- a/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts +++ b/frontend/src/app/workspace/component/left-panel/settings/settings.component.spec.ts @@ -68,6 +68,11 @@ class StubWorkflowActionService { workflowChanged(): Observable<unknown> { return this.workflowChangedSubject.asObservable(); } + + /** Stands in for the real service's own emissions: undo/redo, a reloaded workflow, a co-editor's edit. */ + emitWorkflowChanged(): void { + this.workflowChangedSubject.next(undefined); + } } describe("SettingsComponent", () => { @@ -180,6 +185,21 @@ describe("SettingsComponent", () => { expect(updateModeSpy).toHaveBeenCalledWith(ExecutionMode.MATERIALIZED); }); + it("should re-sync the form when the workflow changes underneath it", () => { + // The settings can move without the form: undo/redo, a reloaded workflow, a co-editor's edit. + workflowActionService.setWorkflowDataTransferBatchSize(777); + workflowActionService.updateExecutionMode(ExecutionMode.MATERIALIZED); + + workflowActionService.emitWorkflowChanged(); + + expect(component.settingsForm.get("dataTransferBatchSize")!.value).toBe(777); + expect(component.settingsForm.get("executionMode")!.value).toBe(ExecutionMode.MATERIALIZED); + // The patch is applied with { emitEvent: false }. Without that, writing the incoming values + // back into the form would re-fire both valueChanges subscriptions and echo the very state we + // just received straight back to the server on every remote change. + expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); + }); + it("should ignore form value changes that fail validation", () => { const setBatchSizeSpy = vi.spyOn(workflowActionService, "setWorkflowDataTransferBatchSize"); diff --git a/frontend/src/app/workspace/component/result-panel/result-panel-modal.component.spec.ts b/frontend/src/app/workspace/component/result-panel/result-panel-modal.component.spec.ts index f82bd871c3..84a1eeff8e 100644 --- a/frontend/src/app/workspace/component/result-panel/result-panel-modal.component.spec.ts +++ b/frontend/src/app/workspace/component/result-panel/result-panel-modal.component.spec.ts @@ -249,6 +249,7 @@ describe("RowModalComponent (template rendering)", () => { const rowData = { video: "data:video/mp4;base64,vid123", + audio: "data:audio/mp3;base64,aud123", image: "data:image/png;base64,img123", }; @@ -281,6 +282,26 @@ describe("RowModalComponent (template rendering)", () => { expect(videoEl.src).toBe(rowData.video); }); + it("should bind the audio element's src to the audio entry's mediaSrc", () => { + const audioEl = fixture.debugElement.query(By.css("audio")).nativeElement as HTMLAudioElement; + expect(audioEl.src).toBe(rowData.audio); + }); + + it("should re-render the audio src when the entry's mediaSrc is replaced", () => { + // For a data-URI fixture `mediaSrc` and `value` hold the same string, so the assertion above + // cannot tell the two apart - `[src]="entry.value"` satisfies it. They diverge exactly where it + // matters: for a remote http(s) URL `mediaSrc` starts empty and is later replaced by the + // `blob:` URL the SSRF-allowlisted proxy produced, while `value` keeps the raw remote URL. + // Binding `value` would load remote media directly and bypass that proxy. This drives the same + // late replacement the proxy path performs, without needing the HTTP round trip. + const audioEntry = component.rowEntries.find(entry => entry.key === "audio")!; + audioEntry.mediaSrc = "data:audio/mp3;base64,REBOUND"; + fixture.detectChanges(); + + const audioEl = fixture.debugElement.query(By.css("audio")).nativeElement as HTMLAudioElement; + expect(audioEl.src).toBe("data:audio/mp3;base64,REBOUND"); + }); + it("should bind the image element's src to the image entry's mediaSrc", () => { const imgEl = fixture.debugElement.query(By.css("img")).nativeElement as HTMLImageElement; expect(imgEl.src).toBe(rowData.image); diff --git a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts index 94a92148ad..b9241ce1e2 100644 --- a/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts @@ -154,6 +154,30 @@ describe("UiUdfParametersParserService", () => { `, [], ], + [ + // `"s".` parses as a member access whose receiver is a string literal, so the node carries + // no VariableName/PropertyName children at all and the member path it yields is empty + // rather than one- or two-part. Such a call has to be dropped, not turned into a parameter + // with a fabricated receiver and type. + "ignore a type written as a member access with no name parts", + ` + self.UiParameter(name="x", type="s".) + self.UiParameter("valid", AttributeType.STRING) + `, + [parameter("valid", "string")], + ], + [ + // A tuple is not a member access, but its direct children are two VariableNames that read + // exactly like `AttributeType` + `STRING`. Only the node-kind check in front of the member + // path stops `(AttributeType, STRING)` from being accepted as the STRING type, so this is + // the case that keeps that check honest. + "ignore a type written as a tuple that reads like a member path", + ` + self.UiParameter(name="x", type=(AttributeType, STRING)) + self.UiParameter("valid", AttributeType.STRING) + `, + [parameter("valid", "string")], + ], ] as ReadonlyArray<readonly [string, string, UiUdfParameter[]]> ).forEach(([description, openBody, expectedParameters]) => { it(`should ${description}`, () => {
![http://example.com/model-cover.png"](http://example.com/model-cover.png"){kind=link}
![http://example.com/dataset-cover.png"](http://example.com/dataset-cover.png"){kind=link}