Copilot commented on code in PR #6537:
URL: https://github.com/apache/texera/pull/6537#discussion_r3609556333
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts:
##########
@@ -54,4 +55,95 @@ describe("UserDatasetFileRendererComponent", () => {
const result = component.isPreviewSupported(unsupportedMimeType);
expect(result).toBe(false);
});
+
+ describe("reloadFileContent", () => {
+ it("flags an unsupported file type without hitting the backend", () => {
+ component.filePath = "archive.bin"; // -> OCTET_STREAM -> unsupported
+
+ component.reloadFileContent();
+
+ expect(component.isFileTypePreviewUnsupported).toBe(true);
+ });
Review Comment:
This test claims the unsupported type path avoids a backend call, but it
never sets `did`/`dvid` (which are required for
`retrieveDatasetVersionSingleFile` to run) and it doesn’t assert the service
wasn’t invoked. As written, it would still pass even if the early-return on
unsupported MIME type were removed.
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts:
##########
@@ -54,4 +55,95 @@ describe("UserDatasetFileRendererComponent", () => {
const result = component.isPreviewSupported(unsupportedMimeType);
expect(result).toBe(false);
});
+
+ describe("reloadFileContent", () => {
+ it("flags an unsupported file type without hitting the backend", () => {
+ component.filePath = "archive.bin"; // -> OCTET_STREAM -> unsupported
+
+ component.reloadFileContent();
+
+ expect(component.isFileTypePreviewUnsupported).toBe(true);
+ });
+
+ it("flags an oversized file before loading it", () => {
+ component.filePath = "notes.txt"; // TXT limit is 1 MB
+ component.fileSize = 5 * 1024 * 1024;
+
+ component.reloadFileContent();
+
+ expect(component.isFileSizeUnloadable).toBe(true);
+ });
+
+ it("retrieves a supported file and switches on the matching display", ()
=> {
+ const datasetService = TestBed.inject(DatasetService);
+ const blob = new Blob(["hello"], { type: "text/plain" });
+ const spy = vi.spyOn(datasetService,
"retrieveDatasetVersionSingleFile").mockReturnValue(of(blob));
+ component.did = 1;
+ component.dvid = 2;
+ component.filePath = "notes.txt";
+ component.isLogin = true;
+ component.fileSize = 100;
+
+ component.reloadFileContent();
+
+ expect(spy).toHaveBeenCalledWith("notes.txt", true);
+ expect(component.displayPlainText).toBe(true);
+ expect(component.isLoading).toBe(false);
+ });
+ });
+
+ describe("error handlers", () => {
+ it("onFileLoadingError sets the loading-error state and clears displays",
() => {
+ component.displayCSV = true;
+
+ component.onFileLoadingError();
+
+ expect(component.isFileLoadingError).toBe(true);
+ expect(component.displayCSV).toBe(false);
+ });
+
+ it("onFileSizeNotLoadable sets the size-unloadable state", () => {
+ component.onFileSizeNotLoadable();
+
+ expect(component.isFileSizeUnloadable).toBe(true);
+ });
+
+ it("onFileTypePreviewUnsupported sets the unsupported-type state", () => {
+ component.onFileTypePreviewUnsupported();
+
+ expect(component.isFileTypePreviewUnsupported).toBe(true);
+ });
+ });
+
+ describe("display toggles", () => {
+ it("toggleImageModal flips showImageModal", () => {
+ expect(component.showImageModal).toBe(false);
+
+ component.toggleImageModal();
+ expect(component.showImageModal).toBe(true);
+
+ component.toggleImageModal();
+ expect(component.showImageModal).toBe(false);
+ });
+
+ it("turnOffAllDisplay resets every display and error flag", () => {
+ component.displayCSV = true;
+ component.displayImage = true;
+ component.displayJson = true;
+ component.isLoading = true;
+ component.isFileLoadingError = true;
+ component.isFileSizeUnloadable = true;
+ component.isFileTypePreviewUnsupported = true;
+
+ component.turnOffAllDisplay();
+
+ expect(component.displayCSV).toBe(false);
+ expect(component.displayImage).toBe(false);
+ expect(component.displayJson).toBe(false);
+ expect(component.isLoading).toBe(false);
+ expect(component.isFileLoadingError).toBe(false);
+ expect(component.isFileSizeUnloadable).toBe(false);
+ expect(component.isFileTypePreviewUnsupported).toBe(false);
+ });
Review Comment:
This test says it “resets every display and error flag”, but it only
sets/asserts a subset of the display booleans. That makes the test name
misleading and leaves `displayXlsx`, `displayPlainText`, `displayMarkdown`,
`displayMP4`, and `displayMP3` unverified.
##########
frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts:
##########
@@ -54,4 +55,95 @@ describe("UserDatasetFileRendererComponent", () => {
const result = component.isPreviewSupported(unsupportedMimeType);
expect(result).toBe(false);
});
+
+ describe("reloadFileContent", () => {
+ it("flags an unsupported file type without hitting the backend", () => {
+ component.filePath = "archive.bin"; // -> OCTET_STREAM -> unsupported
+
+ component.reloadFileContent();
+
+ expect(component.isFileTypePreviewUnsupported).toBe(true);
+ });
+
+ it("flags an oversized file before loading it", () => {
+ component.filePath = "notes.txt"; // TXT limit is 1 MB
+ component.fileSize = 5 * 1024 * 1024;
+
+ component.reloadFileContent();
+
+ expect(component.isFileSizeUnloadable).toBe(true);
+ });
Review Comment:
Same issue as the previous spec: this test doesn’t ensure the “oversized”
early-return is what prevents the backend call, because `did`/`dvid` are unset
and there’s no assertion that `retrieveDatasetVersionSingleFile` wasn’t invoked.
--
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]
