mengw15 commented on code in PR #7916:
URL: https://github.com/apache/texera/pull/7916#discussion_r3842359662
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts:
##########
@@ -2308,4 +2376,251 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(component.formTitle).toBe("untouched");
});
});
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Python UDF virtual-environment loading (rerenderEditorForm)
+ //
+ // The branch only runs for a Python UDF operator and the mock metadata has
+ // none, so the dynamic schema is stubbed rather than adding a fixture
operator
+ // type. Both collaborators emit synchronously, so nothing here waits on a
+ // timer or reaches a backend.
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("Python UDF environment loading", () => {
+ const udfSchema = () =>
+ ({
+ operatorType: "PythonUDFV2",
+ additionalMetadata: {
+ userFriendlyName: "Python UDF",
+ operatorDescription: "runs python",
+ operatorGroupName: "Python",
+ inputPorts: [],
+ outputPorts: [{}],
+ },
+ jsonSchema: {
+ type: "object",
+ properties: {
+ code: { type: "string" },
+ envName: { type: "string" },
+ defaultEnv: { type: "boolean" },
+ },
+ },
+ operatorVersion: "udf-1",
+ }) as any;
+
+ /** Points the frame at a Python UDF operator and returns the stubbed
collaborators. */
+ function selectUdfOperator(opts: { unit?: unknown; pves?: unknown;
predicate?: typeof mockScanPredicate } = {}): {
+ fetchPVEs: ReturnType<typeof vi.fn>;
+ notificationError: ReturnType<typeof vi.fn>;
+ } {
+ const predicate = opts.predicate ?? mockScanPredicate;
+ vi.spyOn(TestBed.inject(DynamicSchemaService),
"getDynamicSchema").mockReturnValue(udfSchema());
+ vi.spyOn(TestBed.inject(ComputingUnitStatusService),
"getSelectedComputingUnit").mockReturnValue(
+ of("unit" in opts ? opts.unit : { computingUnit: { cuid: 7 } }) as any
Review Comment:
Adopted in d6a90ff — replaced with a plain default-on-`undefined`:
```ts
// `undefined` means "not specified", so an explicit `null` still reaches
the component.
const unit = opts.unit === undefined ? { computingUnit: { cuid: 7 } } :
opts.unit;
```
That keeps the default-vs-explicit-`null` distinction the tests rely on and
reads without the `in` trick. (For the record, no call site passed `{ unit:
undefined }`, and the component's `unit?.computingUnit?.cuid` would treat it
the same as `null` anyway — but the readability point stands.)
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts:
##########
@@ -2308,4 +2376,251 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(component.formTitle).toBe("untouched");
});
});
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Python UDF virtual-environment loading (rerenderEditorForm)
+ //
+ // The branch only runs for a Python UDF operator and the mock metadata has
+ // none, so the dynamic schema is stubbed rather than adding a fixture
operator
+ // type. Both collaborators emit synchronously, so nothing here waits on a
+ // timer or reaches a backend.
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("Python UDF environment loading", () => {
+ const udfSchema = () =>
+ ({
+ operatorType: "PythonUDFV2",
+ additionalMetadata: {
+ userFriendlyName: "Python UDF",
+ operatorDescription: "runs python",
+ operatorGroupName: "Python",
+ inputPorts: [],
+ outputPorts: [{}],
+ },
+ jsonSchema: {
+ type: "object",
+ properties: {
+ code: { type: "string" },
+ envName: { type: "string" },
+ defaultEnv: { type: "boolean" },
+ },
+ },
+ operatorVersion: "udf-1",
+ }) as any;
+
+ /** Points the frame at a Python UDF operator and returns the stubbed
collaborators. */
+ function selectUdfOperator(opts: { unit?: unknown; pves?: unknown;
predicate?: typeof mockScanPredicate } = {}): {
+ fetchPVEs: ReturnType<typeof vi.fn>;
+ notificationError: ReturnType<typeof vi.fn>;
+ } {
+ const predicate = opts.predicate ?? mockScanPredicate;
+ vi.spyOn(TestBed.inject(DynamicSchemaService),
"getDynamicSchema").mockReturnValue(udfSchema());
+ vi.spyOn(TestBed.inject(ComputingUnitStatusService),
"getSelectedComputingUnit").mockReturnValue(
+ of("unit" in opts ? opts.unit : { computingUnit: { cuid: 7 } }) as any
+ );
+ const fetchPVEs = vi
+ .spyOn(TestBed.inject(WorkflowPveService), "fetchPVEs")
+ .mockReturnValue((opts.pves ?? of([{ pveName: "env-a" }, { pveName:
"env-b" }])) as any);
+ const notificationError = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+
+ workflowActionService.addOperator(predicate, mockPoint);
+ component.ngOnChanges({
+ currentOperatorId: new SimpleChange(undefined, predicate.operatorID,
true),
+ });
+ fixture.detectChanges();
+ return { fetchPVEs: fetchPVEs as any, notificationError:
notificationError as any };
+ }
+
+ const envField = () => component.formlyFields?.[0]?.fieldGroup?.find(f =>
f.key === "envName");
+
+ it("seeds defaultEnv when the operator's properties do not carry it", ()
=> {
+ selectUdfOperator();
+ expect(component.formData.defaultEnv).toBe(true);
+ });
+
+ it("leaves an explicit defaultEnv alone", () => {
+ selectUdfOperator({
+ predicate: {
+ ...mockScanPredicate,
+ operatorID: "udf-explicit-default",
+ operatorProperties: { defaultEnv: false },
+ },
+ });
+ expect(component.formData.defaultEnv).toBe(false);
+ });
+
+ it("fetches the selected unit's environments and binds them as envName
options", () => {
+ const { fetchPVEs } = selectUdfOperator();
+
+ expect(fetchPVEs).toHaveBeenCalledWith(7);
+ expect((envField()?.props as any).options).toEqual([
+ { value: "env-a", label: "env-a" },
+ { value: "env-b", label: "env-b" },
+ ]);
+ // hideEnvNameWhenDefaultEnvChecked also ran on the success path.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("skips the fetch when the emitted unit carries no cuid", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: { computingUnit: {} } });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ // The other arm supplies an empty list, so the field binds with no
options.
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("skips the fetch when no computing unit is selected", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: null });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("reports an Error failure and still binds the form with no
environments", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
new Error("pve down")) });
+
+ expect(notificationError).toHaveBeenCalledWith("Could not load Python
virtual environments: pve down");
+ expect((envField()?.props as any).options).toEqual([]);
+ // The fallback binding runs hideEnvNameWhenDefaultEnvChecked too.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("stringifies a non-Error failure", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
"plain string failure") });
+
+ expect(notificationError).toHaveBeenCalledWith(
+ "Could not load Python virtual environments: plain string failure"
+ );
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("patches nothing when the schema's properties are absent or not an
object", () => {
+ // Both take the guard's false side, so the clone comes back unchanged
+ // instead of dereferencing a missing envName property.
+ const noProps = (component as any).patchPythonUdfEnvironmentSchema({
type: "object" }, ["env-a"]);
+ expect(noProps).toEqual({ type: "object" });
+
+ const booleanProps = (component as
any).patchPythonUdfEnvironmentSchema({ type: "object", properties: true }, [
+ "env-a",
+ ]);
+ expect(booleanProps).toEqual({ type: "object", properties: true });
+ });
+
+ it("hideEnvNameWhenDefaultEnvChecked is a no-op when the form has no
envName field", () => {
+ component.setFormlyFormBinding({ type: "object", properties: { code: {
type: "string" } } });
+
+ expect(() => (component as
any).hideEnvNameWhenDefaultEnvChecked()).not.toThrow();
+
+ expect(component.formlyFields?.[0]?.fieldGroup?.find(f => f.key ===
"envName")).toBeUndefined();
+ // No other field picked up the defaultEnv hide rule either.
+ const codeExpressions = component.formlyFields?.[0]?.fieldGroup?.find(f
=> f.key === "code")?.expressions as any;
+ expect(codeExpressions?.hide).toBeUndefined();
+ });
+ });
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Early-return guards
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("early-return guards", () => {
+ it("ngOnChanges stops before re-rendering when the new operator id is
unset", () => {
+ const rerenderSpy = vi.spyOn(component, "rerenderEditorForm");
+
+ component.ngOnChanges({ currentOperatorId: new SimpleChange("op-1",
undefined, false) });
+
+ expect(component.currentOperatorId).toBeUndefined();
+ expect(rerenderSpy).not.toHaveBeenCalled();
+ });
+
+ it("the status-update subscription records the update for the selected
operator", () => {
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+ component.currentOperatorId = mockScanPredicate.operatorID;
+ fixture.detectChanges(); // ngOnInit registers the subscription
+
+ (TestBed.inject(WorkflowStatusService) as any).statusSubject.next({
+ [mockScanPredicate.operatorID]: { some: "status" },
+ });
Review Comment:
Adopted in d6a90ff, though not via a stub. This spec runs against the real
`WorkflowStatusService`, and ~196 existing tests depend on its real behaviour
(`rerenderEditorForm` reads `getCurrentStatus()`), so swapping in a mock for
the whole spec is a wide blast radius for a test-only change.
There is a public route that avoids the private field entirely:
`WorkflowStatusService` relays `OperatorStatisticsUpdateEvent` from
`WorkflowWebsocketService.websocketEvent()`, and that method returns the
subject itself. So the tests now feed the update in the way production does:
```ts
function emitOperatorStatistics(statistics: Record<string, unknown>): void {
(TestBed.inject(WorkflowWebsocketService).websocketEvent() as
Subject<TexeraWebsocketEvent>).next({
type: "OperatorStatisticsUpdateEvent",
operatorStatistics: statistics,
} as unknown as TexeraWebsocketEvent);
}
```
Better than a stub for this case: it also proves the websocket-to-subscriber
relay works, rather than asserting against a mock's own subject. Both tests
were re-checked against their failure path after the change so the refactor did
not turn them into no-ops.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts:
##########
@@ -2308,4 +2376,251 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(component.formTitle).toBe("untouched");
});
});
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Python UDF virtual-environment loading (rerenderEditorForm)
+ //
+ // The branch only runs for a Python UDF operator and the mock metadata has
+ // none, so the dynamic schema is stubbed rather than adding a fixture
operator
+ // type. Both collaborators emit synchronously, so nothing here waits on a
+ // timer or reaches a backend.
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("Python UDF environment loading", () => {
+ const udfSchema = () =>
+ ({
+ operatorType: "PythonUDFV2",
+ additionalMetadata: {
+ userFriendlyName: "Python UDF",
+ operatorDescription: "runs python",
+ operatorGroupName: "Python",
+ inputPorts: [],
+ outputPorts: [{}],
+ },
+ jsonSchema: {
+ type: "object",
+ properties: {
+ code: { type: "string" },
+ envName: { type: "string" },
+ defaultEnv: { type: "boolean" },
+ },
+ },
+ operatorVersion: "udf-1",
+ }) as any;
+
+ /** Points the frame at a Python UDF operator and returns the stubbed
collaborators. */
+ function selectUdfOperator(opts: { unit?: unknown; pves?: unknown;
predicate?: typeof mockScanPredicate } = {}): {
+ fetchPVEs: ReturnType<typeof vi.fn>;
+ notificationError: ReturnType<typeof vi.fn>;
+ } {
+ const predicate = opts.predicate ?? mockScanPredicate;
+ vi.spyOn(TestBed.inject(DynamicSchemaService),
"getDynamicSchema").mockReturnValue(udfSchema());
+ vi.spyOn(TestBed.inject(ComputingUnitStatusService),
"getSelectedComputingUnit").mockReturnValue(
+ of("unit" in opts ? opts.unit : { computingUnit: { cuid: 7 } }) as any
+ );
+ const fetchPVEs = vi
+ .spyOn(TestBed.inject(WorkflowPveService), "fetchPVEs")
+ .mockReturnValue((opts.pves ?? of([{ pveName: "env-a" }, { pveName:
"env-b" }])) as any);
+ const notificationError = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+
+ workflowActionService.addOperator(predicate, mockPoint);
+ component.ngOnChanges({
+ currentOperatorId: new SimpleChange(undefined, predicate.operatorID,
true),
+ });
+ fixture.detectChanges();
+ return { fetchPVEs: fetchPVEs as any, notificationError:
notificationError as any };
+ }
+
+ const envField = () => component.formlyFields?.[0]?.fieldGroup?.find(f =>
f.key === "envName");
+
+ it("seeds defaultEnv when the operator's properties do not carry it", ()
=> {
+ selectUdfOperator();
+ expect(component.formData.defaultEnv).toBe(true);
+ });
+
+ it("leaves an explicit defaultEnv alone", () => {
+ selectUdfOperator({
+ predicate: {
+ ...mockScanPredicate,
+ operatorID: "udf-explicit-default",
+ operatorProperties: { defaultEnv: false },
+ },
+ });
+ expect(component.formData.defaultEnv).toBe(false);
+ });
+
+ it("fetches the selected unit's environments and binds them as envName
options", () => {
+ const { fetchPVEs } = selectUdfOperator();
+
+ expect(fetchPVEs).toHaveBeenCalledWith(7);
+ expect((envField()?.props as any).options).toEqual([
+ { value: "env-a", label: "env-a" },
+ { value: "env-b", label: "env-b" },
+ ]);
+ // hideEnvNameWhenDefaultEnvChecked also ran on the success path.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("skips the fetch when the emitted unit carries no cuid", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: { computingUnit: {} } });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ // The other arm supplies an empty list, so the field binds with no
options.
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("skips the fetch when no computing unit is selected", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: null });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("reports an Error failure and still binds the form with no
environments", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
new Error("pve down")) });
+
+ expect(notificationError).toHaveBeenCalledWith("Could not load Python
virtual environments: pve down");
+ expect((envField()?.props as any).options).toEqual([]);
+ // The fallback binding runs hideEnvNameWhenDefaultEnvChecked too.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("stringifies a non-Error failure", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
"plain string failure") });
+
+ expect(notificationError).toHaveBeenCalledWith(
+ "Could not load Python virtual environments: plain string failure"
+ );
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("patches nothing when the schema's properties are absent or not an
object", () => {
+ // Both take the guard's false side, so the clone comes back unchanged
+ // instead of dereferencing a missing envName property.
+ const noProps = (component as any).patchPythonUdfEnvironmentSchema({
type: "object" }, ["env-a"]);
+ expect(noProps).toEqual({ type: "object" });
+
+ const booleanProps = (component as
any).patchPythonUdfEnvironmentSchema({ type: "object", properties: true }, [
+ "env-a",
+ ]);
+ expect(booleanProps).toEqual({ type: "object", properties: true });
+ });
+
+ it("hideEnvNameWhenDefaultEnvChecked is a no-op when the form has no
envName field", () => {
+ component.setFormlyFormBinding({ type: "object", properties: { code: {
type: "string" } } });
+
+ expect(() => (component as
any).hideEnvNameWhenDefaultEnvChecked()).not.toThrow();
+
+ expect(component.formlyFields?.[0]?.fieldGroup?.find(f => f.key ===
"envName")).toBeUndefined();
+ // No other field picked up the defaultEnv hide rule either.
+ const codeExpressions = component.formlyFields?.[0]?.fieldGroup?.find(f
=> f.key === "code")?.expressions as any;
+ expect(codeExpressions?.hide).toBeUndefined();
+ });
+ });
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Early-return guards
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("early-return guards", () => {
+ it("ngOnChanges stops before re-rendering when the new operator id is
unset", () => {
+ const rerenderSpy = vi.spyOn(component, "rerenderEditorForm");
+
+ component.ngOnChanges({ currentOperatorId: new SimpleChange("op-1",
undefined, false) });
+
+ expect(component.currentOperatorId).toBeUndefined();
+ expect(rerenderSpy).not.toHaveBeenCalled();
+ });
+
+ it("the status-update subscription records the update for the selected
operator", () => {
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+ component.currentOperatorId = mockScanPredicate.operatorID;
+ fixture.detectChanges(); // ngOnInit registers the subscription
+
+ (TestBed.inject(WorkflowStatusService) as any).statusSubject.next({
+ [mockScanPredicate.operatorID]: { some: "status" },
+ });
+
+ expect(component.currentOperatorStatus).toEqual({ some: "status" });
+ });
+
+ it("the status-update subscription ignores updates while no operator is
selected", () => {
+ fixture.detectChanges(); // ngOnInit registers the subscription
+ component.currentOperatorId = undefined;
+
+ // getStatusUpdateStream() exposes a read-only view, so drive the
subject behind it.
+ (TestBed.inject(WorkflowStatusService) as any).statusSubject.next({
"op-1": { some: "status" } });
Review Comment:
Adopted in d6a90ff, though not via a stub. This spec runs against the real
`WorkflowStatusService`, and ~196 existing tests depend on its real behaviour
(`rerenderEditorForm` reads `getCurrentStatus()`), so swapping in a mock for
the whole spec is a wide blast radius for a test-only change.
There is a public route that avoids the private field entirely:
`WorkflowStatusService` relays `OperatorStatisticsUpdateEvent` from
`WorkflowWebsocketService.websocketEvent()`, and that method returns the
subject itself. So the tests now feed the update in the way production does:
```ts
function emitOperatorStatistics(statistics: Record<string, unknown>): void {
(TestBed.inject(WorkflowWebsocketService).websocketEvent() as
Subject<TexeraWebsocketEvent>).next({
type: "OperatorStatisticsUpdateEvent",
operatorStatistics: statistics,
} as unknown as TexeraWebsocketEvent);
}
```
Better than a stub for this case: it also proves the websocket-to-subscriber
relay works, rather than asserting against a mock's own subject. Both tests
were re-checked against their failure path after the change so the refactor did
not turn them into no-ops.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts:
##########
@@ -2308,4 +2376,251 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(component.formTitle).toBe("untouched");
});
});
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Python UDF virtual-environment loading (rerenderEditorForm)
+ //
+ // The branch only runs for a Python UDF operator and the mock metadata has
+ // none, so the dynamic schema is stubbed rather than adding a fixture
operator
+ // type. Both collaborators emit synchronously, so nothing here waits on a
+ // timer or reaches a backend.
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("Python UDF environment loading", () => {
+ const udfSchema = () =>
+ ({
+ operatorType: "PythonUDFV2",
+ additionalMetadata: {
+ userFriendlyName: "Python UDF",
+ operatorDescription: "runs python",
+ operatorGroupName: "Python",
+ inputPorts: [],
+ outputPorts: [{}],
+ },
+ jsonSchema: {
+ type: "object",
+ properties: {
+ code: { type: "string" },
+ envName: { type: "string" },
+ defaultEnv: { type: "boolean" },
+ },
+ },
+ operatorVersion: "udf-1",
+ }) as any;
+
+ /** Points the frame at a Python UDF operator and returns the stubbed
collaborators. */
+ function selectUdfOperator(opts: { unit?: unknown; pves?: unknown;
predicate?: typeof mockScanPredicate } = {}): {
+ fetchPVEs: ReturnType<typeof vi.fn>;
+ notificationError: ReturnType<typeof vi.fn>;
+ } {
+ const predicate = opts.predicate ?? mockScanPredicate;
+ vi.spyOn(TestBed.inject(DynamicSchemaService),
"getDynamicSchema").mockReturnValue(udfSchema());
+ vi.spyOn(TestBed.inject(ComputingUnitStatusService),
"getSelectedComputingUnit").mockReturnValue(
+ of("unit" in opts ? opts.unit : { computingUnit: { cuid: 7 } }) as any
+ );
+ const fetchPVEs = vi
+ .spyOn(TestBed.inject(WorkflowPveService), "fetchPVEs")
+ .mockReturnValue((opts.pves ?? of([{ pveName: "env-a" }, { pveName:
"env-b" }])) as any);
+ const notificationError = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+
+ workflowActionService.addOperator(predicate, mockPoint);
+ component.ngOnChanges({
+ currentOperatorId: new SimpleChange(undefined, predicate.operatorID,
true),
+ });
+ fixture.detectChanges();
+ return { fetchPVEs: fetchPVEs as any, notificationError:
notificationError as any };
+ }
+
+ const envField = () => component.formlyFields?.[0]?.fieldGroup?.find(f =>
f.key === "envName");
+
+ it("seeds defaultEnv when the operator's properties do not carry it", ()
=> {
+ selectUdfOperator();
+ expect(component.formData.defaultEnv).toBe(true);
+ });
+
+ it("leaves an explicit defaultEnv alone", () => {
+ selectUdfOperator({
+ predicate: {
+ ...mockScanPredicate,
+ operatorID: "udf-explicit-default",
+ operatorProperties: { defaultEnv: false },
+ },
+ });
+ expect(component.formData.defaultEnv).toBe(false);
+ });
+
+ it("fetches the selected unit's environments and binds them as envName
options", () => {
+ const { fetchPVEs } = selectUdfOperator();
+
+ expect(fetchPVEs).toHaveBeenCalledWith(7);
+ expect((envField()?.props as any).options).toEqual([
+ { value: "env-a", label: "env-a" },
+ { value: "env-b", label: "env-b" },
+ ]);
+ // hideEnvNameWhenDefaultEnvChecked also ran on the success path.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("skips the fetch when the emitted unit carries no cuid", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: { computingUnit: {} } });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ // The other arm supplies an empty list, so the field binds with no
options.
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("skips the fetch when no computing unit is selected", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: null });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("reports an Error failure and still binds the form with no
environments", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
new Error("pve down")) });
+
+ expect(notificationError).toHaveBeenCalledWith("Could not load Python
virtual environments: pve down");
+ expect((envField()?.props as any).options).toEqual([]);
+ // The fallback binding runs hideEnvNameWhenDefaultEnvChecked too.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("stringifies a non-Error failure", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
"plain string failure") });
+
+ expect(notificationError).toHaveBeenCalledWith(
+ "Could not load Python virtual environments: plain string failure"
+ );
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("patches nothing when the schema's properties are absent or not an
object", () => {
+ // Both take the guard's false side, so the clone comes back unchanged
+ // instead of dereferencing a missing envName property.
+ const noProps = (component as any).patchPythonUdfEnvironmentSchema({
type: "object" }, ["env-a"]);
+ expect(noProps).toEqual({ type: "object" });
+
+ const booleanProps = (component as
any).patchPythonUdfEnvironmentSchema({ type: "object", properties: true }, [
+ "env-a",
+ ]);
+ expect(booleanProps).toEqual({ type: "object", properties: true });
+ });
+
+ it("hideEnvNameWhenDefaultEnvChecked is a no-op when the form has no
envName field", () => {
+ component.setFormlyFormBinding({ type: "object", properties: { code: {
type: "string" } } });
+
+ expect(() => (component as
any).hideEnvNameWhenDefaultEnvChecked()).not.toThrow();
+
Review Comment:
Leaving these as they are. `patchPythonUdfEnvironmentSchema` and
`hideEnvNameWhenDefaultEnvChecked` are private, and the `describe("Python UDF
environment schema patching")` block already in this file — added by an earlier
coverage pass — reaches them the same way, so this follows the local convention
rather than introducing it.
The alternatives both cost more than the coupling: extracting them into a
standalone utility is a production change, which is out of scope for a
test-only PR, and going purely through `rerenderEditorForm` cannot reach the
two guards these tests exist for (a schema with no `properties`, and one where
`properties` is a boolean) because the real schema always supplies an object.
The observable-behaviour paths *are* covered separately in the same PR — the
success and failure branches assert through the rendered `envName` field's
`props.options` and `expressions`.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts:
##########
@@ -2308,4 +2376,251 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(component.formTitle).toBe("untouched");
});
});
+
+ // ──────────────────────────────────────────────────────────────────────────
+ // Python UDF virtual-environment loading (rerenderEditorForm)
+ //
+ // The branch only runs for a Python UDF operator and the mock metadata has
+ // none, so the dynamic schema is stubbed rather than adding a fixture
operator
+ // type. Both collaborators emit synchronously, so nothing here waits on a
+ // timer or reaches a backend.
+ // ──────────────────────────────────────────────────────────────────────────
+ describe("Python UDF environment loading", () => {
+ const udfSchema = () =>
+ ({
+ operatorType: "PythonUDFV2",
+ additionalMetadata: {
+ userFriendlyName: "Python UDF",
+ operatorDescription: "runs python",
+ operatorGroupName: "Python",
+ inputPorts: [],
+ outputPorts: [{}],
+ },
+ jsonSchema: {
+ type: "object",
+ properties: {
+ code: { type: "string" },
+ envName: { type: "string" },
+ defaultEnv: { type: "boolean" },
+ },
+ },
+ operatorVersion: "udf-1",
+ }) as any;
+
+ /** Points the frame at a Python UDF operator and returns the stubbed
collaborators. */
+ function selectUdfOperator(opts: { unit?: unknown; pves?: unknown;
predicate?: typeof mockScanPredicate } = {}): {
+ fetchPVEs: ReturnType<typeof vi.fn>;
+ notificationError: ReturnType<typeof vi.fn>;
+ } {
+ const predicate = opts.predicate ?? mockScanPredicate;
+ vi.spyOn(TestBed.inject(DynamicSchemaService),
"getDynamicSchema").mockReturnValue(udfSchema());
+ vi.spyOn(TestBed.inject(ComputingUnitStatusService),
"getSelectedComputingUnit").mockReturnValue(
+ of("unit" in opts ? opts.unit : { computingUnit: { cuid: 7 } }) as any
+ );
+ const fetchPVEs = vi
+ .spyOn(TestBed.inject(WorkflowPveService), "fetchPVEs")
+ .mockReturnValue((opts.pves ?? of([{ pveName: "env-a" }, { pveName:
"env-b" }])) as any);
+ const notificationError = vi
+ .spyOn(TestBed.inject(NotificationService), "error")
+ .mockImplementation(() => undefined as any);
+
+ workflowActionService.addOperator(predicate, mockPoint);
+ component.ngOnChanges({
+ currentOperatorId: new SimpleChange(undefined, predicate.operatorID,
true),
+ });
+ fixture.detectChanges();
+ return { fetchPVEs: fetchPVEs as any, notificationError:
notificationError as any };
+ }
+
+ const envField = () => component.formlyFields?.[0]?.fieldGroup?.find(f =>
f.key === "envName");
+
+ it("seeds defaultEnv when the operator's properties do not carry it", ()
=> {
+ selectUdfOperator();
+ expect(component.formData.defaultEnv).toBe(true);
+ });
+
+ it("leaves an explicit defaultEnv alone", () => {
+ selectUdfOperator({
+ predicate: {
+ ...mockScanPredicate,
+ operatorID: "udf-explicit-default",
+ operatorProperties: { defaultEnv: false },
+ },
+ });
+ expect(component.formData.defaultEnv).toBe(false);
+ });
+
+ it("fetches the selected unit's environments and binds them as envName
options", () => {
+ const { fetchPVEs } = selectUdfOperator();
+
+ expect(fetchPVEs).toHaveBeenCalledWith(7);
+ expect((envField()?.props as any).options).toEqual([
+ { value: "env-a", label: "env-a" },
+ { value: "env-b", label: "env-b" },
+ ]);
+ // hideEnvNameWhenDefaultEnvChecked also ran on the success path.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("skips the fetch when the emitted unit carries no cuid", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: { computingUnit: {} } });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ // The other arm supplies an empty list, so the field binds with no
options.
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("skips the fetch when no computing unit is selected", () => {
+ const { fetchPVEs } = selectUdfOperator({ unit: null });
+
+ expect(fetchPVEs).not.toHaveBeenCalled();
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("reports an Error failure and still binds the form with no
environments", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
new Error("pve down")) });
+
+ expect(notificationError).toHaveBeenCalledWith("Could not load Python
virtual environments: pve down");
+ expect((envField()?.props as any).options).toEqual([]);
+ // The fallback binding runs hideEnvNameWhenDefaultEnvChecked too.
+ expect((envField()?.expressions as
any).hide).toBe("!!field.parent.model.defaultEnv");
+ });
+
+ it("stringifies a non-Error failure", () => {
+ const { notificationError } = selectUdfOperator({ pves: throwError(() =>
"plain string failure") });
+
+ expect(notificationError).toHaveBeenCalledWith(
+ "Could not load Python virtual environments: plain string failure"
+ );
+ expect((envField()?.props as any).options).toEqual([]);
+ });
+
+ it("patches nothing when the schema's properties are absent or not an
object", () => {
+ // Both take the guard's false side, so the clone comes back unchanged
+ // instead of dereferencing a missing envName property.
+ const noProps = (component as any).patchPythonUdfEnvironmentSchema({
type: "object" }, ["env-a"]);
+ expect(noProps).toEqual({ type: "object" });
+
+ const booleanProps = (component as
any).patchPythonUdfEnvironmentSchema({ type: "object", properties: true }, [
+ "env-a",
+ ]);
+ expect(booleanProps).toEqual({ type: "object", properties: true });
+ });
+
+ it("hideEnvNameWhenDefaultEnvChecked is a no-op when the form has no
envName field", () => {
+ component.setFormlyFormBinding({ type: "object", properties: { code: {
type: "string" } } });
+
+ expect(() => (component as
any).hideEnvNameWhenDefaultEnvChecked()).not.toThrow();
+
Review Comment:
Leaving these as they are. `patchPythonUdfEnvironmentSchema` and
`hideEnvNameWhenDefaultEnvChecked` are private, and the `describe("Python UDF
environment schema patching")` block already in this file — added by an earlier
coverage pass — reaches them the same way, so this follows the local convention
rather than introducing it.
The alternatives both cost more than the coupling: extracting them into a
standalone utility is a production change, which is out of scope for a
test-only PR, and going purely through `rerenderEditorForm` cannot reach the
two guards these tests exist for (a schema with no `properties`, and one where
`properties` is a boolean) because the real schema always supplies an object.
The observable-behaviour paths *are* covered separately in the same PR — the
success and failure branches assert through the rendered `envName` field's
`props.options` and `expressions`.
--
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]
