Copilot commented on code in PR #5004:
URL: https://github.com/apache/texera/pull/5004#discussion_r3222481145
##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts:
##########
@@ -24,40 +24,150 @@ import { WorkflowActionService } from
"../../service/workflow-graph/model/workfl
import { mockJavaUDFPredicate, mockPoint } from
"../../service/workflow-graph/model/mock-workflow-data";
import { OperatorMetadataService } from
"../../service/operator-metadata/operator-metadata.service";
import { StubOperatorMetadataService } from
"../../service/operator-metadata/stub-operator-metadata.service";
+import { mockOperatorMetaData } from
"../../service/operator-metadata/mock-operator-metadata.data";
import { commonTestProviders } from "../../../common/testing/test-utils";
+import { OperatorPredicate } from "../../types/workflow-common.interface";
+import { OperatorSchema } from "../../types/operator-schema.interface";
+import { of } from "rxjs";
-describe("CodeEditorDialogComponent", () => {
- let component: CodeEditorComponent;
- let fixture: ComponentFixture<CodeEditorComponent>;
+// Operator types that the constructor's language-detection branch must map
+// to `python`. Anything else falls through to `java`. Local to this spec so
+// we don't perturb the shared mock-workflow-data fixtures.
+const PYTHON_OPERATOR_TYPES = ["PythonUDFV2", "PythonUDFSourceV2",
"DualInputPortsPythonUDFV2"];
Review Comment:
The spec comment claims the constructor language detection maps the listed
Python operator types to `python` and that “anything else falls through to
`java`”, but the component also has an `r` branch for `RUDFSource` / `RUDF`
(code-editor.component.ts:147-156). Consider updating the comment (and ideally
add a small test case for one RUDF type) so the language-detection coverage
matches the actual branches.
##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts:
##########
@@ -24,40 +24,150 @@ import { WorkflowActionService } from
"../../service/workflow-graph/model/workfl
import { mockJavaUDFPredicate, mockPoint } from
"../../service/workflow-graph/model/mock-workflow-data";
import { OperatorMetadataService } from
"../../service/operator-metadata/operator-metadata.service";
import { StubOperatorMetadataService } from
"../../service/operator-metadata/stub-operator-metadata.service";
+import { mockOperatorMetaData } from
"../../service/operator-metadata/mock-operator-metadata.data";
import { commonTestProviders } from "../../../common/testing/test-utils";
+import { OperatorPredicate } from "../../types/workflow-common.interface";
+import { OperatorSchema } from "../../types/operator-schema.interface";
+import { of } from "rxjs";
-describe("CodeEditorDialogComponent", () => {
- let component: CodeEditorComponent;
- let fixture: ComponentFixture<CodeEditorComponent>;
+// Operator types that the constructor's language-detection branch must map
+// to `python`. Anything else falls through to `java`. Local to this spec so
+// we don't perturb the shared mock-workflow-data fixtures.
+const PYTHON_OPERATOR_TYPES = ["PythonUDFV2", "PythonUDFSourceV2",
"DualInputPortsPythonUDFV2"];
+
+// Augment `mockOperatorMetaData` with synthetic schemas for the V2 operator
+// types and one unknown type so `addOperator` and `JointUIService` accept
+// them. Cloning the existing `PythonUDF` schema and renaming the
+// `operatorType` is the cheapest way to satisfy both `operatorTypeExists`
+// and the schema-driven joint element creation.
+const baseSchema = mockOperatorMetaData.operators.find(op => op.operatorType
=== "PythonUDF");
+if (!baseSchema) {
+ throw new Error(
+ "CodeEditorComponent spec setup expected a PythonUDF schema in
mockOperatorMetaData — fixture has drifted."
+ );
+}
+const synthesizeSchema = (operatorType: string): OperatorSchema => ({
...baseSchema, operatorType });
+const augmentedSchemas: OperatorSchema[] = [
+ ...mockOperatorMetaData.operators,
+ ...PYTHON_OPERATOR_TYPES.map(synthesizeSchema),
+ synthesizeSchema("SomeUnknownType"),
+];
+class AugmentedStubMetadataService extends StubOperatorMetadataService {
+ // JointUIService snapshots `operatorSchemas` from this stream once on
+ // construction, so we have to feed it the augmented list (overriding only
+ // `getOperatorSchema`/`operatorTypeExists` is not enough).
+ private readonly augmentedMetadata = of({
+ ...mockOperatorMetaData,
+ operators: augmentedSchemas,
+ });
+ override getOperatorMetadata(): typeof this.augmentedMetadata {
+ return this.augmentedMetadata;
+ }
+ override getOperatorSchema(operatorType: string): OperatorSchema {
+ const schema = augmentedSchemas.find(op => op.operatorType ===
operatorType);
+ if (!schema) throw new Error(`unknown operatorType ${operatorType}`);
+ return schema;
+ }
+ override operatorTypeExists(operatorType: string): boolean {
+ return augmentedSchemas.some(op => op.operatorType === operatorType);
+ }
+}
+
+const buildPredicate = (operatorID: string, operatorType: string):
OperatorPredicate => ({
+ operatorID,
+ operatorType,
+ operatorVersion: "p1",
+ operatorProperties: {},
+ inputPorts: [{ portID: "input-0" }],
+ outputPorts: [{ portID: "output-0" }],
+ showAdvanced: false,
+ isDisabled: false,
+});
+
+describe("CodeEditorComponent", () => {
let workflowActionService: WorkflowActionService;
beforeEach(async () => {
await TestBed.configureTestingModule({
providers: [
WorkflowActionService,
- {
- provide: OperatorMetadataService,
- useClass: StubOperatorMetadataService,
- },
+ { provide: OperatorMetadataService, useClass:
AugmentedStubMetadataService },
...commonTestProviders,
],
imports: [CodeEditorComponent, HttpClientTestingModule],
}).compileComponents();
workflowActionService = TestBed.inject(WorkflowActionService);
- workflowActionService.addOperator(mockJavaUDFPredicate, mockPoint);
-
workflowActionService.getJointGraphWrapper().highlightOperators(mockJavaUDFPredicate.operatorID);
- fixture = TestBed.createComponent(CodeEditorComponent);
- component = fixture.componentInstance;
+ });
+
+ function makeFixture(predicate: OperatorPredicate):
ComponentFixture<CodeEditorComponent> {
+ workflowActionService.addOperator(predicate, mockPoint);
+
workflowActionService.getJointGraphWrapper().highlightOperators(predicate.operatorID);
+ const fixture = TestBed.createComponent(CodeEditorComponent);
fixture.detectChanges();
+ return fixture;
+ }
+
+ it("creates with the highlighted operator", () => {
+ const fixture = makeFixture(mockJavaUDFPredicate);
+ expect(fixture.componentInstance).toBeTruthy();
+
expect(fixture.componentInstance.currentOperatorId).toBe(mockJavaUDFPredicate.operatorID);
});
- it("should create", () => {
- expect(component).toBeTruthy();
+ // Language detection — the three V2-era operator types must always pick
+ // `python`, and any other type must pick `java`. The exact branch lives
+ // in the constructor; the public `language` field is what the rest of
+ // the editor (LSP wiring, file-suffix selection) keys off.
+
+ PYTHON_OPERATOR_TYPES.forEach((operatorType, index) => {
+ it(`picks language="python" for operatorType=${operatorType}`, () => {
+ const fixture = makeFixture(buildPredicate(`p-${index}`, operatorType));
+ expect(fixture.componentInstance.language).toBe("python");
+ expect(fixture.componentInstance.languageTitle).toBe("Python UDF");
+ });
});
- // it("should create a websocket when the editor is opened", () => {
- // let socketInstance = component.getLanguageServerSocket();
- // expect(socketInstance).toBeTruthy();
- // });
+ it('picks language="java" for plain JavaUDF', () => {
+ const fixture = makeFixture(mockJavaUDFPredicate);
+ expect(fixture.componentInstance.language).toBe("java");
+ expect(fixture.componentInstance.languageTitle).toBe("Java UDF");
+ });
+
+ it('picks language="java" for unknown operator types', () => {
+ const fixture = makeFixture(buildPredicate("u-0", "SomeUnknownType"));
+ expect(fixture.componentInstance.language).toBe("java");
+ expect(fixture.componentInstance.languageTitle).toBe("Java UDF");
+ });
+
+ it("derives languageTitle as Capitalized(language) + ' UDF'", () => {
+ const fixture = makeFixture(buildPredicate("p-x", "PythonUDFV2"));
+ const c = fixture.componentInstance;
+ // Independent re-derivation matches whatever the component computed.
+ const expected = `${c.language[0].toUpperCase()}${c.language.slice(1)}
UDF`;
+ expect(c.languageTitle).toBe(expected);
+ });
+
+ // Coeditor cursor styles — getCoeditorCursorStyles takes the awareness-
+ // sourced clientId + colour and returns a SafeStyle. We assert the wrapper
+ // shape (truthy DomSanitizer-wrapped object) for valid inputs. Exact CSS
+ // contents are sanitizer-internal and differ across builds, so we don't
+ // pin them here.
+
+ it("produces a SafeStyle for a coeditor with a numeric clientId and a hex
colour", () => {
+ const fixture = makeFixture(mockJavaUDFPredicate);
Review Comment:
The coeditor cursor style assertions/comments refer to a “SafeStyle”, but
`getCoeditorCursorStyles` returns `DomSanitizer.bypassSecurityTrustHtml(...)`
and is consumed via `[innerHTML]` in the template, i.e., it’s a SafeHtml value.
Renaming the comment/test descriptions to match the actual return kind will
avoid confusion for future maintainers.
--
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]
