Re: [PR] test(frontend): extend ShareAccessComponent unit test coverage [texera]

[via GitHub]

aglinxinyuan commented on code in PR #6771:
URL: https://github.com/apache/texera/pull/6771#discussion_r3627783987


##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
       expect(notificationSpy.success).toHaveBeenCalledWith("Dataset 
unpublished successfully");
     });
   });
+
+  describe("hasWriteAccess without a resolved email", () => {
+    it("returns false when the current user has no email at all", () => {
+      // an empty string makes the stubbed UserService.getCurrentUser return 
undefined,
+      // so this.currentEmail resolves to undefined and the early-return guard 
is exercised
+      const c = setupComponent({ currentEmail: "" });
+      expect(c.currentEmail).toBeUndefined();
+      expect(c.hasWriteAccess).toBe(false);
+    });

Review Comment:
   Fixed — the test now sets currentEmail directly to undefined and asserts 
hasWriteAccess, so it exercises the no-email guard independently of how the 
user-service stub resolves an empty email.



##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
       expect(notificationSpy.success).toHaveBeenCalledWith("Dataset 
unpublished successfully");
     });
   });
+
+  describe("hasWriteAccess without a resolved email", () => {
+    it("returns false when the current user has no email at all", () => {
+      // an empty string makes the stubbed UserService.getCurrentUser return 
undefined,
+      // so this.currentEmail resolves to undefined and the early-return guard 
is exercised
+      const c = setupComponent({ currentEmail: "" });
+      expect(c.currentEmail).toBeUndefined();
+      expect(c.hasWriteAccess).toBe(false);
+    });
+  });
+
+  describe("removeEmailTag", () => {
+    it("removes the matching email and keeps the others", () => {
+      const c = setupComponent();
+      c.emailTags = ["a...@example.com", "b...@example.com", 
"c...@example.com"];
+      c.removeEmailTag("b...@example.com");
+      expect(c.emailTags).toEqual(["a...@example.com", "c...@example.com"]);
+    });
+
+    it("leaves tags unchanged when the email is not present", () => {
+      const c = setupComponent();
+      c.emailTags = ["a...@example.com"];
+      c.removeEmailTag("miss...@example.com");
+      expect(c.emailTags).toEqual(["a...@example.com"]);
+    });
+  });
+
+  describe("onChange", () => {
+    it("filters allOwners case-insensitively by the typed value", () => {
+      const c = setupComponent();
+      (c as any).allOwners = ["Alice", "Bob", "alfred"];
+      c.onChange("al");

Review Comment:
   Fixed — allOwners is now populated via c.allOwners.push(...) (the field is 
readonly but the array is mutable), no more cast to any.



##########
frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts:
##########
@@ -512,4 +512,208 @@ describe("ShareAccessComponent", () => {
       expect(notificationSpy.success).toHaveBeenCalledWith("Dataset 
unpublished successfully");
     });
   });
+
+  describe("hasWriteAccess without a resolved email", () => {
+    it("returns false when the current user has no email at all", () => {
+      // an empty string makes the stubbed UserService.getCurrentUser return 
undefined,
+      // so this.currentEmail resolves to undefined and the early-return guard 
is exercised
+      const c = setupComponent({ currentEmail: "" });
+      expect(c.currentEmail).toBeUndefined();
+      expect(c.hasWriteAccess).toBe(false);
+    });
+  });
+
+  describe("removeEmailTag", () => {
+    it("removes the matching email and keeps the others", () => {
+      const c = setupComponent();
+      c.emailTags = ["a...@example.com", "b...@example.com", 
"c...@example.com"];
+      c.removeEmailTag("b...@example.com");
+      expect(c.emailTags).toEqual(["a...@example.com", "c...@example.com"]);
+    });
+
+    it("leaves tags unchanged when the email is not present", () => {
+      const c = setupComponent();
+      c.emailTags = ["a...@example.com"];
+      c.removeEmailTag("miss...@example.com");
+      expect(c.emailTags).toEqual(["a...@example.com"]);
+    });
+  });
+
+  describe("onChange", () => {
+    it("filters allOwners case-insensitively by the typed value", () => {
+      const c = setupComponent();
+      (c as any).allOwners = ["Alice", "Bob", "alfred"];
+      c.onChange("al");
+      expect(c.filteredOwners).toEqual(["Alice", "alfred"]);
+    });
+
+    it("clears filteredOwners when the value is null", () => {
+      const c = setupComponent();
+      (c as any).allOwners = ["Alice"];
+      c.filteredOwners = ["stale"];

Review Comment:
   Fixed — same here, populated via push() instead of casting to any to 
reassign.



-- 
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: notifications-unsubscr...@texera.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org