Copilot commented on code in PR #6602:
URL: https://github.com/apache/texera/pull/6602#discussion_r3613824888
##########
frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.spec.ts:
##########
@@ -20,9 +20,11 @@
import { TestBed } from "@angular/core/testing";
import { HttpClient } from "@angular/common/http";
import { HttpClientTestingModule, HttpTestingController } from
"@angular/common/http/testing";
-import { OperatorMetadataService } from "./operator-metadata.service";
+import { OPERATOR_METADATA_ENDPOINT, OperatorMetadataService } from
"./operator-metadata.service";
import { mockOperatorMetaData } from "./mock-operator-metadata.data";
import { commonTestProviders } from "../../../common/testing/test-utils";
+import { AppSettings } from "../../../common/app-setting";
+import { OperatorMetadata } from "../../types/operator-schema.interface";
Review Comment:
`OperatorMetadata` is only used as a TypeScript type in this spec. Importing
it as a value can generate an unnecessary runtime import and is inconsistent
with other specs that use `import type` for type-only dependencies.
##########
frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.spec.ts:
##########
@@ -57,4 +59,62 @@ describe("OperatorMetadataService", () => {
const req = httpTestingController.match(request => request.method ===
"GET");
req[0].flush(mockOperatorMetaData);
});
+
+ const metadataUrl =
`${AppSettings.getApiEndpoint()}/${OPERATOR_METADATA_ENDPOINT}`;
+
+ // The service fetches metadata once in its constructor (shareReplay), so a
single
+ // request is pending after injection; flush it with the fixture.
+ const flushMetadata = () =>
+ httpTestingController.expectOne(req => req.method === "GET" && req.url ===
metadataUrl).flush(mockOperatorMetaData);
+
+ it("getOperatorMetadata emits the fetched metadata to subscribers", () => {
+ let emitted: OperatorMetadata | undefined;
+ service.getOperatorMetadata().subscribe(m => (emitted = m));
+ flushMetadata();
+ expect(emitted).toEqual(mockOperatorMetaData);
+ });
+
+ it("getOperatorSchema returns the schema for a known operator type", () => {
+ flushMetadata();
+ const schema = service.getOperatorSchema("ScanSource");
+ expect(schema.operatorType).toBe("ScanSource");
+ expect(schema).toBe(mockOperatorMetaData.operators.find(op =>
op.operatorType === "ScanSource"));
+ });
Review Comment:
This assertion uses `toBe(...)` (reference equality) against an object
returned from `HttpTestingController.flush(...)`. That couples the test to
object identity and can become brittle if the HTTP testing layer
clones/serializes the response. Prefer deep equality (`toEqual`) or compare
stable fields.
--
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]
