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-7733-60300e3a1ce28ac9d6c698d3bd0d8b202a697411 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 958264aee9043b860adcbd3b6271866d653c86ce Author: Xinyuan Lin <[email protected]> AuthorDate: Tue Aug 18 07:46:14 2026 +0000 test(frontend): cover the console frame's controls and the preset guards (#7733) ### What changes were proposed in this PR? Two small frontend targets. The headline is the console frame template, whose line percentage hid the real gap: | File | Before | After | |---|---|---| | `console-frame.component.html` | branches **0/8**, functions **5/9** | **8/8 branches, 9/9 functions** | | `console-frame.component.ts` | 96.9% lines, 95% functions | **65/65 lines (100%), 20/20 functions** | | `preset.service.ts` | 94.8% lines, 90.5% branches | **111/115 lines, 70/74 branches** | 7 tests added. Covered: `ngOnChanges` adopting a new operator id, the empty-list fallback, typing into the command box through a real `input` event and submitting, narrowing the send by picking a worker in the `nz-select` overlay, the settings dropdown's two toggles, and two `preset.service` validation guards. **This is 8 lines by line-count.** What earns it is the branch and function coverage on the template — every interactive control in the console frame was previously unexercised — and that all 8 mutations die. ### Not an #7458 case, and worth saying so The obvious guess for a dark template here is the `TestBed.overrideComponent` attribution loss behind merged PRs #7535, #7627, #7629, #7661, #7681 and #7727. It is not: the existing spec uses `imports: [ConsoleFrameComponent, ...]` with no override, and the coverage map shows bindings executing counted (the `*ngFor` statement has 124 hits). So this extends the existing TestBed rather than appending a separate one — the opposite call from #7727, for a checkable reason. ### Verification 8 mutations, **8 killed, no survivors**, each applied one at a time with the anchor asserted unique, reverted and `git diff`-checked between every run. All failures are assertion failures, never compile errors. | Mutation | Killed by | |---|---| | **exchange** `[(ngModel)]="showTimestamp"` and `"showSource"` | toggles the timestamp and source tags independently | | `[(ngModel)]="targetWorker"` -> one-way | narrows the command to the worker picked | | `[(ngModel)]="command"` -> one-way | sends the text typed into the command box | | **exchange** `currentValue` and `previousValue` | `ngOnChanges` adopts the newly bound operator id | | remove the `\|\| []` fallback | falls back to an empty list for an unseen operator | | **exchange** the `"error"` and `"info"` switch bodies | refuses to save with an 'info' severity | | `push(replacementPreset)` -> `push(originalPreset)` | stores the replacement when the dictionary has no entry yet | | give `"warning"` a default toast instead of throwing | refuses to save with a 'warning' severity | The first one is the reason the fixture is not degenerate: the kill lands on the *independence* assertion (timestamp off, source still on). Turning both toggles off at once would have survived the exchange. One mechanical note worth recording: the settings dropdown's menu is projected into a CDK overlay wired in `ngAfterViewInit` behind an `auditTime(150)`, so its fixture must be created **inside** `fakeAsync`. Created in a plain `beforeEach`, the timers escape `tick()`, the overlay never attaches, and the switch count is 0 — a test written against that state would pass while asserting nothing. ### Deliberately not included, with evidence - **`updatePreset` (lines 184-190) is dead *and* buggy**, so no test was written for it. It has zero call sites repo-wide outside its own spec. And `indexOf(presets, originalPreset)` is lodash reference-equality against a freshly `JSON.parse`d array, so it always returns `-1`: `splice(-1, 1)` deletes the **wrong** preset and `presets[-1] = ...` is a silent no-op that `JSON.stringify` drops. Its sibling `updateOrCreatePreset` carries the comment *"presets are freshly JSON-parsed, so reference-based indexOf would miss"* and uses `findIndex(isEqual)` — the fix was applied there and not here. Any test would cement the bug. - **`console-frame.component.html` lines 54 and 59 are structurally unreachable**: `#checkedTemplate` and `#unCheckedTemplate` are each declared **twice** (36/41 and 53/58), and both `nz-switch`es resolve to the first pair. The coverage map proves it — statements at 37/42 have 46 hits (2 switches x 23 fixtures) while 54/59 have 0. - Two `.ts` branch arms are guard-guaranteed: `renderConsole()`'s `if (this.operatorId)` already forces the ternary's true leg, and `#consoleList` is unconditional in the template so its `@ViewChild` is always set in a rendered fixture. A third, weaker observation, reported and not pinned: `ngOnChanges` does `this.operatorId = changes.operatorId?.currentValue`, so an `ngOnChanges` fired by a change to `consoleInputEnabled` alone would wipe `operatorId` to `undefined` and silently disable the debug console. `ResultPanelComponent` always sets both inputs together, so it is latent rather than triggered today; the new test pins only the normal path. No production file is touched. ### Any related issues, documentation, discussions? Closes #7732 ### How was this PR tested? ``` npx ng test --watch=false --include="**/preset.service.spec.ts" --include="**/console-frame.component.spec.ts" ``` ``` Test Files 2 passed (2) ``` 86 tests green in the two target specs; 174 green including the consumer specs `result-panel.component.spec.ts` and `preset-wrapper.component.spec.ts`, checked for CDK-overlay leakage across specs. `yarn format:ci` passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../console-frame/console-frame.component.spec.ts | 190 ++++++++++++++++++++- .../service/preset/preset.service.spec.ts | 31 ++++ 2 files changed, 220 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts b/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts index 20392ce767..af17ba018c 100644 --- a/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/result-panel/console-frame/console-frame.component.spec.ts @@ -17,7 +17,7 @@ * under the License. */ -import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ComponentFixture, TestBed, fakeAsync, flush, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { Subject } from "rxjs"; import { ConsoleFrameComponent } from "./console-frame.component"; @@ -171,6 +171,34 @@ describe("ConsoleFrameComponent", () => { component.renderConsole(); expect(getWorkerIds).not.toHaveBeenCalled(); }); + + it("displayConsoleMessages falls back to an empty list for an operator the service has never seen", () => { + // WorkflowConsoleService.getConsoleMessages returns undefined until the + // operator has produced its first message; the frame must not render undefined. + component.consoleMessages = [consoleMessage("PRINT")]; + getConsoleMessages.mockReturnValue(undefined); + + component.displayConsoleMessages("op-never-run"); + + expect(component.consoleMessages).toEqual([]); + }); + + it("ngOnChanges adopts the newly bound operator id and re-renders for it", () => { + // ResultPanelComponent recreates this frame with {operatorId, consoleInputEnabled} + // inputs, so the operator the console follows arrives through ngOnChanges. + getWorkerIds.mockReturnValue(["w-op2-1"]); + getConsoleMessages.mockReturnValue([consoleMessage("COMMAND")]); + + component.ngOnChanges({ + operatorId: { currentValue: "op2", previousValue: "op1", firstChange: false, isFirstChange: () => false }, + }); + + expect(component.operatorId).toBe("op2"); + expect(getWorkerIds).toHaveBeenCalledWith("op2"); + expect(getConsoleMessages).toHaveBeenCalledWith("op2"); + expect(component.workerIds).toEqual(["w-op2-1"]); + expect(component.consoleMessages).toEqual([consoleMessage("COMMAND")]); + }); }); describe("debug controls", () => { @@ -378,5 +406,165 @@ describe("ConsoleFrameComponent", () => { expect(send).toHaveBeenCalledWith("DebugCommandRequest", { operatorId: "op1", workerId: "w-0", cmd: "break" }); expect(send).toHaveBeenCalledWith("DebugCommandRequest", { operatorId: "op1", workerId: "w-1", cmd: "break" }); }); + + it("sends the text typed into the command box, then clears it", fakeAsync(() => { + component.operatorId = "op1"; + component.workerIds = ["w-7"]; + component.consoleInputEnabled = true; + fixture.detectChanges(); + + // Drive the input the way a user does — set the DOM value and let the + // DefaultValueAccessor push it back through [(ngModel)]. Nothing here + // assigns component.command, so a broken view-to-model binding would send + // the empty string instead of the typed text. + const input = fixture.debugElement.query(By.css("input[nz-input]")).nativeElement as HTMLInputElement; + input.value = "print(row)"; + input.dispatchEvent(new Event("input", { bubbles: true })); + fixture.detectChanges(); + + input.dispatchEvent(new KeyboardEvent("keyup", { key: "Enter", bubbles: true })); + fixture.detectChanges(); + + expect(send).toHaveBeenCalledWith("DebugCommandRequest", { + operatorId: "op1", + workerId: "w-7", + cmd: "print(row)", + }); + // the box is emptied after submitting, so the next command starts clean + expect(input.value).toBe(""); + flush(); + })); + + it("narrows the command to the worker picked in the target-worker dropdown", fakeAsync(() => { + component.operatorId = "op1"; + component.workerIds = ["w-7", "w-8"]; + component.consoleInputEnabled = true; + fixture.detectChanges(); + + // open the nz-select and pick the *second* worker (W8), so a binding that + // ignored the selection would still be sitting on W7 / All Workers. + const select = fixture.debugElement.query(By.css("nz-select")).nativeElement as HTMLElement; + select.click(); + tick(500); + fixture.detectChanges(); + + const options = Array.from(document.querySelectorAll("nz-option-item")); + expect(options.map(option => option.textContent?.trim())).toEqual(["W7", "W8", "All Workers"]); + (options[1] as HTMLElement).click(); + tick(500); + fixture.detectChanges(); + + // the closed select shows the picked worker + expect(fixture.debugElement.query(By.css("nz-select-item")).nativeElement.textContent.trim()).toBe("W8"); + + const input = fixture.debugElement.query(By.css("input[nz-input]")).nativeElement as HTMLInputElement; + input.value = "where"; + input.dispatchEvent(new Event("input", { bubbles: true })); + fixture.detectChanges(); + input.dispatchEvent(new KeyboardEvent("keyup", { key: "Enter", bubbles: true })); + + // exactly one send, to w-8 only — not broadcast, and not to w-7 + expect(send).toHaveBeenCalledTimes(1); + expect(send).toHaveBeenCalledWith("DebugCommandRequest", { operatorId: "op1", workerId: "w-8", cmd: "where" }); + flush(); + })); }); }); + +/** + * The settings dropdown (the little gear above the console) hosts the + * "Show Timestamp" / "Show Source" switches. Its menu is projected into a CDK + * overlay that only attaches once the trigger is hovered, and the trigger + * pipeline is wired in ngAfterViewInit behind an auditTime, so the fixture has + * to be created *inside* fakeAsync for tick() to drive it. That is why this + * lives in its own describe rather than reusing the fixture above. + */ +describe("ConsoleFrameComponent settings dropdown", () => { + const message: ConsoleMessage = { + workerId: "w-3", + timestamp: { nanos: 0, seconds: 1 }, + msgType: { name: "PRINT" }, + source: "operator-a:main", + title: "title", + message: "body", + }; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ConsoleFrameComponent, HttpClientTestingModule, NzDropDownModule], + providers: [ + { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, + { provide: ComputingUnitStatusService, useClass: MockComputingUnitStatusService }, + { + provide: ExecuteWorkflowService, + useValue: { + getExecutionStateStream: () => new Subject<StateEvent>().asObservable(), + getWorkerIds: () => [], + skipTuples: () => {}, + retryExecution: () => {}, + }, + }, + { + provide: WorkflowConsoleService, + useValue: { + getConsoleMessageUpdateStream: () => new Subject<void>().asObservable(), + getConsoleMessages: () => [], + }, + }, + { provide: WorkflowWebsocketService, useValue: { send: () => {} } }, + { provide: NotificationService, useValue: { error: () => {} } }, + { provide: UdfDebugService, useValue: { doStep: () => {}, doContinue: () => {} } }, + ...commonTestProviders, + ], + }).compileComponents(); + }); + + it("toggles the timestamp and source tags independently from the settings menu", fakeAsync(() => { + const fixture = TestBed.createComponent(ConsoleFrameComponent); + fixture.componentInstance.consoleMessages = [message]; + fixture.detectChanges(); + + const timestampTags = () => fixture.debugElement.queryAll(By.css(".timestamp-tag")).length; + const sourceTags = () => fixture.debugElement.queryAll(By.css(".source-tag")).length; + + // both switches default to on, so both tags start out rendered + expect(timestampTags()).toBe(1); + expect(sourceTags()).toBe(1); + + // hovering the gear attaches the dropdown overlay + fixture.debugElement + .query(By.css("a[nz-dropdown]")) + .nativeElement.dispatchEvent(new MouseEvent("mouseenter", { bubbles: true })); + tick(300); + fixture.detectChanges(); + + const menuItems = Array.from(document.querySelectorAll("li[nz-menu-item]")); + expect(menuItems.map(item => item.textContent?.trim())).toEqual(["Show Timestamp", "Show Source"]); + const switches = Array.from(document.querySelectorAll("nz-switch button.ant-switch")) as HTMLElement[]; + expect(switches.length).toBe(2); + expect(switches[0].classList.contains("ant-switch-checked")).toBe(true); + + // flip only "Show Timestamp": the timestamp tag goes away and the source tag stays. + // Asserting the two independently is what distinguishes the bindings — turning + // both off at once would look the same if the two switches were swapped. + switches[0].click(); + tick(300); + fixture.detectChanges(); + + expect(switches[0].classList.contains("ant-switch-checked")).toBe(false); + expect(timestampTags()).toBe(0); + expect(sourceTags()).toBe(1); + + // now flip "Show Source" as well + switches[1].click(); + tick(300); + fixture.detectChanges(); + + expect(switches[1].classList.contains("ant-switch-checked")).toBe(false); + expect(sourceTags()).toBe(0); + expect(timestampTags()).toBe(0); + + fixture.destroy(); + flush(); + })); +}); diff --git a/frontend/src/app/workspace/service/preset/preset.service.spec.ts b/frontend/src/app/workspace/service/preset/preset.service.spec.ts index 51a24132a2..a9f32e2b01 100644 --- a/frontend/src/app/workspace/service/preset/preset.service.spec.ts +++ b/frontend/src/app/workspace/service/preset/preset.service.spec.ts @@ -195,6 +195,22 @@ describe("PresetService", () => { } }); + it("refuses to save with an 'info' severity when no message is supplied", () => { + // There is no default text for the informational severity, so callers that + // omit `displayMessage` must be rejected rather than shown a blank toast. + expect(() => + presetService.savePresets(presetType, presetTarget, [{ presetProperty: "v1" }], undefined, "info") + ).toThrow("no default save preset info message"); + expect(messageStub.info).not.toHaveBeenCalled(); + }); + + it("refuses to save with a 'warning' severity when no message is supplied", () => { + expect(() => + presetService.savePresets(presetType, presetTarget, [{ presetProperty: "v1" }], undefined, "warning") + ).toThrow("no default save preset warning message"); + expect(messageStub.warning).not.toHaveBeenCalled(); + }); + it("deletePreset reports the caller's message as an error toast", () => { userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]))); @@ -530,6 +546,21 @@ describe("PresetService", () => { expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify(stored)); }); + it("stores the replacement when the dictionary has no entry yet", () => { + // First write for this operator type: fetchKey resolves to null, so the + // missing entry has to read as an empty list rather than being parsed. + userConfigStub.fetchKey.mockReturnValue(of(null)); + + presetService.updateOrCreatePreset( + presetType, + presetTarget, + { presetProperty: "missing" }, + { presetProperty: "v2" } + ); + + expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify([{ presetProperty: "v2" }])); + }); + it("appends the replacement when neither preset already exists", () => { userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }])));
