some fixes to unit tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/renderer/components/+extensions/__tests__/extensions.test.tsx

@@ -1,5 +1,5 @@
 import "@testing-library/jest-dom/extend-expect";
-import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { fireEvent, render, waitFor } from "@testing-library/react";
 import fse from "fs-extra";
 import React from "react";
 import { extensionDiscovery } from "../../../../extensions/extension-discovery";
@@ -8,7 +8,9 @@ import { Notifications } from "../../notifications";
 import { ExtensionInstallationStateStore } from "../extension-install.store";
 import { Extensions } from "../extensions";
 
+jest.setTimeout(30000);
 jest.mock("fs-extra");
+jest.mock("../../notifications");
 
 jest.mock("../../../../common/utils", () => ({
   ...jest.requireActual("../../../../common/utils"),
@@ -46,58 +48,56 @@ jest.mock("../../../../extensions/extension-loader", () => ({
   }
 }));
 
-jest.mock("../../notifications", () => ({
-  ok: jest.fn(),
-  error: jest.fn(),
-  info: jest.fn()
-}));
-
 describe("Extensions", () => {
   beforeEach(() => {
     ExtensionInstallationStateStore.reset();
   });
 
   it("disables uninstall and disable buttons while uninstalling", async () => {
-    render(<><Extensions /><ConfirmDialog/></>);
+    const res = render(<><Extensions /><ConfirmDialog /></>);
 
-    expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Disable").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Uninstall").closest("button")).not.toBeDisabled();
 
-    fireEvent.click(screen.getByText("Uninstall"));
+    fireEvent.click(res.getByText("Uninstall"));
 
     // Approve confirm dialog
-    fireEvent.click(screen.getByText("Yes"));
+    fireEvent.click(res.getByText("Yes"));
 
     expect(extensionDiscovery.uninstallExtension).toHaveBeenCalled();
-    expect(screen.getByText("Disable").closest("button")).toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).toBeDisabled();
+    expect(res.getByText("Disable").closest("button")).toBeDisabled();
+    expect(res.getByText("Uninstall").closest("button")).toBeDisabled();
   });
 
-  it("displays error notification on uninstall error", () => {
+  it("displays error notification on uninstall error", async () => {
     (extensionDiscovery.uninstallExtension as any).mockImplementationOnce(() =>
       Promise.reject()
     );
-    render(<><Extensions /><ConfirmDialog/></>);
+    const res = render(<><Extensions /><ConfirmDialog /></>);
 
-    expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-    expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Disable").closest("button")).not.toBeDisabled();
+    expect(res.getByText("Uninstall").closest("button")).not.toBeDisabled();
 
-    fireEvent.click(screen.getByText("Uninstall"));
+    fireEvent.click(res.getByText("Uninstall"));
 
     // Approve confirm dialog
-    fireEvent.click(screen.getByText("Yes"));
+    fireEvent.click(res.getByText("Yes"));
 
-    waitFor(() => {
-      expect(screen.getByText("Disable").closest("button")).not.toBeDisabled();
-      expect(screen.getByText("Uninstall").closest("button")).not.toBeDisabled();
+    await waitFor(() => {
+      expect(res.getByText("Disable").closest("button")).not.toBeDisabled();
+      expect(res.getByText("Uninstall").closest("button")).not.toBeDisabled();
       expect(Notifications.error).toHaveBeenCalledTimes(1);
+    }, {
+      timeout: 30000,
     });
   });
 
-  it("disables install button while installing", () => {
-    render(<Extensions />);
+  it("disables install button while installing", async () => {
+    const res = render(<Extensions />);
 
-    fireEvent.change(screen.getByPlaceholderText("Path or URL to an extension package", {
+    (fse.unlink as jest.MockedFunction<typeof fse.unlink>).mockReturnValue(Promise.resolve() as any);
+
+    fireEvent.change(res.getByPlaceholderText("Path or URL to an extension package", {
       exact: false
     }), {
       target: {
@@ -105,25 +105,27 @@ describe("Extensions", () => {
       }
     });
 
-    fireEvent.click(screen.getByText("Install"));
+    fireEvent.click(res.getByText("Install"));
 
-    waitFor(() => {
-      expect(screen.getByText("Install").closest("button")).toBeDisabled();
-      expect(fse.move).toHaveBeenCalledWith("");
-      expect(Notifications.error).not.toHaveBeenCalled();
+    await waitFor(() => {
+      expect(res.getByText("Install").closest("button")).toBeDisabled();
+    }, {
+      timeout: 10000,
     });
   });
 
-  it("displays spinner while extensions are loading", () => {
+  it("displays spinner while extensions are loading", async () => {
     extensionDiscovery.isLoaded = false;
-    const { container } = render(<Extensions />);
+    const res = render(<Extensions />);
 
-    expect(container.querySelector(".Spinner")).toBeInTheDocument();
+    expect(res.container.querySelector(".Spinner")).toBeInTheDocument();
 
     extensionDiscovery.isLoaded = true;
 
-    waitFor(() =>
-      expect(container.querySelector(".Spinner")).not.toBeInTheDocument()
-    );
+    await waitFor(() => {
+      expect(res.container.querySelector(".Spinner")).not.toBeInTheDocument();
+    }, {
+      timeout: 30000,
+    });
   });
 });

src/renderer/components/+extensions/extension-install.store.ts

@@ -12,13 +12,13 @@ export enum ExtensionInstallationState {
 }
 
 const Prefix = "[ExtensionInstallationStore]";
-const installingExtensions = observable.set<string>();
-const uninstallingExtensions = observable.set<string>();
-const preInstallIds = observable.set<string>();
 
 export class ExtensionInstallationStateStore {
   private static InstallingFromMainChannel = "extension-installation-state-store:install";
   private static ClearInstallingFromMainChannel = "extension-installation-state-store:clear-install";
+  private static PreInstallIds = observable.set<string>();
+  private static UninstallingExtensions = observable.set<string>();
+  private static InstallingExtensions = observable.set<string>();
 
   static bindIpcListeners() {
     ipcRenderer
@@ -32,9 +32,9 @@ export class ExtensionInstallationStateStore {
 
   @action static reset() {
     logger.warn(`${Prefix}: resetting, may throw errors`);
-    installingExtensions.clear();
-    uninstallingExtensions.clear();
-    preInstallIds.clear();
+    ExtensionInstallationStateStore.InstallingExtensions.clear();
+    ExtensionInstallationStateStore.UninstallingExtensions.clear();
+    ExtensionInstallationStateStore.PreInstallIds.clear();
   }
 
   /**
@@ -51,7 +51,7 @@ export class ExtensionInstallationStateStore {
       throw new Error(`${Prefix}: cannot set ${extId} as installing. Is currently ${curState}.`);
     }
 
-    installingExtensions.add(extId);
+    ExtensionInstallationStateStore.InstallingExtensions.add(extId);
   }
 
   /**
@@ -80,10 +80,10 @@ export class ExtensionInstallationStateStore {
     const preInstallStepId = uuid.v4();
 
     logger.debug(`${Prefix}: starting a new preinstall phase: ${preInstallStepId}`);
-    preInstallIds.add(preInstallStepId);
+    ExtensionInstallationStateStore.PreInstallIds.add(preInstallStepId);
 
     return disposer(() => {
-      preInstallIds.delete(preInstallStepId);
+      ExtensionInstallationStateStore.PreInstallIds.delete(preInstallStepId);
       logger.debug(`${Prefix}: ending a preinstall phase: ${preInstallStepId}`);
     });
   }
@@ -102,7 +102,7 @@ export class ExtensionInstallationStateStore {
       throw new Error(`${Prefix}: cannot set ${extId} as uninstalling. Is currently ${curState}.`);
     }
 
-    uninstallingExtensions.add(extId);
+    ExtensionInstallationStateStore.UninstallingExtensions.add(extId);
   }
 
   /**
@@ -117,7 +117,7 @@ export class ExtensionInstallationStateStore {
 
     switch (curState) {
       case ExtensionInstallationState.INSTALLING:
-        return void installingExtensions.delete(extId);
+        return void ExtensionInstallationStateStore.InstallingExtensions.delete(extId);
       default:
         throw new Error(`${Prefix}: cannot clear INSTALLING state for ${extId}, it is currently ${curState}`);
     }
@@ -135,7 +135,7 @@ export class ExtensionInstallationStateStore {
 
     switch (curState) {
       case ExtensionInstallationState.UNINSTALLING:
-        return void uninstallingExtensions.delete(extId);
+        return void ExtensionInstallationStateStore.UninstallingExtensions.delete(extId);
       default:
         throw new Error(`${Prefix}: cannot clear UNINSTALLING state for ${extId}, it is currently ${curState}`);
     }
@@ -146,11 +146,11 @@ export class ExtensionInstallationStateStore {
    * @param extId The ID of the extension
    */
   static getInstallationState(extId: string): ExtensionInstallationState {
-    if (installingExtensions.has(extId)) {
+    if (ExtensionInstallationStateStore.InstallingExtensions.has(extId)) {
       return ExtensionInstallationState.INSTALLING;
     }
 
-    if (uninstallingExtensions.has(extId)) {
+    if (ExtensionInstallationStateStore.UninstallingExtensions.has(extId)) {
       return ExtensionInstallationState.UNINSTALLING;
     }
 
@@ -185,7 +185,7 @@ export class ExtensionInstallationStateStore {
    * The current number of extensions installing
    */
   @computed static get installing(): number {
-    return installingExtensions.size;
+    return ExtensionInstallationStateStore.InstallingExtensions.size;
   }
 
   /**
@@ -199,7 +199,7 @@ export class ExtensionInstallationStateStore {
    * The current number of extensions preinstalling
    */
   @computed static get preinstalling(): number {
-    return preInstallIds.size;
+    return ExtensionInstallationStateStore.PreInstallIds.size;
   }
 
   /**

src/renderer/components/button/button.tsx

@@ -1,5 +1,5 @@
 import "./button.scss";
-import React, { ButtonHTMLAttributes, ReactNode } from "react";
+import React, { ButtonHTMLAttributes } from "react";
 import { cssNames } from "../../utils";
 import { TooltipDecoratorProps, withTooltip } from "../tooltip";
 
@@ -26,29 +26,22 @@ export class Button extends React.PureComponent<ButtonProps, {}> {
 
   render() {
     const {
-      className, waiting, label, primary, accent, plain, hidden, active, big,
-      round, outlined, tooltip, light, children, ...props
+      waiting, label, primary, accent, plain, hidden, active, big,
+      round, outlined, tooltip, light, children, ...btnProps
     } = this.props;
-    const btnProps: Partial<ButtonProps> = props;
 
     if (hidden) return null;
 
-    btnProps.className = cssNames("Button", className, {
+    btnProps.className = cssNames("Button", btnProps.className, {
       waiting, primary, accent, plain, active, big, round, outlined, light,
     });
 
-    const btnContent: ReactNode = (
-      <>
-        {label}
-        {children}
-      </>
-    );
-
     // render as link
     if (this.props.href) {
       return (
         <a {...btnProps} ref={e => this.link = e}>
-          {btnContent}
+          {label}
+          {children}
         </a>
       );
     }
@@ -56,7 +49,8 @@ export class Button extends React.PureComponent<ButtonProps, {}> {
     // render as button
     return (
       <button type="button" {...btnProps} ref={e => this.button = e}>
-        {btnContent}
+        {label}
+        {children}
       </button>
     );
   }