From d747977806e4e97cd3d9ca5384b6601738611495 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 10 Oct 2024 19:08:01 -0400 Subject: [PATCH] Fix children in tree nodes persisting on error (#3224) * Fix children in tree nodes persisting on error Signed-off-by: Timothy Johnson * Update unit tests to ensure children don't persist Signed-off-by: Timothy Johnson --------- Signed-off-by: Timothy Johnson Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../dataset/ZoweDatasetNode.unit.test.ts | 4 ++++ .../trees/job/ZoweJobNode.unit.test.ts | 20 ++++++++++++++++++- .../__unit__/trees/uss/USSTree.unit.test.ts | 1 + .../trees/uss/ZoweUSSNode.unit.test.ts | 7 +++++-- .../src/trees/dataset/ZoweDatasetNode.ts | 2 +- .../src/trees/job/ZoweJobNode.ts | 4 ++-- .../src/trees/uss/ZoweUSSNode.ts | 2 +- 7 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/dataset/ZoweDatasetNode.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/dataset/ZoweDatasetNode.unit.test.ts index db0ca1f12b..51a17ff69a 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/dataset/ZoweDatasetNode.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/dataset/ZoweDatasetNode.unit.test.ts @@ -149,6 +149,10 @@ describe("ZoweDatasetNode Unit Tests", () => { profile: profileOne, }); jest.spyOn(zosfiles.List, "allMembers").mockRejectedValueOnce(new Error(subNode.label as string)); + // Populate node with children from previous search to ensure they are removed + subNode.children = [ + new ZoweDatasetNode({ label: "old", collapsibleState: vscode.TreeItemCollapsibleState.None, session, profile: profileOne }), + ]; subNode.dirty = true; const response = await subNode.getChildren(); expect(response).toEqual([]); diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts index 623a0691f9..54f98b38d3 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/job/ZoweJobNode.unit.test.ts @@ -30,7 +30,7 @@ import { Profiles } from "../../../../src/configuration/Profiles"; import { ZoweExplorerApiRegister } from "../../../../src/extending/ZoweExplorerApiRegister"; import { ZoweLocalStorage } from "../../../../src/tools/ZoweLocalStorage"; import { JobFSProvider } from "../../../../src/trees/job/JobFSProvider"; -import { ZoweJobNode } from "../../../../src/trees/job/ZoweJobNode"; +import { ZoweJobNode, ZoweSpoolNode } from "../../../../src/trees/job/ZoweJobNode"; import { SharedContext } from "../../../../src/trees/shared/SharedContext"; import { SharedTreeProviders } from "../../../../src/trees/shared/SharedTreeProviders"; import { JobInit } from "../../../../src/trees/job/JobInit"; @@ -392,6 +392,15 @@ describe("ZoweJobNode unit tests - Function getChildren", () => { jest.spyOn(ZoweExplorerApiRegister, "getJesApi").mockReturnValueOnce({ getSpoolFiles: jest.fn().mockResolvedValue(new Error("Response Fail")), } as any); + // Populate node with children from previous search to ensure they are removed + globalMocks.testJobNode.children = [ + new ZoweSpoolNode({ + label: "old", + collapsibleState: vscode.TreeItemCollapsibleState.None, + session: globalMocks.testSession, + profile: globalMocks.testProfile, + }), + ]; globalMocks.testJobNode.dirty = true; const spools = await globalMocks.testJobNode.getChildren(); expect(spools).toEqual([]); @@ -442,6 +451,15 @@ describe("ZoweJobNode unit tests - Function getChildren", () => { it("should return empty list if there is error retrieving jobs", async () => { const globalMocks = await createGlobalMocks(); await globalMocks.testJobsProvider.addSession("fake"); + // Populate node with children from previous search to ensure they are removed + globalMocks.testJobsProvider.mSessionNodes[1].children = [ + new ZoweJobNode({ + label: "old", + collapsibleState: vscode.TreeItemCollapsibleState.Collapsed, + session: globalMocks.testSession, + profile: globalMocks.testProfile, + }), + ]; globalMocks.testJobsProvider.mSessionNodes[1].filtered = true; globalMocks.mockGetJobsByParameters.mockRejectedValue(new Error("Response Fail")); jest.spyOn(ZoweExplorerApiRegister, "getJesApi").mockReturnValueOnce({ diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/uss/USSTree.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/uss/USSTree.unit.test.ts index a22d8f21c8..17cfac602d 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/uss/USSTree.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/uss/USSTree.unit.test.ts @@ -1685,6 +1685,7 @@ describe("USSTree Unit Tests - Function crossLparMove", () => { profile: ussDirNode.getProfile(), }), ]; + ussDirNode.dirty = false; const deleteMock = jest.spyOn(UssFSProvider.instance, "delete").mockResolvedValue(undefined); const readFileMock = jest.spyOn(UssFSProvider.instance, "readFile").mockResolvedValue(new Uint8Array([1, 2, 3])); diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/uss/ZoweUSSNode.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/uss/ZoweUSSNode.unit.test.ts index 34e76bc095..5d00e1ac6e 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/uss/ZoweUSSNode.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/uss/ZoweUSSNode.unit.test.ts @@ -25,7 +25,7 @@ import { createInstanceOfProfile, createValidIProfile, } from "../../../__mocks__/mockCreators/shared"; -import { createUSSTree } from "../../../__mocks__/mockCreators/uss"; +import { createUSSNode, createUSSTree } from "../../../__mocks__/mockCreators/uss"; import { Constants } from "../../../../src/configuration/Constants"; import { ZoweLocalStorage } from "../../../../src/tools/ZoweLocalStorage"; import { UssFSProvider } from "../../../../src/trees/uss/UssFSProvider"; @@ -846,6 +846,8 @@ describe("ZoweUSSNode Unit Tests - Function node.getChildren()", () => { const globalMocks = createGlobalMocks(); const blockMocks = createBlockMocks(globalMocks); + // Populate node with children from previous search to ensure they are removed + blockMocks.childNode.children = [createUSSNode(globalMocks.session, globalMocks.profileOne)]; blockMocks.childNode.contextValue = Constants.USS_SESSION_CONTEXT; blockMocks.childNode.fullPath = "Throw Error"; blockMocks.childNode.dirty = true; @@ -854,7 +856,8 @@ describe("ZoweUSSNode Unit Tests - Function node.getChildren()", () => { throw new Error("Throwing an error to check error handling for unit tests!"); }); - await blockMocks.childNode.getChildren(); + const response = await blockMocks.childNode.getChildren(); + expect(response).toEqual([]); expect(globalMocks.showErrorMessage.mock.calls.length).toEqual(1); expect(globalMocks.showErrorMessage.mock.calls[0][0]).toEqual( "Retrieving response from USS list API Error: Throwing an error to check error handling for unit tests!" diff --git a/packages/zowe-explorer/src/trees/dataset/ZoweDatasetNode.ts b/packages/zowe-explorer/src/trees/dataset/ZoweDatasetNode.ts index 527d272260..355c9a5397 100644 --- a/packages/zowe-explorer/src/trees/dataset/ZoweDatasetNode.ts +++ b/packages/zowe-explorer/src/trees/dataset/ZoweDatasetNode.ts @@ -230,7 +230,7 @@ export class ZoweDatasetNode extends ZoweTreeNode implements IZoweDatasetTreeNod const cachedProfile = Profiles.getInstance().loadNamedProfile(this.getProfileName()); const responses = await this.getDatasets(cachedProfile); if (responses == null) { - return this.children; + return []; } // push nodes to an object with property names to avoid duplicates diff --git a/packages/zowe-explorer/src/trees/job/ZoweJobNode.ts b/packages/zowe-explorer/src/trees/job/ZoweJobNode.ts index 0daea1bab7..4d4f7dfc86 100644 --- a/packages/zowe-explorer/src/trees/job/ZoweJobNode.ts +++ b/packages/zowe-explorer/src/trees/job/ZoweJobNode.ts @@ -139,7 +139,7 @@ export class ZoweJobNode extends ZoweTreeNode implements IZoweJobTreeNode { // Fetch spool files under job node const spools = await this.getSpoolFiles(this.job); if (spools == null) { - return this.children; + return []; } else if (!spools.length) { const noSpoolNode = new ZoweSpoolNode({ label: vscode.l10n.t("No spool files found"), @@ -192,7 +192,7 @@ export class ZoweJobNode extends ZoweTreeNode implements IZoweJobTreeNode { // Fetch jobs under session node const jobs = await this.getJobs(this._owner, this._prefix, this._searchId, this._jobStatus); if (jobs == null) { - return this.children; + return []; } else if (jobs.length === 0) { const noJobsNode = new ZoweJobNode({ label: vscode.l10n.t("No jobs found"), diff --git a/packages/zowe-explorer/src/trees/uss/ZoweUSSNode.ts b/packages/zowe-explorer/src/trees/uss/ZoweUSSNode.ts index 78503884f9..f7f7951131 100644 --- a/packages/zowe-explorer/src/trees/uss/ZoweUSSNode.ts +++ b/packages/zowe-explorer/src/trees/uss/ZoweUSSNode.ts @@ -206,7 +206,7 @@ export class ZoweUSSNode extends ZoweTreeNode implements IZoweUSSTreeNode { const cachedProfile = Profiles.getInstance().loadNamedProfile(this.getProfileName()); const response = await this.getUssFiles(cachedProfile); if (!response.success) { - return this.children; + return []; } // If search path has changed, invalidate all children