Copilot commented on code in PR #7916:
URL: https://github.com/apache/texera/pull/7916#discussion_r3842285149
##########
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:
The `"unit" in opts` pattern is non-obvious here and will also treat `{
unit: undefined }` as “provided”, resulting in `of(undefined)` (different from
the default unit and different from the explicit `null` test case). Consider
using an explicit `hasOwn` check (or similar) to preserve the “default vs
explicit null” intent while avoiding accidental `undefined` propagation.
##########
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:
These tests reach into `WorkflowStatusService` internals via `as any` to
access `statusSubject`. That couples the tests to private implementation
details and can break with refactors that keep the public API stable. Prefer
providing a dedicated mock/stub `WorkflowStatusService` in the TestBed where
the `Subject` is part of the mock’s public surface (or emitting through a
public test hook/API if available), so the test depends only on an explicit
contract.
##########
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:
These tests reach into `WorkflowStatusService` internals via `as any` to
access `statusSubject`. That couples the tests to private implementation
details and can break with refactors that keep the public API stable. Prefer
providing a dedicated mock/stub `WorkflowStatusService` in the TestBed where
the `Subject` is part of the mock’s public surface (or emitting through a
public test hook/API if available), so the test depends only on an explicit
contract.
##########
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:
These assertions call component internals via `(component as any)`, which
makes the tests more brittle to refactors (e.g., renames/visibility changes)
even when behavior remains correct. If the goal is to validate externally
observable behavior, consider asserting through
`rerenderEditorForm`/`setFormlyFormBinding` outcomes only; if direct unit
testing of the helper is required, consider extracting the logic into a
standalone utility (or otherwise exposing a stable test surface) to reduce
coupling to private methods.
##########
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:
These assertions call component internals via `(component as any)`, which
makes the tests more brittle to refactors (e.g., renames/visibility changes)
even when behavior remains correct. If the goal is to validate externally
observable behavior, consider asserting through
`rerenderEditorForm`/`setFormlyFormBinding` outcomes only; if direct unit
testing of the helper is required, consider extracting the logic into a
standalone utility (or otherwise exposing a stable test surface) to reduce
coupling to private methods.
--
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]
