aglinxinyuan commented on code in PR #5004:
URL: https://github.com/apache/texera/pull/5004#discussion_r3222451355
##########
frontend/src/app/dashboard/service/user/download/download.service.spec.ts:
##########
@@ -61,198 +58,219 @@ describe("DownloadService", () => {
datasetServiceSpy = TestBed.inject(DatasetService) as unknown as
Mocked<DatasetService>;
fileSaverServiceSpy = TestBed.inject(FileSaverService) as unknown as
Mocked<FileSaverService>;
notificationServiceSpy = TestBed.inject(NotificationService) as unknown as
Mocked<NotificationService>;
+ workflowPersistServiceSpy = TestBed.inject(WorkflowPersistService) as
unknown as Mocked<WorkflowPersistService>;
+ httpMock = TestBed.inject(HttpTestingController);
});
- it.skip("should download a single file successfully", () => {
- const filePath = "test/file.txt";
+ // ─── downloadSingleFile ───────────────────────────────────────────────────
+
+ it("downloads a single file and saves it under the basename of the path",
async () => {
const mockBlob = new Blob(["test content"], { type: "text/plain" });
+
datasetServiceSpy.retrieveDatasetVersionSingleFile.mockReturnValue(of(mockBlob));
+ const result = await
firstValueFrom(downloadService.downloadSingleFile("test/file.txt", true));
+
+ expect(result).toBe(mockBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
+
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith("test/file.txt",
true);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"file.txt");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("File
test/file.txt has been downloaded");
+ });
+
+ it("falls back to a default filename when the path has no basename segment",
async () => {
+ const mockBlob = new Blob(["x"], { type: "text/plain" });
datasetServiceSpy.retrieveDatasetVersionSingleFile.mockReturnValue(of(mockBlob));
- downloadService.downloadSingleFile(filePath, true).subscribe({
- next: blob => {
- expect(blob).toBe(mockBlob);
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
-
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath,
true);
- expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"file.txt");
- expect(notificationServiceSpy.success).toHaveBeenCalledWith("File
test/file.txt has been downloaded");
- done();
- },
- error: (error: unknown) => {
- fail("Should not have thrown an error: " + error);
- },
- });
+ await firstValueFrom(downloadService.downloadSingleFile("", true));
+
+ // path.split("/").pop() returns "" for "", which falls through to the
default name
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"download");
});
- it.skip("should handle download failure correctly", () => {
- const filePath = "test/file.txt";
- const errorMessage = "Download failed";
-
-
datasetServiceSpy.retrieveDatasetVersionSingleFile.mockReturnValue(throwError(()
=> new Error(errorMessage)));
-
- downloadService.downloadSingleFile(filePath, true).subscribe({
- next: () => {
- fail("Should have thrown an error");
- },
- error: (error: unknown) => {
- expect(error).toBeTruthy();
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
-
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith(filePath,
true);
- expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
- expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading file 'test/file.txt'");
- done();
- },
- });
+ it("propagates errors from downloadSingleFile and emits the error
notification", async () => {
+
datasetServiceSpy.retrieveDatasetVersionSingleFile.mockReturnValue(throwError(()
=> new Error("boom")));
+
+ await
expect(firstValueFrom(downloadService.downloadSingleFile("test/file.txt",
true))).rejects.toThrow("boom");
+
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download file test/file.txt");
+ expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
+ expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading file 'test/file.txt'");
});
- it.skip("should download a dataset successfully", () => {
- const datasetId = 1;
- const datasetName = "TestDataset";
- const mockBlob = new Blob(["dataset content"], { type: "application/zip"
});
+ it("passes isLogin=false through to retrieveDatasetVersionSingleFile", async
() => {
+ const mockBlob = new Blob(["x"], { type: "text/plain" });
+
datasetServiceSpy.retrieveDatasetVersionSingleFile.mockReturnValue(of(mockBlob));
+
+ await
firstValueFrom(downloadService.downloadSingleFile("public/sample.csv", false));
+
expect(datasetServiceSpy.retrieveDatasetVersionSingleFile).toHaveBeenCalledWith("public/sample.csv",
false);
+ });
+
+ // ─── downloadDataset ──────────────────────────────────────────────────────
+
+ it("downloads the latest dataset version as a zip named after the dataset",
async () => {
+ const mockBlob = new Blob(["dataset content"], { type: "application/zip"
});
datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(of(mockBlob));
- downloadService.downloadDataset(datasetId, datasetName).subscribe({
- next: blob => {
- expect(blob).toBe(mockBlob);
- expect(notificationServiceSpy.info).toHaveBeenCalledWith(
- "Starting to download the latest version of the dataset as ZIP"
- );
-
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(datasetId);
- expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"TestDataset.zip");
- expect(notificationServiceSpy.success).toHaveBeenCalledWith(
- "The latest version of the dataset has been downloaded as ZIP"
- );
- done();
- },
- error: (error: unknown) => {
- fail("Should not have thrown an error");
- },
- });
+ const result = await firstValueFrom(downloadService.downloadDataset(1,
"TestDataset"));
+
+ expect(result).toBe(mockBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith(
+ "Starting to download the latest version of the dataset as ZIP"
+ );
+
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(1);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"TestDataset.zip");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith(
+ "The latest version of the dataset has been downloaded as ZIP"
+ );
});
- it.skip("should handle dataset download failure correctly", () => {
- const datasetId = 1;
- const datasetName = "TestDataset";
- const errorMessage = "Dataset download failed";
-
- datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(throwError(()
=> new Error(errorMessage)));
-
- downloadService.downloadDataset(datasetId, datasetName).subscribe({
- next: () => {
- fail("Should have thrown an error");
- },
- error: (error: unknown) => {
- expect(error).toBeTruthy();
- expect(notificationServiceSpy.info).toHaveBeenCalledWith(
- "Starting to download the latest version of the dataset as ZIP"
- );
-
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(datasetId);
- expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
- expect(notificationServiceSpy.error).toHaveBeenCalledWith(
- "Error downloading the latest version of the dataset as ZIP"
- );
- done();
- },
- });
+ it("emits the dataset error notification and rethrows on retrieve failure",
async () => {
+ datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(throwError(()
=> new Error("fail")));
+
+ await expect(firstValueFrom(downloadService.downloadDataset(1,
"TestDataset"))).rejects.toThrow("fail");
+
+ expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
+ expect(notificationServiceSpy.error).toHaveBeenCalledWith(
+ "Error downloading the latest version of the dataset as ZIP"
+ );
});
- it.skip("should download a dataset version successfully", () => {
- const datasetId = 1;
- const datasetVersionId = 1;
- const datasetName = "TestDataset";
- const versionName = "v1.0";
- const mockBlob = new Blob(["version content"], { type: "application/zip"
});
+ // ─── downloadDatasetVersion ───────────────────────────────────────────────
+ it("downloads a specific dataset version with composite zip name", async ()
=> {
+ const mockBlob = new Blob(["v1"], { type: "application/zip" });
datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(of(mockBlob));
- downloadService.downloadDatasetVersion(datasetId, datasetVersionId,
datasetName, versionName).subscribe({
- next: blob => {
- expect(blob).toBe(mockBlob);
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download version v1.0 as ZIP");
-
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(datasetId,
datasetVersionId);
- expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"TestDataset-v1.0.zip");
- expect(notificationServiceSpy.success).toHaveBeenCalledWith("Version
v1.0 has been downloaded as ZIP");
- done();
- },
- error: (error: unknown) => {
- fail("Should not have thrown an error");
- },
- });
+ const result = await
firstValueFrom(downloadService.downloadDatasetVersion(1, 2, "TestDataset",
"v1.0"));
+
+ expect(result).toBe(mockBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download version v1.0 as ZIP");
+
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(1, 2);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(mockBlob,
"TestDataset-v1.0.zip");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("Version v1.0
has been downloaded as ZIP");
});
- it.skip("should handle dataset version download failure correctly", () => {
- const datasetId = 1;
- const datasetVersionId = 1;
- const datasetName = "TestDataset";
- const versionName = "v1.0";
- const errorMessage = "Dataset version download failed";
-
- datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(throwError(()
=> new Error(errorMessage)));
-
- downloadService.downloadDatasetVersion(datasetId, datasetVersionId,
datasetName, versionName).subscribe({
- next: () => {
- fail("Should have thrown an error");
- },
- error: (error: unknown) => {
- expect(error).toBeTruthy();
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download version v1.0 as ZIP");
-
expect(datasetServiceSpy.retrieveDatasetVersionZip).toHaveBeenCalledWith(datasetId,
datasetVersionId);
- expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
- expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading version 'v1.0' as ZIP");
- done();
- },
- });
+ it("emits the version-specific error notification on retrieve failure",
async () => {
+ datasetServiceSpy.retrieveDatasetVersionZip.mockReturnValue(throwError(()
=> new Error("nope")));
+
+ await expect(firstValueFrom(downloadService.downloadDatasetVersion(1, 2,
"TestDataset", "v1.0"))).rejects.toThrow(
+ "nope"
+ );
+
+ expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading version 'v1.0' as ZIP");
});
- it.skip("should download workflows as ZIP successfully", () => {
- const workflowEntries = [
- { id: 1, name: "Workflow1" },
- { id: 2, name: "Workflow2" },
- ];
- const mockBlob = new Blob(["zip content"], { type: "application/zip" });
+ // ─── downloadWorkflow ─────────────────────────────────────────────────────
- vi.spyOn(downloadService as any,
"createWorkflowsZip").mockReturnValue(of(mockBlob));
+ it("downloads a workflow as a JSON blob named after the workflow", async ()
=> {
+ const workflowContent = { hello: "world", operators: [] };
+ workflowPersistServiceSpy.retrieveWorkflow.mockReturnValue(of({ content:
workflowContent } as any));
- downloadService.downloadWorkflowsAsZip(workflowEntries).subscribe({
- next: blob => {
- expect(blob).toBe(mockBlob);
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download workflows as ZIP");
- expect((downloadService as
any).createWorkflowsZip).toHaveBeenCalledWith(workflowEntries);
- expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(
- mockBlob,
- expect.stringMatching(/^workflowExports-.*\.zip$/)
- );
- expect(notificationServiceSpy.success).toHaveBeenCalledWith("Workflows
have been downloaded as ZIP");
- done();
- },
- error: (error: unknown) => {
- fail("Should not have thrown an error");
- },
- });
+ const result = await firstValueFrom(downloadService.downloadWorkflow(42,
"MyWorkflow"));
+
+ expect(result.fileName).toBe("MyWorkflow.json");
+ expect(result.blob).toBeInstanceOf(Blob);
+ expect(result.blob.type).toBe("text/plain;charset=utf-8");
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(result.blob,
"MyWorkflow.json");
+ // Blob.text() isn't shipped by jsdom, so we don't pin the body content
+ // here; the saveAs assertion above already verifies the path that
+ // produced it.
});
- it.skip("should handle workflows ZIP download failure correctly", () => {
- const workflowEntries = [
+ // ─── downloadWorkflowsAsZip ───────────────────────────────────────────────
+
+ it("downloads the workflow ZIP and routes through createWorkflowsZip", async
() => {
+ const mockBlob = new Blob(["zip"], { type: "application/zip" });
+ const entries = [
{ id: 1, name: "Workflow1" },
{ id: 2, name: "Workflow2" },
];
- const errorMessage = "Workflows ZIP download failed";
-
- vi.spyOn(downloadService as any,
"createWorkflowsZip").mockReturnValue(throwError(() => new
Error(errorMessage)));
-
- downloadService.downloadWorkflowsAsZip(workflowEntries).subscribe({
- next: () => {
- fail("Should have thrown an error");
- },
- error: (error: unknown) => {
- expect(error).toBeTruthy();
- expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download workflows as ZIP");
- expect((downloadService as
any).createWorkflowsZip).toHaveBeenCalledWith(workflowEntries);
- expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
- expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading workflows as ZIP");
- done();
- },
- });
+ vi.spyOn(downloadService as any,
"createWorkflowsZip").mockReturnValue(of(mockBlob));
+
+ const result = await
firstValueFrom(downloadService.downloadWorkflowsAsZip(entries));
+
+ expect(result).toBe(mockBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download workflows as ZIP");
+ expect((downloadService as
any).createWorkflowsZip).toHaveBeenCalledWith(entries);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(
+ mockBlob,
+ expect.stringMatching(/^workflowExports-.*\.zip$/)
+ );
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("Workflows
have been downloaded as ZIP");
+ });
+
+ it("propagates errors from createWorkflowsZip with the expected error
notification", async () => {
+ vi.spyOn(downloadService as any,
"createWorkflowsZip").mockReturnValue(throwError(() => new Error("zip fail")));
+
+ await expect(firstValueFrom(downloadService.downloadWorkflowsAsZip([{ id:
1, name: "W" }]))).rejects.toThrow(
+ "zip fail"
+ );
+
+ expect(fileSaverServiceSpy.saveAs).not.toHaveBeenCalled();
+ expect(notificationServiceSpy.error).toHaveBeenCalledWith("Error
downloading workflows as ZIP");
+ });
+
+ // ─── downloadOperatorsResult ──────────────────────────────────────────────
+
+ it("downloads a single operator file directly when there's exactly one
file", async () => {
+ const fileBlob = new Blob(["hello"], { type: "text/plain" });
+ const result = await firstValueFrom(
+ downloadService.downloadOperatorsResult([of([{ filename: "out.csv",
blob: fileBlob }])], {
+ wid: 1,
+ name: "W",
+ } as any)
+ );
+
+ expect(result).toBe(fileBlob);
+ expect(notificationServiceSpy.info).toHaveBeenCalledWith("Starting to
download operator result");
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(fileBlob,
"out.csv");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("Operator
result has been downloaded");
+ });
+
+ // The multi-file zip path goes through `new JSZip()` against the
+ // `import * as JSZip from "jszip"` namespace, which the build flags as
+ // `Constructing "JSZip" will crash at run-time because it's an import
+ // namespace object`. Vitest reproduces the failure (`__vite_ssr_import_*
+ // is not a constructor`). Tracked as a separate cleanup in the codebase;
+ // the test is here as a placeholder so we re-enable it once the import
+ // is normalised to a default import.
+ it.skip("zips multiple operator files into a workflow-named archive", async
() => {
+ const a = new Blob(["a"], { type: "text/plain" });
+ const b = new Blob(["b"], { type: "text/plain" });
+ const result = await firstValueFrom(
+ downloadService.downloadOperatorsResult(
+ [
+ of([
+ { filename: "a.csv", blob: a },
+ { filename: "b.csv", blob: b },
+ ]),
+ ],
+ { wid: 7, name: "TwoFile" } as any
+ )
+ );
+
+ expect(result).toBeInstanceOf(Blob);
+ expect(fileSaverServiceSpy.saveAs).toHaveBeenCalledWith(expect.any(Blob),
"results_7_TwoFile.zip");
+ expect(notificationServiceSpy.success).toHaveBeenCalledWith("Operator
results have been downloaded as ZIP");
+ });
+
+ it("errors out cleanly when no operator result files are provided", async ()
=> {
+ await expect(
+ firstValueFrom(downloadService.downloadOperatorsResult([of([])], { wid:
1, name: "Empty" } as any))
+ ).rejects.toThrow("No files to download");
+ });
+
+ // ─── getWorkflowResultDownloadability ─────────────────────────────────────
+
+ it("hits the downloadability endpoint and returns the operator → labels
map", async () => {
+ const promise =
lastValueFrom(downloadService.getWorkflowResultDownloadability(99));
+ const req = httpMock.expectOne(r =>
r.url.includes("/99/result/downloadability"));
+ expect(req.request.method).toBe("GET");
+ req.flush({ "op-1": ["my-dataset"], "op-2": [] });
+
+ const map = await promise;
+ expect(map).toEqual({ "op-1": ["my-dataset"], "op-2": [] });
+ httpMock.verify();
});
Review Comment:
fixed.
##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts:
##########
@@ -24,40 +24,145 @@ 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") ?? ({} as OperatorSchema);
Review Comment:
fixed.
##########
frontend/src/app/workspace/component/code-editor-dialog/annotation-suggestion.component.spec.ts:
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { AnnotationSuggestionComponent } from
"./annotation-suggestion.component";
+
+describe("AnnotationSuggestionComponent", () => {
+ let component: AnnotationSuggestionComponent;
+ let fixture: ComponentFixture<AnnotationSuggestionComponent>;
+
+ beforeEach(async () => {
+ await TestBed.configureTestingModule({}).compileComponents();
Review Comment:
fixed.
--
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]
