Copilot commented on code in PR #7501:
URL: https://github.com/apache/texera/pull/7501#discussion_r3746721423
##########
frontend/src/app/dashboard/component/admin/execution/admin-execution.component.spec.ts:
##########
@@ -537,3 +537,97 @@ describe("AdminExecutionComponent methods (#6550)", () => {
});
});
});
+
+describe("AdminExecutionComponent template rendering", () => {
+ let component: AdminExecutionComponent;
+ let fixture: ComponentFixture<AdminExecutionComponent>;
+ let service: AdminExecutionService;
+
+ const makeExecution = (overrides: Partial<Execution> = {}): Execution => ({
+ workflowName: "a-very-long-workflow-name-to-truncate",
+ workflowId: 11,
+ userName: "alice",
+ userId: 1,
+ executionId: 21,
+ executionStatus: "COMPLETED",
+ executionTime: 3661_000,
+ executionName: "run-1",
+ startTime: 1_700_000_000_000,
+ endTime: 1_700_000_100_000,
+ access: true,
+ ...overrides,
+ });
+
+ beforeEach(async () => {
+ await TestBed.configureTestingModule({
+ providers: [AdminExecutionService, ...commonTestProviders],
+ imports: [AdminExecutionComponent, HttpClientTestingModule,
NzDropDownModule, NzModalModule],
+ }).compileComponents();
+
+ fixture = TestBed.createComponent(AdminExecutionComponent);
+ component = fixture.componentInstance;
+ service = TestBed.inject(AdminExecutionService);
+ // Keep the fetches inert; the rows are seeded directly so ngOnInit's
pollers have
+ // nothing to do (they are covered by the lifecycle tests above).
+ vi.spyOn(service, "getExecutionList").mockReturnValue(of([]));
+ vi.spyOn(service, "getTotalWorkflows").mockReturnValue(of(0));
+ });
+
+ afterEach(() => fixture.destroy());
+
+ const renderRows = (executions: Execution[]): void => {
+ // ngOnInit refills listOfExecutions from the service, so supply the rows
through the
+ // stub rather than assigning them (which the fetch would overwrite).
+ vi.mocked(service.getExecutionList).mockReturnValue(of(executions));
+ fixture.detectChanges();
+ fixture.detectChanges();
+ };
+
+ // nz-table renders an internal measure row with empty cells; keep only real
data rows.
+ const dataRows = () =>
+ fixture.debugElement
+ .queryAll(By.css("tbody tr"))
+ .filter(row => !row.nativeElement.hasAttribute("nz-table-measure-row"));
+
+ const cellsOf = (rowIndex: number): string[] =>
+ dataRows()
+ [rowIndex].queryAll(By.css("td"))
+ .map(td => (td.nativeElement.textContent ?? "").trim());
+
+ it("renders one row per execution with the truncated name and formatted
duration", () => {
+ const execution = makeExecution();
+ renderRows([execution]);
+
+ expect(dataRows()).toHaveLength(1);
+ const cells = cellsOf(0);
+ // the interpolations run the component's own helpers, so assert against
them
+
expect(cells[0]).toContain(component.maxStringLength(execution.workflowName,
16));
+ expect(cells[0]).toContain(String(execution.workflowId));
+ expect(cells.join("
")).toContain(component.convertSecondsToTime(execution.executionTime));
+ });
+
+ it("renders the Not Available arm when the execution time is negative", ()
=> {
+ renderRows([makeExecution({ executionTime: -1, endTime: 0 })]);
+
+ expect(cellsOf(0).join(" ")).toContain("Not Available");
+ });
+
+ it("kills the execution with its workflow id when the kill control is
clicked", () => {
+ const killSpy = vi.spyOn(component, "killExecution").mockImplementation(()
=> {});
+ renderRows([makeExecution({ executionStatus: "RUNNING", workflowId: 42
})]);
+
+ const killControl = fixture.debugElement
+ .queryAll(By.css("tbody tr button, tbody tr i"))
+ .find(el => (el.nativeElement.getAttribute("nztype") ??
"").includes("close"));
+
+ if (killControl) {
+ killControl.triggerEventHandler("click", null);
+ expect(killSpy).toHaveBeenCalledWith(42);
+ } else {
+ // the control is behind a status the fixture does not reach; exercise
the
+ // binding target directly so the handler is still covered
+ component.killExecution(42);
+ expect(killSpy).toHaveBeenCalledWith(42);
+ }
Review Comment:
The kill-control lookup is searching for an icon with nztype containing
"close", but the template’s kill button uses `nzType="stop"`
(admin-execution.component.html:144). As a result, `killControl` will be
undefined and the test will always fall back to calling
component.killExecution(42) directly, which doesn’t exercise the template
(click) binding.
Select the actual kill button/icon ("stop" or the kill tooltip) and trigger
its click so the template binding is covered.
##########
frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts:
##########
@@ -348,4 +348,63 @@ describe("PresetWrapperComponent", () => {
expect(messageStub.error).toHaveBeenCalledTimes(1);
});
});
+
+ // ─── template rendering
────────────────────────────────────────────────────
+ describe("template rendering", () => {
+ const initWith = (presets: Preset[]): void => {
+ // ngOnInit re-populates searchResults from the service, so feed the
presets
+ // through the stub rather than assigning after init (which it would
overwrite).
+ presetServiceStub.getPresets.mockReturnValue(of(presets));
+ component.field = buildField();
+ component.ngOnInit();
+ fixture.detectChanges();
+ };
+
+ it("renders the save button and saves the preset when it is clicked", ()
=> {
+ initWith([]);
+
+ const saveBtn = fixture.nativeElement.querySelector(".save-button") as
HTMLButtonElement;
+ expect(saveBtn).toBeTruthy();
+
+ const savePreset = vi.spyOn(component,
"savePreset").mockImplementation(() => {});
+ saveBtn.click();
+
+ 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.
+ 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");
+ });
+
+ it("binds an empty dropdown list when there are no presets", () => {
+ initWith([]);
+
+ expect(component.searchResults).toEqual([]);
+ });
+
+ it("applies the preset the row's (click) binding targets", () => {
+ initWith([testPreset]);
+
+ component.applyPreset(testPreset);
+
+
expect(presetServiceStub.applyPreset).toHaveBeenCalledWith(expect.anything(),
expect.anything(), testPreset);
+ });
Review Comment:
This test calls component.applyPreset(...) directly, so it does not execute
the template's row (click) binding (preset-wrapper.component.html:62) and won’t
improve template coverage for that markup.
To cover the template branch/binding, open the dropdown overlay (e.g., set
presetMenuVisible true / trigger nzVisibleChange and use CDK OverlayContainer),
then click the rendered .dropdown-entry element and assert
presetService.applyPreset was called with the preset.
##########
frontend/src/app/common/formly/preset-wrapper/preset-wrapper.component.spec.ts:
##########
@@ -348,4 +348,63 @@ describe("PresetWrapperComponent", () => {
expect(messageStub.error).toHaveBeenCalledTimes(1);
});
});
+
+ // ─── template rendering
────────────────────────────────────────────────────
+ describe("template rendering", () => {
+ const initWith = (presets: Preset[]): void => {
+ // ngOnInit re-populates searchResults from the service, so feed the
presets
+ // through the stub rather than assigning after init (which it would
overwrite).
+ presetServiceStub.getPresets.mockReturnValue(of(presets));
+ component.field = buildField();
+ component.ngOnInit();
+ fixture.detectChanges();
+ };
+
+ it("renders the save button and saves the preset when it is clicked", ()
=> {
+ initWith([]);
+
+ const saveBtn = fixture.nativeElement.querySelector(".save-button") as
HTMLButtonElement;
+ expect(saveBtn).toBeTruthy();
+
+ const savePreset = vi.spyOn(component,
"savePreset").mockImplementation(() => {});
+ saveBtn.click();
+
+ 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.
+ 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");
+ });
+
+ it("binds an empty dropdown list when there are no presets", () => {
+ initWith([]);
+
+ expect(component.searchResults).toEqual([]);
+ });
+
+ it("applies the preset the row's (click) binding targets", () => {
+ initWith([testPreset]);
+
+ component.applyPreset(testPreset);
+
+
expect(presetServiceStub.applyPreset).toHaveBeenCalledWith(expect.anything(),
expect.anything(), testPreset);
+ });
+
+ it("deletes the preset the delete button's (click) binding targets", () =>
{
+ initWith([testPreset]);
+
+ component.deletePreset(testPreset);
+
+ expect(presetServiceStub.deletePreset).toHaveBeenCalled();
+ });
Review Comment:
This test calls component.deletePreset(...) directly, so it does not execute
the template’s delete button (click) handler `($event.stopPropagation();
deletePreset(preset))` (preset-wrapper.component.html:74) and won’t cover the
stopPropagation branch.
To exercise the template binding, trigger the delete button’s click handler
from the rendered dropdown entry (likely via the CDK overlay) and pass an event
object with a stopPropagation spy, then assert both stopPropagation and
presetService.deletePreset were called.
##########
frontend/src/app/dashboard/component/user/markdown-description/markdown-description.component.spec.ts:
##########
@@ -309,4 +309,84 @@ describe("MarkdownDescriptionComponent", () => {
expect(parse).not.toHaveBeenCalled();
expect(component.renderedDescription).toBe("");
});
+
+ // ─── template rendering
────────────────────────────────────────────────────
+ // Drive the markup through the DOM so the (click) attributes and
interpolations
+ // in the template actually execute.
+ describe("template rendering", () => {
+ it("enters edit mode when the Edit button is clicked", async () => {
+ const fixture = await createFixture();
+ const component = fixture.componentInstance;
+ component.editable = true;
+ fixture.detectChanges();
+
+ const editBtn = fixture.nativeElement.querySelector(".md-actions
button") as HTMLButtonElement;
+ expect(editBtn).toBeTruthy();
+ editBtn.click();
+ fixture.detectChanges();
+
+ expect(component.currentMode).toBe("edit");
+ // the edit-mode arm of the template now renders
+ expect(fixture.nativeElement.querySelector(".md-split")).toBeTruthy();
+ });
+
+ it("renders the parsed markdown through the innerHTML binding", async ()
=> {
+ const fixture = await createFixture();
+ const component = fixture.componentInstance;
+ component.description = "hello";
+ fixture.detectChanges();
+ await fixture.whenStable();
+ fixture.detectChanges();
+
+ const rendered = fixture.nativeElement.querySelector(".md-rendered") as
HTMLElement;
+ expect(rendered).toBeTruthy();
+ expect(rendered.innerHTML).toContain("hello");
+ });
+
+ it("falls back to the no-description template when nothing is rendered",
async () => {
+ const fixture = await createFixture();
+ fixture.componentInstance.description = "";
+ fixture.detectChanges();
+ await fixture.whenStable();
+ fixture.detectChanges();
+
+ expect(fixture.nativeElement.querySelector(".md-rendered")).toBeNull();
+ expect(fixture.nativeElement.textContent).toContain("No description
provided.");
+ });
+
+ it("toggles view-more from the template and flips the chevron binding",
async () => {
+ const fixture = await createFixture();
+ const component = fixture.componentInstance;
+ // the button is behind enableViewMore && hasOverflow
+ component.enableViewMore = true;
+ component.hasOverflow = true;
+ fixture.detectChanges();
+
+ const viewMoreBtn =
fixture.nativeElement.querySelector(".view-more-btn") as HTMLButtonElement;
+ expect(viewMoreBtn).toBeTruthy();
+ // the label interpolation and the chevron's [nzType] are driven by the
same flag
+ expect(viewMoreBtn.textContent).toContain("View more");
+
+ viewMoreBtn.click();
+ fixture.detectChanges();
+
+ expect(component.isExpanded).toBe(true);
+ expect(viewMoreBtn.textContent).toContain("View less");
+
+ viewMoreBtn.click();
+ fixture.detectChanges();
+
+ expect(component.isExpanded).toBe(false);
+ expect(viewMoreBtn.textContent).toContain("View more");
+ });
Review Comment:
This test name/comment claims it "flips the chevron binding", but it only
asserts the label text and component.isExpanded. The template also binds the
chevron icon `[nzType]="isExpanded ? 'up' : 'down'"`
(markdown-description.component.html:57), and a regression there would not be
caught.
Add an assertion that the icon’s bound type/class changes alongside the
label when toggling.
##########
frontend/src/app/workspace/component/property-editor/property-editor.component.spec.ts:
##########
@@ -388,4 +388,36 @@ describe("PropertyEditorComponent", () => {
freshFixture.destroy();
}
});
+
+ // ─── template rendering
────────────────────────────────────────────────────
+ // The docked buttons swap between a collapse and an expand item depending
on the
+ // panel width; drive both through the DOM so each (click) binding executes.
+ describe("template rendering", () => {
+ const dockedItems = (): HTMLElement[] =>
+ Array.from(fixture.nativeElement.querySelectorAll("#docked-buttons li"))
as HTMLElement[];
+
+ it("collapses the panel when the docked collapse item is clicked", () => {
+ component.width = 280;
+ fixture.detectChanges();
+
+ const items = dockedItems();
+ expect(items).toHaveLength(1); // only the collapse arm renders while
open
+ items[0].click();
+ fixture.detectChanges();
+
+ expect(component.width).toBe(0);
+ });
+
+ it("reopens the panel when the collapsed docked item is clicked", () => {
+ component.width = 0;
+ fixture.detectChanges();
+
+ const items = dockedItems();
+ expect(items).toHaveLength(1); // only the expand arm renders while
collapsed
+ items[0].click();
+ fixture.detectChanges();
+
+ expect(component.width).toBe(280);
+ });
Review Comment:
Clicking the collapsed docked item calls openPanel(), which schedules a
setTimeout via updateHeightBasedOnContent(). Because this spec doesn’t
stub/flush that timer, the callback can fire after fixture.destroy() in
afterEach and intermittently throw (e.g., calling
changeDetectorRef.detectChanges() on a destroyed fixture).
Stub updateHeightBasedOnContent() in this test (similar to the existing
openPanel/resetPanels specs earlier in the file) to avoid leaving a pending
timer during teardown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
