From 03cd6aac8753674a33a60fe21f489620813d0aa1 Mon Sep 17 00:00:00 2001 From: Iku-turso Date: Tue, 22 Mar 2022 15:33:11 +0200 Subject: [PATCH] Remove kludge in previous commit by explicitly permitting specific side effects where old unit tests require it Signed-off-by: Iku-turso --- src/common/__tests__/base-store.test.ts | 10 +++++++++ src/common/__tests__/cluster-store.test.ts | 11 +++++++++- src/common/__tests__/user-store.test.ts | 10 +++++++++ .../app-version/app-version.injectable.ts | 5 +---- ...get-configuration-file-model.injectable.ts | 3 +-- src/main/__test__/kube-auth-proxy.test.ts | 10 ++++++++- .../__test__/kubeconfig-sync.test.ts | 10 +++++++++ .../protocol-handler/__test__/router.test.ts | 10 +++++++++ .../components/+catalog/catalog.test.tsx | 22 ++++++++++--------- .../+extensions/__tests__/extensions.test.tsx | 10 +++++++++ .../dock/__test__/dock-tabs.test.tsx | 11 +++++++++- .../__test__/log-resource-selector.test.tsx | 10 +++++++++ .../__tests__/hotbar-remove-command.test.tsx | 14 ++++++++++-- .../components/select/select.test.tsx | 10 +++++++++ 14 files changed, 125 insertions(+), 21 deletions(-) diff --git a/src/common/__tests__/base-store.test.ts b/src/common/__tests__/base-store.test.ts index 115eb817bf..a9892d1555 100644 --- a/src/common/__tests__/base-store.test.ts +++ b/src/common/__tests__/base-store.test.ts @@ -11,6 +11,10 @@ import { readFileSync } from "fs"; import directoryForUserDataInjectable from "../app-paths/directory-for-user-data/directory-for-user-data.injectable"; import { getDiForUnitTesting } from "../../main/getDiForUnitTesting"; +import getConfigurationFileModelInjectable + from "../get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../get-configuration-file-model/app-version/app-version.injectable"; jest.mock("electron", () => ({ ipcMain: { @@ -82,6 +86,12 @@ describe("BaseStore", () => { mainDi.override(directoryForUserDataInjectable, () => "some-user-data-directory"); + // @ts-ignore + mainDi.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + mainDi.permitSideEffects(appVersionInjectable); + await mainDi.runSetups(); store = undefined; diff --git a/src/common/__tests__/cluster-store.test.ts b/src/common/__tests__/cluster-store.test.ts index 284f77b0e8..40822b6729 100644 --- a/src/common/__tests__/cluster-store.test.ts +++ b/src/common/__tests__/cluster-store.test.ts @@ -18,7 +18,6 @@ import type { DiContainer, } from "@ogre-tools/injectable"; - import { createClusterInjectionToken } from "../cluster/create-cluster-injection-token"; import directoryForUserDataInjectable @@ -26,6 +25,10 @@ import directoryForUserDataInjectable import kubeAuthProxyCaInjectable from "../../main/kube-auth-proxy/kube-auth-proxy-ca.injectable"; import createKubeAuthProxyCertFilesInjectable from "../../main/kube-auth-proxy/create-kube-auth-proxy-cert-files.injectable"; import { getDiForUnitTesting } from "../../main/getDiForUnitTesting"; +import getConfigurationFileModelInjectable + from "../get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../get-configuration-file-model/app-version/app-version.injectable"; console = new Console(stdout, stderr); @@ -89,6 +92,12 @@ describe("cluster-store", () => { mainDi.override(clusterStoreInjectable, (di) => ClusterStore.createInstance({ createCluster: di.inject(createClusterInjectionToken) })); mainDi.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); + // @ts-ignore + mainDi.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + mainDi.permitSideEffects(appVersionInjectable); + await mainDi.runSetups(); createCluster = mainDi.inject(createClusterInjectionToken); diff --git a/src/common/__tests__/user-store.test.ts b/src/common/__tests__/user-store.test.ts index db37b63d42..a606b10e26 100644 --- a/src/common/__tests__/user-store.test.ts +++ b/src/common/__tests__/user-store.test.ts @@ -33,6 +33,10 @@ import type { ClusterStoreModel } from "../cluster-store/cluster-store"; import { defaultTheme } from "../vars"; import writeFileInjectable from "../fs/write-file.injectable"; import { getDiForUnitTesting } from "../../main/getDiForUnitTesting"; +import getConfigurationFileModelInjectable + from "../get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../get-configuration-file-model/app-version/app-version.injectable"; console = new Console(stdout, stderr); @@ -49,6 +53,12 @@ describe("user store tests", () => { di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); di.override(userStoreInjectable, () => UserStore.createInstance()); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + await di.runSetups(); }); diff --git a/src/common/get-configuration-file-model/app-version/app-version.injectable.ts b/src/common/get-configuration-file-model/app-version/app-version.injectable.ts index 30e65386fb..0fe3142332 100644 --- a/src/common/get-configuration-file-model/app-version/app-version.injectable.ts +++ b/src/common/get-configuration-file-model/app-version/app-version.injectable.ts @@ -8,10 +8,7 @@ import packageInfo from "../../../../package.json"; const appVersionInjectable = getInjectable({ id: "app-version", instantiate: () => packageInfo.version, - - // Todo: actually this does cause side-effects, but many legacy unit tests use fsMock and - // non-overriden Conf to unit test migration of configuration. - // causesSideEffects: true, + causesSideEffects: true, }); export default appVersionInjectable; diff --git a/src/common/get-configuration-file-model/get-configuration-file-model.injectable.ts b/src/common/get-configuration-file-model/get-configuration-file-model.injectable.ts index c065924610..41eeca1ce7 100644 --- a/src/common/get-configuration-file-model/get-configuration-file-model.injectable.ts +++ b/src/common/get-configuration-file-model/get-configuration-file-model.injectable.ts @@ -20,8 +20,7 @@ const getConfigurationFileModelInjectable = getInjectable({ cwd: di.inject(directoryForUserDataInjectable), }), - // Todo: actually this does cause side-effects, but many legacy unit tests use fsMock to unit test this. - // causesSideEffects: true, + causesSideEffects: true, }); export default getConfigurationFileModelInjectable; diff --git a/src/main/__test__/kube-auth-proxy.test.ts b/src/main/__test__/kube-auth-proxy.test.ts index 348112e791..1e35976c2c 100644 --- a/src/main/__test__/kube-auth-proxy.test.ts +++ b/src/main/__test__/kube-auth-proxy.test.ts @@ -52,6 +52,8 @@ import createKubeAuthProxyInjectable from "../kube-auth-proxy/create-kube-auth-p import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token"; import path from "path"; import spawnInjectable from "../child-process/spawn.injectable"; +import getConfigurationFileModelInjectable from "../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable from "../../common/get-configuration-file-model/app-version/app-version.injectable"; console = new Console(stdout, stderr); @@ -96,6 +98,12 @@ describe("kube auth proxy tests", () => { di.override(spawnInjectable, () => mockSpawn); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + mockFs(mockMinikubeConfig); await di.runSetups(); @@ -134,7 +142,7 @@ describe("kube auth proxy tests", () => { beforeEach(async () => { mockedCP = mock(); listeners = new EventEmitter(); - + jest.spyOn(Kubectl.prototype, "checkBinary").mockReturnValueOnce(Promise.resolve(true)); jest.spyOn(Kubectl.prototype, "ensureKubectl").mockReturnValueOnce(Promise.resolve(false)); mockedCP.on.mockImplementation((event: string, listener: (message: any, sendHandle: any) => void): ChildProcess => { diff --git a/src/main/catalog-sources/__test__/kubeconfig-sync.test.ts b/src/main/catalog-sources/__test__/kubeconfig-sync.test.ts index 9c0155570a..a4e93c4ce4 100644 --- a/src/main/catalog-sources/__test__/kubeconfig-sync.test.ts +++ b/src/main/catalog-sources/__test__/kubeconfig-sync.test.ts @@ -16,6 +16,10 @@ import { getDiForUnitTesting } from "../../getDiForUnitTesting"; import { createClusterInjectionToken } from "../../../common/cluster/create-cluster-injection-token"; import directoryForKubeConfigsInjectable from "../../../common/app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable"; import { ClusterStore } from "../../../common/cluster-store/cluster-store"; +import getConfigurationFileModelInjectable + from "../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../common/get-configuration-file-model/app-version/app-version.injectable"; jest.mock("electron", () => ({ app: { @@ -52,6 +56,12 @@ describe("kubeconfig-sync.source tests", () => { ClusterStore.createInstance({ createCluster: () => null }), ); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + di.inject(clusterStoreInjectable); ClusterManager.createInstance(); diff --git a/src/main/protocol-handler/__test__/router.test.ts b/src/main/protocol-handler/__test__/router.test.ts index 20be8ae314..bc3f5b6e51 100644 --- a/src/main/protocol-handler/__test__/router.test.ts +++ b/src/main/protocol-handler/__test__/router.test.ts @@ -19,6 +19,10 @@ import lensProtocolRouterMainInjectable from "../lens-protocol-router-main/lens-protocol-router-main.injectable"; import extensionsStoreInjectable from "../../../extensions/extensions-store/extensions-store.injectable"; +import getConfigurationFileModelInjectable + from "../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../common/get-configuration-file-model/app-version/app-version.injectable"; jest.mock("../../../common/ipc"); @@ -44,6 +48,12 @@ describe("protocol router tests", () => { di.override(extensionsStoreInjectable, () => ExtensionsStore.createInstance()); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + await di.runSetups(); extensionLoader = di.inject(extensionLoaderInjectable); diff --git a/src/renderer/components/+catalog/catalog.test.tsx b/src/renderer/components/+catalog/catalog.test.tsx index 19a74cf0d7..7242b007f3 100644 --- a/src/renderer/components/+catalog/catalog.test.tsx +++ b/src/renderer/components/+catalog/catalog.test.tsx @@ -24,6 +24,10 @@ import { UserStore } from "../../../common/user-store"; import mockFs from "mock-fs"; import directoryForUserDataInjectable from "../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; +import getConfigurationFileModelInjectable + from "../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../common/get-configuration-file-model/app-version/app-version.injectable"; import type { AppEvent } from "../../../common/app-event-bus/event-bus"; import appEventBusInjectable from "../../../common/app-event-bus/app-event-bus.injectable"; @@ -95,6 +99,12 @@ describe("", () => { di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + await di.runSetups(); mockFs(); @@ -281,11 +291,7 @@ describe("", () => { it("emits catalog open AppEvent", () => { render( - , + , ); expect(emitEvent).toHaveBeenCalledWith( { @@ -296,11 +302,7 @@ describe("", () => { it("emits catalog change AppEvent when changing the category", () => { render( - , + , ); userEvent.click(screen.getByText("Web Links")); diff --git a/src/renderer/components/+extensions/__tests__/extensions.test.tsx b/src/renderer/components/+extensions/__tests__/extensions.test.tsx index 8589aa80e7..42ea06fc60 100644 --- a/src/renderer/components/+extensions/__tests__/extensions.test.tsx +++ b/src/renderer/components/+extensions/__tests__/extensions.test.tsx @@ -20,6 +20,10 @@ import { DiRender, renderFor } from "../../test-utils/renderFor"; import extensionDiscoveryInjectable from "../../../../extensions/extension-discovery/extension-discovery.injectable"; import directoryForUserDataInjectable from "../../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; import directoryForDownloadsInjectable from "../../../../common/app-paths/directory-for-downloads/directory-for-downloads.injectable"; +import getConfigurationFileModelInjectable + from "../../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../../common/get-configuration-file-model/app-version/app-version.injectable"; mockWindow(); @@ -53,6 +57,12 @@ describe("Extensions", () => { di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); di.override(directoryForDownloadsInjectable, () => "some-directory-for-downloads"); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + mockFs({ "some-directory-for-user-data": {}, }); diff --git a/src/renderer/components/dock/__test__/dock-tabs.test.tsx b/src/renderer/components/dock/__test__/dock-tabs.test.tsx index 87c0dd5c58..f31149bdfe 100644 --- a/src/renderer/components/dock/__test__/dock-tabs.test.tsx +++ b/src/renderer/components/dock/__test__/dock-tabs.test.tsx @@ -18,6 +18,10 @@ import type { DiRender } from "../../test-utils/renderFor"; import { renderFor } from "../../test-utils/renderFor"; import directoryForUserDataInjectable from "../../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; +import getConfigurationFileModelInjectable + from "../../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../../common/get-configuration-file-model/app-version/app-version.injectable"; jest.mock("electron", () => ({ app: { @@ -73,7 +77,6 @@ describe("", () => { beforeEach(async () => { const di = getDiForUnitTesting({ doGeneralOverrides: true }); - render = renderFor(di); di.override( @@ -81,6 +84,12 @@ describe("", () => { () => "some-test-suite-specific-directory-for-user-data", ); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + await di.runSetups(); dockStore = di.inject(dockStoreInjectable); diff --git a/src/renderer/components/dock/logs/__test__/log-resource-selector.test.tsx b/src/renderer/components/dock/logs/__test__/log-resource-selector.test.tsx index e4479ff783..edb5b6ed05 100644 --- a/src/renderer/components/dock/logs/__test__/log-resource-selector.test.tsx +++ b/src/renderer/components/dock/logs/__test__/log-resource-selector.test.tsx @@ -21,6 +21,10 @@ import { LogTabViewModel, LogTabViewModelDependencies } from "../logs-view-model import type { TabId } from "../../dock/store"; import userEvent from "@testing-library/user-event"; import { SearchStore } from "../../../../search-store/search-store"; +import getConfigurationFileModelInjectable + from "../../../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../../../common/get-configuration-file-model/app-version/app-version.injectable"; jest.mock("electron", () => ({ app: { @@ -131,6 +135,12 @@ describe("", () => { di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); di.override(callForLogsInjectable, () => () => Promise.resolve("some-logs")); + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + render = renderFor(di); await di.runSetups(); diff --git a/src/renderer/components/hotbar/__tests__/hotbar-remove-command.test.tsx b/src/renderer/components/hotbar/__tests__/hotbar-remove-command.test.tsx index 465f12de0a..c9ce0942d4 100644 --- a/src/renderer/components/hotbar/__tests__/hotbar-remove-command.test.tsx +++ b/src/renderer/components/hotbar/__tests__/hotbar-remove-command.test.tsx @@ -10,13 +10,14 @@ import React from "react"; import type { DiContainer } from "@ogre-tools/injectable"; import { getDiForUnitTesting } from "../../../getDiForUnitTesting"; import { DiRender, renderFor } from "../../test-utils/renderFor"; -import hotbarStoreInjectable from "../../../../common/hotbar-store.injectable"; +import hotbarStoreInjectable, { HotbarStore } from "../../../../common/hotbar-store.injectable"; import { ThemeStore } from "../../../theme.store"; import { ConfirmDialog } from "../../confirm-dialog"; -import type { HotbarStore } from "../../../../common/hotbar-store"; import { UserStore } from "../../../../common/user-store"; import mockFs from "mock-fs"; import directoryForUserDataInjectable from "../../../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; +import getConfigurationFileModelInjectable from "../../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable from "../../../../common/get-configuration-file-model/app-version/app-version.injectable"; const mockHotbars: { [id: string]: any } = { "1": { @@ -44,6 +45,15 @@ describe("", () => { di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); + // @ts-ignore + di.permitSideEffects(hotbarStoreInjectable); + + // @ts-ignore + di.permitSideEffects(getConfigurationFileModelInjectable); + + // @ts-ignore + di.permitSideEffects(appVersionInjectable); + render = renderFor(di); UserStore.createInstance(); diff --git a/src/renderer/components/select/select.test.tsx b/src/renderer/components/select/select.test.tsx index 8db25354d3..5e7ce327e2 100644 --- a/src/renderer/components/select/select.test.tsx +++ b/src/renderer/components/select/select.test.tsx @@ -16,6 +16,10 @@ import directoryForUserDataInjectable import rendererExtensionsInjectable from "../../../extensions/renderer-extensions.injectable"; import { computed } from "mobx"; import type { LensRendererExtension } from "../../../extensions/lens-renderer-extension"; +import getConfigurationFileModelInjectable + from "../../../common/get-configuration-file-model/get-configuration-file-model.injectable"; +import appVersionInjectable + from "../../../common/get-configuration-file-model/app-version/app-version.injectable"; jest.mock("electron", () => ({ ipcRenderer: { @@ -38,6 +42,12 @@ describe("