From 72ad62865e954779c59d4e5d3c64f8685178cd07 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:43:20 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B9=20replace=20direct=20console=20log?= =?UTF-8?q?s=20with=20dedicated=20logger=20module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Created `src/logger.ts` to wrap console methods. - Replaced direct calls to `console.warn`, `console.info`, and `console.error` throughout the app code with the new logger. - Updated all unit tests to mock and expect calls on the `logger` rather than the global `console`. Co-authored-by: Horizontal42 <75383519+Horizontal42@users.noreply.github.com> --- src/engine/index.ts | 3 ++- src/extensions.test.ts | 41 +++++++++++++++++++++++------------------ src/extensions.ts | 5 +++-- src/logger.ts | 6 ++++++ src/main.ts | 9 +++++---- src/storage.test.ts | 34 +++++++++++++++++++++------------- src/storage.ts | 24 +++++++++++++----------- src/updater.test.ts | 14 ++++++++++---- src/updater.ts | 7 ++++--- 9 files changed, 87 insertions(+), 56 deletions(-) create mode 100644 src/logger.ts diff --git a/src/engine/index.ts b/src/engine/index.ts index 2cc981e..9c26db5 100644 --- a/src/engine/index.ts +++ b/src/engine/index.ts @@ -9,6 +9,7 @@ import { formatValue } from "./formatter"; import { detectDateOrder } from "./datetime"; import snapshot from "./rates-snapshot.json"; import { CRYPTO } from "./extraunits"; +import { logger } from "../logger"; const cryptoSnapshot: Record = {}; for (const c of CRYPTO) cryptoSnapshot[c.code] = 1 / c.snapshotUsd; @@ -144,7 +145,7 @@ export class SumEngine { } else { // Surface unexpected bugs/BigInt errors and log them lineError = e instanceof Error ? e.message : String(e); - console.warn("evaluate failed:", e); + logger.warn("evaluate failed:", e); } value = null; } diff --git a/src/extensions.test.ts b/src/extensions.test.ts index 104eda9..cc2c715 100644 --- a/src/extensions.test.ts +++ b/src/extensions.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { makeApi, runExtensions } from "./extensions"; import { SumEngine } from "./engine"; +import { logger } from "./logger"; describe("extensions", () => { let mockEngine: any; @@ -37,17 +38,17 @@ describe("extensions", () => { }); describe("runExtensions", () => { - let consoleErrorSpy: any; - let consoleWarnSpy: any; + let loggerErrorSpy: any; + let loggerWarnSpy: any; beforeEach(() => { - consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + loggerErrorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + loggerWarnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); }); afterEach(() => { - consoleErrorSpy.mockRestore(); - consoleWarnSpy.mockRestore(); + loggerErrorSpy.mockRestore(); + loggerWarnSpy.mockRestore(); vi.resetModules(); vi.doUnmock("quickjs-emscripten-core"); }); @@ -65,14 +66,14 @@ describe("extensions", () => { baseUnitId: "USD", ratio: 1, }); - expect(consoleErrorSpy).not.toHaveBeenCalled(); + expect(loggerErrorSpy).not.toHaveBeenCalled(); }); - it("swallows errors and logs to console.error", async () => { + it("swallows errors and logs to logger.error", async () => { await runExtensions(mockEngine as SumEngine, [{ name: "script1", code: "throw new Error('test error');" }]); - expect(consoleErrorSpy).toHaveBeenCalledWith("extension script1 failed:", expect.any(Error)); - expect(consoleErrorSpy.mock.calls[0][1].message).toContain("test error"); + expect(loggerErrorSpy).toHaveBeenCalledWith("extension script1 failed:", expect.any(Error)); + expect(loggerErrorSpy.mock.calls[0][1].message).toContain("test error"); }); it("continues running other scripts if one fails", async () => { @@ -84,7 +85,7 @@ describe("extensions", () => { expect(mockEngine.setVariable).toHaveBeenCalledWith("var1", 10); expect(mockEngine.setVariable).toHaveBeenCalledWith("var3", 30); - expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(loggerErrorSpy).toHaveBeenCalledTimes(1); }); it("passes unit-carrying values through setVariable", async () => { @@ -99,7 +100,7 @@ describe("extensions", () => { await runExtensions(mockEngine as SumEngine, [{ name: "bad", code: "numi.setVariable('x', 'nope');" }]); expect(mockEngine.setVariable).not.toHaveBeenCalled(); - expect(consoleErrorSpy.mock.calls[0][1].message).toContain("numi.setVariable"); + expect(loggerErrorSpy.mock.calls[0][1].message).toContain("numi.setVariable"); }); it("registers a function callable with one array of values", async () => { @@ -133,7 +134,7 @@ describe("extensions", () => { { name: "probe", code: "if (typeof window !== 'undefined' || typeof fetch !== 'undefined' || typeof console !== 'undefined') throw new Error('leak');" }, ]); - expect(consoleErrorSpy).not.toHaveBeenCalled(); + expect(loggerErrorSpy).not.toHaveBeenCalled(); }); it("interrupts an infinite loop at load time", async () => { @@ -142,8 +143,8 @@ describe("extensions", () => { { name: "after", code: "numi.setVariable('ok', 1);" }, ]); - expect(consoleErrorSpy).toHaveBeenCalledTimes(1); - expect(consoleErrorSpy.mock.calls[0][0]).toBe("extension spin failed:"); + expect(loggerErrorSpy).toHaveBeenCalledTimes(1); + expect(loggerErrorSpy.mock.calls[0][0]).toBe("extension spin failed:"); // the runtime survives the interrupt and keeps loading scripts expect(mockEngine.setVariable).toHaveBeenCalledWith("ok", 1); }); @@ -186,19 +187,23 @@ describe("extensions", () => { })); const { runExtensions: run } = await import("./extensions"); + const { logger } = await import("./logger"); + const localLoggerWarnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); + await run(mockEngine as SumEngine, [{ name: "s", code: "numi.setVariable('x', 1);" }]); expect(mockEngine.setVariable).not.toHaveBeenCalled(); - expect(consoleWarnSpy).toHaveBeenCalledWith( + expect(localLoggerWarnSpy).toHaveBeenCalledWith( "extensions disabled: QuickJS sandbox unavailable:", expect.any(Error), ); + localLoggerWarnSpy.mockRestore(); }); it("does not load the sandbox when there are no scripts", async () => { await runExtensions(mockEngine as SumEngine, []); - expect(consoleWarnSpy).not.toHaveBeenCalled(); - expect(consoleErrorSpy).not.toHaveBeenCalled(); + expect(loggerWarnSpy).not.toHaveBeenCalled(); + expect(loggerErrorSpy).not.toHaveBeenCalled(); }); }); }); diff --git a/src/extensions.ts b/src/extensions.ts index 0722e68..aabc75c 100644 --- a/src/extensions.ts +++ b/src/extensions.ts @@ -7,6 +7,7 @@ import { } from "quickjs-emscripten-core"; import releaseSyncVariant from "@jitl/quickjs-singlefile-browser-release-sync"; import { SumEngine, ExtensionUnitSpec, ExtensionValue } from "./engine"; +import { logger } from "./logger"; interface ExtensionApi { setVariable(name: string, value: number | ExtensionValue): void; @@ -75,7 +76,7 @@ export async function runExtensions( try { await load(engine, scripts); } catch (e) { - console.warn("extensions disabled: QuickJS sandbox unavailable:", e); + logger.warn("extensions disabled: QuickJS sandbox unavailable:", e); } } @@ -200,7 +201,7 @@ function evaluateScripts( } res.value.dispose(); } catch (e) { - console.error(`extension ${s.name} failed:`, e); + logger.error(`extension ${s.name} failed:`, e); } } } diff --git a/src/logger.ts b/src/logger.ts new file mode 100644 index 0000000..ea7de8e --- /dev/null +++ b/src/logger.ts @@ -0,0 +1,6 @@ +export const logger = { + log: (...args: any[]) => console.log(...args), + info: (...args: any[]) => console.info(...args), + warn: (...args: any[]) => console.warn(...args), + error: (...args: any[]) => console.error(...args), +}; diff --git a/src/main.ts b/src/main.ts index 6102e50..233d50b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,6 +17,7 @@ import { checkForUpdate } from "./updater"; import { Workspace } from "./workspace"; import { EN, RU } from "./engine/vocab-data"; import { exportSheetImage } from "./ui/export"; +import { logger } from "./logger"; import { initSearch, SearchController } from "./ui/search"; import pkg from "../package.json"; @@ -433,7 +434,7 @@ function bindDataDirSettings(): void { try { await migrateDataDir(settings.dataDir, picked, strategy); } catch (e) { - console.warn("migrate failed", e); + logger.warn("migrate failed", e); toast(t("folderError")); return; } @@ -534,7 +535,7 @@ async function registerHotkey(old: string | null, combo: string): Promise { if (enabled) await auto.enable(); else await auto.disable(); } catch (e) { - console.warn("autostart failed", e); + logger.warn("autostart failed", e); } } async function applyAlwaysOnTop(enabled: boolean): Promise { if (!isTauri()) return; const { getCurrentWindow } = await import("@tauri-apps/api/window"); - await getCurrentWindow().setAlwaysOnTop(enabled).catch((e) => console.warn("always-on-top failed", e)); + await getCurrentWindow().setAlwaysOnTop(enabled).catch((e) => logger.warn("always-on-top failed", e)); } async function refreshRates(force = false): Promise { diff --git a/src/storage.test.ts b/src/storage.test.ts index 35e00b2..d899aa9 100644 --- a/src/storage.test.ts +++ b/src/storage.test.ts @@ -1,11 +1,20 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { isTauri, loadSettings, saveSettings, defaultSettingsData, loadAppData, saveAppData, flushAppData, setDataDir, runBackups, backupDeletedSheet, openBackupsFolder, chooseFolder, dataDirHasDocuments, migrateDataDir, fetchRates, fetchHistoricalRates } from './storage'; import { invoke } from '@tauri-apps/api/core'; +import { logger } from './logger'; vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn(), })); +vi.mock('./logger', () => ({ + logger: { + warn: vi.fn(), + info: vi.fn(), + error: vi.fn(), + }, +})); + describe('storage', () => { beforeEach(() => { vi.resetAllMocks(); @@ -14,7 +23,6 @@ describe('storage', () => { getItem: vi.fn(), setItem: vi.fn(), }); - vi.stubGlobal('console', { warn: vi.fn() }); vi.stubGlobal('fetch', vi.fn()); }); @@ -54,7 +62,7 @@ describe('storage', () => { it('falls back to defaultSettingsData if localStorage throws', async () => { vi.mocked(localStorage.getItem).mockImplementation(() => { throw new Error('localStorage error'); }); const settings = await loadSettings(); - expect(console.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); expect(settings).toEqual(defaultSettingsData); }); @@ -62,7 +70,7 @@ describe('storage', () => { vi.stubGlobal('window', { __TAURI_INTERNALS__: {} }); vi.mocked(invoke).mockRejectedValue(new Error('invoke error')); const settings = await loadSettings(); - expect(console.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); expect(settings).toEqual(defaultSettingsData); }); @@ -75,7 +83,7 @@ describe('storage', () => { it('falls back to defaultSettingsData if JSON is invalid', async () => { vi.mocked(localStorage.getItem).mockReturnValue("invalid json"); const settings = await loadSettings(); - expect(console.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadSettings failed', expect.any(Error)); expect(settings).toEqual(defaultSettingsData); }); }); @@ -124,7 +132,7 @@ describe('storage', () => { it('returns null if localStorage throws', async () => { vi.mocked(localStorage.getItem).mockImplementation(() => { throw new Error('localStorage error'); }); const data = await loadAppData(''); - expect(console.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); expect(data).toBeNull(); }); @@ -132,7 +140,7 @@ describe('storage', () => { vi.stubGlobal('window', { __TAURI_INTERNALS__: {} }); vi.mocked(invoke).mockRejectedValue(new Error('invoke error')); const data = await loadAppData(''); - expect(console.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); expect(data).toBeNull(); }); @@ -145,7 +153,7 @@ describe('storage', () => { it('returns null if JSON is invalid', async () => { vi.mocked(localStorage.getItem).mockReturnValue("invalid json"); const data = await loadAppData(''); - expect(console.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('loadAppData failed', expect.any(Error)); expect(data).toBeNull(); }); }); @@ -223,7 +231,7 @@ describe('storage', () => { await flushAppData(); - expect(console.warn).toHaveBeenCalledWith('saveAppData failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('saveAppData failed', expect.any(Error)); }); it('handles invoke throw gracefully in flushAppData', async () => { @@ -234,7 +242,7 @@ describe('storage', () => { await flushAppData(); - expect(console.warn).toHaveBeenCalledWith('saveAppData failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('saveAppData failed', expect.any(Error)); }); }); @@ -255,7 +263,7 @@ describe('storage', () => { vi.stubGlobal('window', { __TAURI_INTERNALS__: {} }); vi.mocked(invoke).mockRejectedValue(new Error('invoke error')); await runBackups('dir', 10); - expect(console.warn).toHaveBeenCalledWith('runBackups failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('runBackups failed', expect.any(Error)); }); }); @@ -281,7 +289,7 @@ describe('storage', () => { vi.stubGlobal('window', { __TAURI_INTERNALS__: {} }); vi.mocked(invoke).mockRejectedValue(new Error('invoke error')); await backupDeletedSheet('dir', 'title', 'contents'); - expect(console.warn).toHaveBeenCalledWith('backupDeletedSheet failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('backupDeletedSheet failed', expect.any(Error)); }); }); @@ -369,7 +377,7 @@ describe('storage', () => { it('returns null if fetch fails', async () => { vi.mocked(fetch).mockRejectedValue(new Error('Network error')); const result = await fetchRates(); - expect(console.warn).toHaveBeenCalledWith('fetchRates failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('fetchRates failed', expect.any(Error)); expect(result).toBeNull(); }); @@ -404,7 +412,7 @@ describe('storage', () => { it('returns null if fetch fails', async () => { vi.mocked(fetch).mockRejectedValue(new Error('Network error')); const result = await fetchHistoricalRates('2024-01-01'); - expect(console.warn).toHaveBeenCalledWith('fetchHistoricalRates failed', '2024-01-01', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('fetchHistoricalRates failed', '2024-01-01', expect.any(Error)); expect(result).toBeNull(); }); diff --git a/src/storage.ts b/src/storage.ts index 9544485..1a4b790 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -1,5 +1,7 @@ // Storage: Tauri commands in the app, localStorage fallback for `vite dev`. +import { logger } from "./logger"; + export interface DocMeta { id: string; title: string; @@ -79,7 +81,7 @@ export async function loadSettings(): Promise { : localStorage.getItem("summarum.settings"); if (raw) return { ...defaultSettingsData, ...JSON.parse(raw) }; } catch (e) { - console.warn("loadSettings failed", e); + logger.warn("loadSettings failed", e); } return { ...defaultSettingsData }; } @@ -99,7 +101,7 @@ export async function loadAppData(dataDir: string): Promise { : localStorage.getItem("summarum.documents"); if (raw) return JSON.parse(raw); } catch (e) { - console.warn("loadAppData failed", e); + logger.warn("loadAppData failed", e); } return null; } @@ -132,7 +134,7 @@ export async function flushAppData(): Promise { if (isTauri()) await invoke("save_documents", { dir: dirArg(currentDataDir), contents: raw }); else localStorage.setItem("summarum.documents", raw); } catch (e) { - console.warn("saveAppData failed", e); + logger.warn("saveAppData failed", e); } } @@ -150,7 +152,7 @@ export async function runBackups(dataDir: string, retentionDays: number): Promis try { await invoke("run_backups", { dir: dirArg(dataDir), retentionDays }); } catch (e) { - console.warn("runBackups failed", e); + logger.warn("runBackups failed", e); } } @@ -159,7 +161,7 @@ export async function backupDeletedSheet(dataDir: string, title: string, content try { await invoke("backup_deleted_sheet", { dir: dirArg(dataDir), title, contents }); } catch (e) { - console.warn("backupDeletedSheet failed", e); + logger.warn("backupDeletedSheet failed", e); } } @@ -205,7 +207,7 @@ export async function fetchRates(force = false): Promise { return { date: json.time_last_update_utc, rates: json.rates, fetchedAt: Math.floor(Date.now() / 1000) }; } } catch (e) { - console.warn("fetchRates failed", e); + logger.warn("fetchRates failed", e); } return null; } @@ -227,7 +229,7 @@ export async function fetchHistoricalRatesBatch(dates: string[]): Promise>("fetch_market_data", { symbols }); } catch (e) { - console.warn("fetchMarketData failed", e); + logger.warn("fetchMarketData failed", e); return {}; } } @@ -277,7 +279,7 @@ export async function loadExtensionScripts(): Promise<{ name: string; code: stri try { return await invoke<{ name: string; code: string }[]>("load_extensions"); } catch (e) { - console.warn("loadExtensions failed", e); + logger.warn("loadExtensions failed", e); return []; } } @@ -314,7 +316,7 @@ export async function onFileDrop(cb: (content: string) => void): Promise { const content = await invoke("read_text_file", { path }); cb(content); } catch (e) { - console.warn("file drop rejected", path, e); + logger.warn("file drop rejected", path, e); } } }); diff --git a/src/updater.test.ts b/src/updater.test.ts index 2b56531..220cf61 100644 --- a/src/updater.test.ts +++ b/src/updater.test.ts @@ -2,6 +2,14 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { checkForUpdate } from './updater'; import * as storage from './storage'; import type { Update } from '@tauri-apps/plugin-updater'; +import { logger } from './logger'; + +vi.mock('./logger', () => ({ + logger: { + info: vi.fn(), + warn: vi.fn(), + }, +})); vi.mock('./storage', () => ({ isTauri: vi.fn(), @@ -22,8 +30,6 @@ vi.mock('@tauri-apps/plugin-process', () => ({ describe('checkForUpdate', () => { beforeEach(() => { vi.resetAllMocks(); - vi.spyOn(console, 'info').mockImplementation(() => {}); - vi.spyOn(console, 'warn').mockImplementation(() => {}); }); it('should return null if not in Tauri', async () => { @@ -39,7 +45,7 @@ describe('checkForUpdate', () => { const result = await checkForUpdate(); expect(result).toBe('error'); - expect(console.warn).toHaveBeenCalledWith('update check failed', expect.any(Error)); + expect(logger.warn).toHaveBeenCalledWith('update check failed', expect.any(Error)); }); it('should return null if no update is available', async () => { @@ -52,7 +58,7 @@ describe('checkForUpdate', () => { const result = await checkForUpdate(); expect(result).toBeNull(); - expect(console.info).toHaveBeenCalledWith("no update: running 1.0.0, that's the latest published release"); + expect(logger.info).toHaveBeenCalledWith("no update: running 1.0.0, that's the latest published release"); }); it('should return an update object if an update is available', async () => { diff --git a/src/updater.ts b/src/updater.ts index 292859e..b93a5eb 100644 --- a/src/updater.ts +++ b/src/updater.ts @@ -1,4 +1,5 @@ import { isTauri } from "./storage"; +import { logger } from "./logger"; interface AvailableUpdate { version: string; @@ -19,10 +20,10 @@ export async function checkForUpdate(): Promise ${update.version}`); + logger.info(`update available: ${update.currentVersion} -> ${update.version}`); return { version: update.version, async install() { @@ -32,7 +33,7 @@ export async function checkForUpdate(): Promise