Make attempts to log error throw in unit tests

Errors cannot be allowed to happen without a unit test explicitly causing it. Errors cannot be allowed to happen without author of unit test knowing it.

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

src/common/log-error.global-override-for-injectable.ts

@@ -0,0 +1,11 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getGlobalOverrideForFunction } from "./test-utils/get-global-override-for-function";
+import logErrorInjectable from "./log-error.injectable";
+
+// Note: this should remain as it is, and throw if called. Logging error is something
+// that cannot happen without a unit test explicitly causing it. It cannot be allowed
+// to happen without author of unit test knowing it.
+export default getGlobalOverrideForFunction(logErrorInjectable);

src/common/utils/with-error-logging/with-error-logging.test.ts

@@ -4,28 +4,25 @@
  */
 
 import { getDiForUnitTesting } from "../../../main/getDiForUnitTesting";
-import loggerInjectable from "../../logger.injectable";
-import type { Logger } from "../../logger";
 import withErrorLoggingInjectable from "./with-error-logging.injectable";
 import { pipeline } from "@ogre-tools/fp";
 import type { AsyncFnMock } from "@async-fn/jest";
 import asyncFn from "@async-fn/jest";
 import { getPromiseStatus } from "../../test-utils/get-promise-status";
+import logErrorInjectable from "../../log-error.injectable";
 
 describe("with-error-logging", () => {
   describe("given decorated sync function", () => {
-    let loggerStub: Logger;
     let toBeDecorated: jest.Mock<number | undefined, [string, string]>;
     let decorated: (a: string, b: string) => number | undefined;
+    let logErrorMock: jest.Mock;
 
     beforeEach(() => {
       const di = getDiForUnitTesting({ doGeneralOverrides: true });
 
-      loggerStub = {
+      logErrorMock = jest.fn();
-        error: jest.fn(),
-      } as unknown as Logger;
 
-      di.override(loggerInjectable, () => loggerStub);
+      di.override(logErrorInjectable, () => logErrorMock);
 
       const withErrorLoggingFor = di.inject(withErrorLoggingInjectable);
 
@@ -52,7 +49,7 @@ describe("with-error-logging", () => {
       });
 
       it("does not log error", () => {
-        expect(loggerStub.error).not.toHaveBeenCalled();
+        expect(logErrorMock).not.toHaveBeenCalled();
       });
 
       it("returns the value", () => {
@@ -75,7 +72,7 @@ describe("with-error-logging", () => {
       });
 
       it("does not log error", () => {
-        expect(loggerStub.error).not.toHaveBeenCalled();
+        expect(logErrorMock).not.toHaveBeenCalled();
       });
 
       it("returns nothing", () => {
@@ -104,7 +101,7 @@ describe("with-error-logging", () => {
       });
 
       it("logs the error", () => {
-        expect(loggerStub.error).toHaveBeenCalledWith("some-error-message-for-some-error", error);
+        expect(logErrorMock).toHaveBeenCalledWith("some-error-message-for-some-error", error);
       });
 
       it("throws", () => {
@@ -114,18 +111,16 @@ describe("with-error-logging", () => {
   });
 
   describe("given decorated async function", () => {
-    let loggerStub: Logger;
     let decorated: (a: string, b: string) => Promise<number | undefined>;
     let toBeDecorated: AsyncFnMock<typeof decorated>;
+    let logErrorMock: jest.Mock;
 
     beforeEach(() => {
       const di = getDiForUnitTesting({ doGeneralOverrides: true });
 
-      loggerStub = {
+      logErrorMock = jest.fn();
-        error: jest.fn(),
-      } as unknown as Logger;
 
-      di.override(loggerInjectable, () => loggerStub);
+      di.override(logErrorInjectable, () => logErrorMock);
 
       const withErrorLoggingFor = di.inject(withErrorLoggingInjectable);
 
@@ -153,7 +148,7 @@ describe("with-error-logging", () => {
       });
 
       it("does not log error yet", () => {
-        expect(loggerStub.error).not.toHaveBeenCalled();
+        expect(logErrorMock).not.toHaveBeenCalled();
       });
 
       it("does not resolve yet", async () => {
@@ -176,7 +171,7 @@ describe("with-error-logging", () => {
             error = e;
           }
 
-          expect(loggerStub.error).toHaveBeenCalledWith("some-error-message-for-some-error", error);
+          expect(logErrorMock).toHaveBeenCalledWith("some-error-message-for-some-error", error);
         });
 
         it("rejects", () => {
@@ -198,7 +193,7 @@ describe("with-error-logging", () => {
         });
 
         it("logs the rejection", () => {
-          expect(loggerStub.error).toHaveBeenCalledWith(
+          expect(logErrorMock).toHaveBeenCalledWith(
             "some-error-message-for-some-rejection",
             error,
           );
@@ -215,7 +210,7 @@ describe("with-error-logging", () => {
         });
 
         it("does not log error", () => {
-          expect(loggerStub.error).not.toHaveBeenCalled();
+          expect(logErrorMock).not.toHaveBeenCalled();
         });
 
         it("resolves with the value", async () => {
@@ -231,7 +226,7 @@ describe("with-error-logging", () => {
         });
 
         it("does not log error", () => {
-          expect(loggerStub.error).not.toHaveBeenCalled();
+          expect(logErrorMock).not.toHaveBeenCalled();
         });
 
         it("resolves without value", async () => {

src/common/utils/with-orphan-promise/with-orphan-promise.test.ts

@@ -5,21 +5,20 @@
 import type { AsyncFnMock } from "@async-fn/jest";
 import asyncFn from "@async-fn/jest";
 import { getDiForUnitTesting } from "../../../main/getDiForUnitTesting";
-import loggerInjectable from "../../logger.injectable";
-import type { Logger } from "../../logger";
 import withOrphanPromiseInjectable from "./with-orphan-promise.injectable";
+import logErrorInjectable from "../../log-error.injectable";
 
 describe("with orphan promise, when called", () => {
   let toBeDecorated: AsyncFnMock<(arg1: string, arg2: string) => Promise<string>>;
   let actual: void;
-  let loggerStub: Logger;
+  let logErrorMock: jest.Mock;
 
   beforeEach(() => {
     const di = getDiForUnitTesting({ doGeneralOverrides: true });
 
-    loggerStub = { error: jest.fn() } as unknown as Logger;
+    logErrorMock = jest.fn();
 
-    di.override(loggerInjectable, () => loggerStub);
+    di.override(logErrorInjectable, () => logErrorMock);
 
     const withOrphanPromise = di.inject(withOrphanPromiseInjectable);
 
@@ -49,7 +48,7 @@ describe("with orphan promise, when called", () => {
     });
 
     it("logs the rejection", () => {
-      expect(loggerStub.error).toHaveBeenCalledWith("Orphan promise rejection encountered", "some-error");
+      expect(logErrorMock).toHaveBeenCalledWith("Orphan promise rejection encountered", "some-error");
     });
 
     it("nothing else happens", () => {

src/features/application-menu/handling-of-orphan-application-menu-items.test.ts

@@ -10,8 +10,7 @@ import { getCompositePaths } from "../../common/utils/composite/get-composite-pa
 import { getInjectable } from "@ogre-tools/injectable";
 import applicationMenuItemInjectionToken from "./main/menu-items/application-menu-item-injection-token";
 import { runInAction } from "mobx";
-import loggerInjectable from "../../common/logger.injectable";
+import logErrorInjectable from "../../common/log-error.injectable";
-import type { Logger } from "../../common/logger";
 
 describe("handling-of-orphan-application-menu-items, given orphan menu item", () => {
   let builder: ApplicationBuilder;
@@ -45,7 +44,7 @@ describe("handling-of-orphan-application-menu-items, given orphan menu item", ()
         mainDi.register(someOrphanMenuItemInjectable);
       });
 
-      mainDi.override(loggerInjectable, () => ({ error: logErrorMock }) as unknown as Logger);
+      mainDi.override(logErrorInjectable, () => logErrorMock);
 
       mainDi.override(
         populateApplicationMenuInjectable,

src/features/preferences/navigation-to-telemetry-preferences.test.tsx

@@ -110,6 +110,13 @@ describe("preferences - navigation to telemetry preferences", () => {
         name: "some-test-extension-name",
 
         rendererOptions: {
+          appPreferenceTabs: [
+            {
+              title: "irrelevant",
+              id: "not-telemetry",
+            },
+          ],
+
           appPreferences: [
             {
               title: "irrelevant",

src/features/tray/clicking-tray-menu-item-originating-from-extension.test.ts

@@ -4,10 +4,9 @@
  */
 import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
 import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
-import loggerInjectable from "../../common/logger.injectable";
-import type { Logger } from "../../common/logger";
 import type { FakeExtensionOptions } from "../../renderer/components/test-utils/get-extension-fake";
 import getRandomIdInjectable from "../../common/utils/get-random-id.injectable";
+import logErrorInjectable from "../../common/log-error.injectable";
 
 describe("clicking tray menu item originating from extension", () => {
   let builder: ApplicationBuilder;
@@ -19,7 +18,7 @@ describe("clicking tray menu item originating from extension", () => {
     builder.beforeApplicationStart((mainDi) => {
       logErrorMock = jest.fn();
 
-      mainDi.override(loggerInjectable, () => ({ error: logErrorMock }) as unknown as Logger);
+      mainDi.override(logErrorInjectable, () => logErrorMock);
       mainDi.override(getRandomIdInjectable, () => () => "some-random-id");
     });