Copilot commented on code in PR #6599:
URL: https://github.com/apache/texera/pull/6599#discussion_r3613266327
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -717,5 +717,305 @@ describe("SavedWorkflowSectionComponent", () => {
expect(searchSpy).toHaveBeenCalledWith(true);
});
});
+
+ describe("uncovered branch coverage", () => {
+ // A FileReader whose result is intentionally not a string, so
handleFileUploads
+ // exercises its "file is not a string" guard. readAsText fires onload
on the next
+ // tick, mirroring the real async contract (the component assigns onload
afterwards).
+ class NonStringFileReader {
+ public result: unknown = 12345;
+ public onload: (() => void) | null = null;
+ readAsText(): void {
+ setTimeout(() => this.onload?.(), 0);
+ }
+ }
+
+ afterEach(() => {
+ vi.unstubAllGlobals();
+ });
+
+ describe("view-child getters", () => {
+ it("searchResultsComponent getter throws before the view is
initialized", () => {
+ (component as any)._searchResultsComponent = undefined;
+ expect(() => component.searchResultsComponent).toThrow(
+ "Property cannot be accessed before it is initialized."
+ );
+ });
+
+ it("filters getter throws before the view is initialized", () => {
+ (component as any)._filters = undefined;
+ expect(() => component.filters).toThrow("Property cannot be accessed
before it is initialized.");
+ });
+
+ it("re-searches whenever the filters component emits
masterFilterListChange", () => {
+ const searchSpy = vi.spyOn(component,
"search").mockResolvedValue(undefined);
+
+ component.filters.masterFilterListChange.emit([]);
+
+ expect(searchSpy).toHaveBeenCalled();
+ });
+ });
+
+ describe("view type initialization", () => {
+ it("restores the persisted 'card' view mode when the component is
constructed", () => {
+ localStorage.setItem(VIEW_MODE_KEY, "card");
+
+ const freshFixture = TestBed.createComponent(UserWorkflowComponent);
+
+ expect(freshFixture.componentInstance.viewType).toBe("card");
+ freshFixture.destroy();
+ });
Review Comment:
This test mutates `localStorage` but doesn’t clean it up, which can make
other tests order-dependent (especially if other tests rely on the default view
mode). Clear or restore `VIEW_MODE_KEY` at the end of the test (or in an
`afterEach`) to keep the suite hermetic.
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -717,5 +717,305 @@ describe("SavedWorkflowSectionComponent", () => {
expect(searchSpy).toHaveBeenCalledWith(true);
});
});
+
+ describe("uncovered branch coverage", () => {
+ // A FileReader whose result is intentionally not a string, so
handleFileUploads
+ // exercises its "file is not a string" guard. readAsText fires onload
on the next
+ // tick, mirroring the real async contract (the component assigns onload
afterwards).
+ class NonStringFileReader {
+ public result: unknown = 12345;
+ public onload: (() => void) | null = null;
+ readAsText(): void {
+ setTimeout(() => this.onload?.(), 0);
+ }
+ }
Review Comment:
The `NonStringFileReader` stub doesn’t match the `FileReader` surface
area/signatures (`onload` typically receives an event; `readAsText` is called
with a `Blob`). Tightening the stub to accept the expected parameters (even if
unused) will prevent type drift and reduce brittleness if the component starts
using the event argument or relies on method arity.
##########
frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts:
##########
@@ -717,5 +717,305 @@ describe("SavedWorkflowSectionComponent", () => {
expect(searchSpy).toHaveBeenCalledWith(true);
});
});
+
+ describe("uncovered branch coverage", () => {
+ // A FileReader whose result is intentionally not a string, so
handleFileUploads
+ // exercises its "file is not a string" guard. readAsText fires onload
on the next
+ // tick, mirroring the real async contract (the component assigns onload
afterwards).
+ class NonStringFileReader {
+ public result: unknown = 12345;
+ public onload: (() => void) | null = null;
+ readAsText(): void {
+ setTimeout(() => this.onload?.(), 0);
+ }
+ }
+
+ afterEach(() => {
+ vi.unstubAllGlobals();
+ });
Review Comment:
This block cleans up `stubGlobal` calls, but the added tests also create
multiple spies (e.g., `vi.spyOn(window, \"alert\")`, `vi.spyOn(console,
\"log\")`, router/navigation spies). If the surrounding suite isn’t already
restoring mocks automatically, these spies can leak between tests. Consider
restoring mocks here as well (e.g., via `vi.restoreAllMocks()` or an equivalent
existing project pattern) to keep isolation strong.
--
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]
