Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -193,5 +193,51 @@ describe("ComputingUnitActionsService", () => {

expect(notificationService.error).toHaveBeenCalledWith("Failed to terminate computing unit: kaboom");
});

// ng-zorro renders string content as HTML, and an admin sees other users' unit names, so a name must reach the
// modal and the toast as text.
describe("a unit name that holds markup", () => {
const markup = '<a href="https://evil.example">Confirm</a> & <b>x</b>';
const escaped = "&lt;a href=&quot;https://evil.example&quot;&gt;Confirm&lt;/a&gt; &amp; &lt;b&gt;x&lt;/b&gt;";

it.each(["kubernetes", "local"])("is escaped in the confirmation modal of a %s unit", type => {
service.confirmAndTerminate(7, unit({ type, name: markup }));

expect(modalService.confirm.mock.calls[0][0].nzContent).toContain(escaped);
});

it("is escaped in the success message", () => {
service.confirmAndTerminate(7, unit({ name: markup }));

modalService.confirm.mock.calls[0][0].nzOnOk();

expect(notificationService.success).toHaveBeenCalledWith(`Terminated Computing Unit: ${escaped}`);
});
});

describe("onTerminated callback", () => {
it("runs once the confirmed termination succeeds, and not before the modal is confirmed", () => {
const onTerminated = vi.fn();
service.confirmAndTerminate(7, unit(), onTerminated);
expect(onTerminated).not.toHaveBeenCalled();

modalService.confirm.mock.calls[0][0].nzOnOk();

expect(onTerminated).toHaveBeenCalledTimes(1);
});

it.each([
["reports failure", of(false)],
["observable errors", throwError(() => new Error("kaboom"))],
])("does not run when the termination %s", (_outcome, termination) => {
const onTerminated = vi.fn();
statusService.terminateComputingUnit.mockReturnValue(termination);
service.confirmAndTerminate(7, unit(), onTerminated);

modalService.confirm.mock.calls[0][0].nzOnOk();

expect(onTerminated).not.toHaveBeenCalled();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

import { Injectable } from "@angular/core";
import { escape as escapeHtml } from "lodash-es";
import { Observable } from "rxjs";
import { NzModalService } from "ng-zorro-antd/modal";
import { ShareAccessComponent } from "../../../../dashboard/component/user/share-access/share-access.component";
Expand Down Expand Up @@ -88,13 +89,15 @@ export class ComputingUnitActionsService {
throw new Error("Unsupported computing unit type");
}

confirmAndTerminate(cuid: number, unit: DashboardWorkflowComputingUnit): void {
/** Runs `onTerminated` once the unit is terminated, and not if the modal is cancelled or the termination fails. */
confirmAndTerminate(cuid: number, unit: DashboardWorkflowComputingUnit, onTerminated?: () => void): void {
if (!unit.computingUnit.uri) {
this.notificationService.error("Invalid computing unit.");
return;
}

const unitName = unit.computingUnit.name;
// ng-zorro renders this string as HTML and any user can name their unit, so the name is escaped.
const unitName = escapeHtml(unit.computingUnit.name);
const unitType = unit?.computingUnit.type || "kubernetes"; // fallback
const templates = unitTypeMessageTemplate[unitType];

Expand All @@ -118,6 +121,7 @@ export class ComputingUnitActionsService {
next: (success: boolean) => {
if (success) {
this.notificationService.success(`Terminated Computing Unit: ${unitName}`);
onTerminated?.();
} else {
this.notificationService.error("Failed to terminate computing unit");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@
<th nzWidth="40px"></th>
<th
[nzSortFn]="sortByName"
nzWidth="20%">
nzWidth="18%">
Name
</th>
<th
[nzSortFn]="sortByOwner"
nzWidth="20%">
nzWidth="18%">
Owner
</th>
<th
[nzSortFn]="sortByType"
[nzFilters]="typeFilters"
[nzFilterFn]="filterByType"
nzWidth="12%">
nzWidth="10%">
Type
</th>
<th
Expand All @@ -76,7 +76,8 @@
nzWidth="16%">
Created
</th>
<th nzWidth="20%">Resources</th>
<th nzWidth="18%">Resources</th>
<th nzWidth="8%">Actions</th>
</tr>
</thead>
<tbody>
Expand Down Expand Up @@ -106,10 +107,23 @@
{{ formatRelativeTime(unit.computingUnit.creationTime) }}
</td>
<td>{{ resourceSummary(unit) }}</td>
<td>
<button
nz-button
nzType="text"
nzDanger
[nz-tooltip]="terminateTooltip(unit)"
[attr.aria-label]="terminateTooltip(unit)"
(click)="terminate(unit)">
<span
nz-icon
nzType="delete"></span>
</button>
</td>
</tr>
<tr *ngIf="expandedCuids.has(unit.computingUnit.cuid)">
<td></td>
<td colspan="6">
<td colspan="7">
<dl class="spec-detail">
<div *ngFor="let field of specFields">
<dt>{{ field.label }}</dt>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import { HttpErrorResponse } from "@angular/common/http";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { of, Subject, throwError } from "rxjs";
import { NzMessageService } from "ng-zorro-antd/message";
import { NzModalService } from "ng-zorro-antd/modal";
import { AdminComputingUnitComponent } from "./admin-computing-unit.component";
import { WorkflowComputingUnitManagingService } from "../../../../common/service/computing-unit/workflow-computing-unit/workflow-computing-unit-managing.service";
import { ComputingUnitActionsService } from "../../../../common/service/computing-unit/computing-unit-actions/computing-unit-actions.service";
import {
DashboardWorkflowComputingUnit,
WorkflowComputingUnit,
Expand Down Expand Up @@ -81,8 +83,8 @@ function withResource(over: Partial<WorkflowComputingUnitResourceLimit>): Partia
return withCu({ resource: { ...makeUnit().computingUnit.resource, ...over } });
}

function localUnit(): DashboardWorkflowComputingUnit {
return makeUnit(withCu({ type: "local", resource: NAN_RESOURCE }));
function localUnit(cuid = 1): DashboardWorkflowComputingUnit {
return makeUnit(withCu({ cuid, type: "local", resource: NAN_RESOURCE }));
}

describe("AdminComputingUnitComponent", () => {
Expand All @@ -92,7 +94,7 @@ describe("AdminComputingUnitComponent", () => {

beforeEach(async () => {
await TestBed.configureTestingModule({
providers: [{ provide: UserService, useClass: StubUserService }, ...commonTestProviders],
providers: [NzModalService, { provide: UserService, useClass: StubUserService }, ...commonTestProviders],
imports: [AdminComputingUnitComponent, ...commonTestImports],
}).compileComponents();

Expand All @@ -108,6 +110,23 @@ describe("AdminComputingUnitComponent", () => {
fixture.destroy();
});

// `nz-table` adds a hidden measure row, so match on the owner cell every data row has.
const dataRows = () =>
Array.from<HTMLElement>(fixture.nativeElement.querySelectorAll("tbody tr")).filter(
row => row.querySelector("texera-user-avatar") !== null
);

// Stubs the shared confirmation, so no modal opens. `confirmed` stands for the admin confirming it and the
// termination succeeding.
const stubConfirm = (confirmed = false) =>
vi
.spyOn(TestBed.inject(ComputingUnitActionsService), "confirmAndTerminate")
.mockImplementation((_cuid, _unit, onTerminated) => {
if (confirmed) {
onTerminated?.();
}
});

it("should create", () => {
expect(component).toBeTruthy();
});
Expand All @@ -119,12 +138,9 @@ describe("AdminComputingUnitComponent", () => {

fixture.detectChanges();

// `nz-table` adds a hidden measure row, so match on the owner cell every data row has.
const dataRows = Array.from<HTMLElement>(fixture.nativeElement.querySelectorAll("tbody tr")).filter(
row => row.querySelector("texera-user-avatar") !== null
);
expect(dataRows.length).toBe(1);
expect(dataRows[0].textContent).toContain("alice");
const rows = dataRows();
expect(rows.length).toBe(1);
expect(rows[0].textContent).toContain("alice");
});

describe("loading and polling", () => {
Expand Down Expand Up @@ -257,6 +273,28 @@ describe("AdminComputingUnitComponent", () => {
});
});

// A poll that started before a termination can answer after it, with a list that still has the unit.
it("does not bring a terminated row back when a poll that was in flight answers", () => {
const first = makeUnit();
const second = makeUnit(withCu({ cuid: 2 }));
const inFlight = new Subject<DashboardWorkflowComputingUnit[]>();
vi.mocked(service.listAllComputingUnits)
.mockReturnValueOnce(of([first, second]))
.mockReturnValueOnce(inFlight);
stubConfirm(true);
const cuids = () => component.computingUnits.map(u => u.computingUnit.cuid);

component.ngOnInit();
// The next poll is now waiting on the backend.
vi.advanceTimersByTime(5000);
component.terminate(first);
expect(cuids()).toEqual([2]);

inFlight.next([first, second]);
inFlight.complete();
expect(cuids()).toEqual([2]);
});

// `switchMap` would cancel the slow response on every tick, so the table would never refresh.
it("lets a response slower than the interval finish instead of cancelling it", () => {
const requests: Subject<DashboardWorkflowComputingUnit[]>[] = [];
Expand Down Expand Up @@ -348,6 +386,71 @@ describe("AdminComputingUnitComponent", () => {
expect(specDetail()).not.toBeNull();
});

describe("terminating a unit", () => {
// The row's only `nz-button`: the expander is a plain button.
const terminateButtons = () =>
Array.from<HTMLButtonElement>(fixture.nativeElement.querySelectorAll("button[nz-button]"));
const first = makeUnit();
const second = makeUnit(withCu({ cuid: 2, name: "second" }));

it("gives every row a terminate button, labelled for its unit type", () => {
vi.mocked(service.listAllComputingUnits).mockReturnValue(of([first, localUnit(2)]));

fixture.detectChanges();

expect(terminateButtons().map(b => b.getAttribute("aria-label"))).toEqual([
"Terminate this computing unit",
"Disconnect from this computing unit",
]);
});

it("asks to confirm the termination of the unit on the clicked row", () => {
const confirm = stubConfirm();
vi.mocked(service.listAllComputingUnits).mockReturnValue(of([first, second]));
fixture.detectChanges();

terminateButtons()[1].click();

expect(confirm).toHaveBeenCalledTimes(1);
expect(confirm).toHaveBeenCalledWith(2, second, expect.any(Function));
});

it("drops only that row once the termination succeeds, without waiting for the next poll", () => {
stubConfirm(true);
vi.mocked(service.listAllComputingUnits).mockReturnValue(of([first, second]));
fixture.detectChanges();

terminateButtons()[0].click();
fixture.detectChanges();

expect(component.computingUnits.map(u => u.computingUnit.cuid)).toEqual([2]);
expect(dataRows()).toHaveLength(1);
});

// The modal can be cancelled and the request can fail, and in both the unit lives on.
it("keeps the row until the termination has succeeded", () => {
stubConfirm();
vi.mocked(service.listAllComputingUnits).mockReturnValue(of([first, second]));
fixture.detectChanges();

terminateButtons()[0].click();
fixture.detectChanges();

expect(dataRows()).toHaveLength(2);
});

// The actions column is the seventh after the expander, and the detail row must span all of them.
it("spans the expanded detail row across every column, the actions one included", () => {
vi.mocked(service.listAllComputingUnits).mockReturnValue(of([first]));
component.expandedCuids.add(first.computingUnit.cuid);

fixture.detectChanges();

const detailCell = specDetail()!.closest("td")!;
expect(detailCell.getAttribute("colspan")).toBe("7");
});
});

describe("resourceSummary", () => {
it("joins CPU, memory and GPU with a middot and labels", () => {
const unit = makeUnit(withResource({ gpuLimit: "1" }));
Expand Down
Loading
Loading