From 9ad3f0f87cdacf04198a1808c4f13f203c88d330 Mon Sep 17 00:00:00 2001 From: Graham Langford Date: Thu, 3 Oct 2024 10:21:06 -0500 Subject: [PATCH 1/2] only clear mod component debug logs if logValues is enabled --- src/contentScript/contentScriptPlatform.ts | 16 +++++++++++----- src/extensionPages/extensionPagePlatform.ts | 16 +++++++++++----- src/platform/platformTypes/debuggerProtocol.ts | 5 ++++- src/runtime/reducePipeline.ts | 16 +++++++++++----- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/contentScript/contentScriptPlatform.ts b/src/contentScript/contentScriptPlatform.ts index fb4d26f48a..75f4c57b08 100644 --- a/src/contentScript/contentScriptPlatform.ts +++ b/src/contentScript/contentScriptPlatform.ts @@ -270,11 +270,17 @@ class ContentScriptPlatform extends PlatformBase { override get debugger(): PlatformProtocol["debugger"] { return { - async clear(componentId: UUID): Promise { - await Promise.all([ - traces.clear(componentId), - clearModComponentDebugLogs(componentId), - ]); + async clear( + componentId: UUID, + { logValues }: { logValues: boolean }, + ): Promise { + const clearPromises = [traces.clear(componentId)]; + + if (logValues) { + clearPromises.push(clearModComponentDebugLogs(componentId)); + } + + await Promise.all(clearPromises); }, traces: { enter: traces.addEntry, diff --git a/src/extensionPages/extensionPagePlatform.ts b/src/extensionPages/extensionPagePlatform.ts index e46483c3b6..f7fb96cda3 100644 --- a/src/extensionPages/extensionPagePlatform.ts +++ b/src/extensionPages/extensionPagePlatform.ts @@ -69,11 +69,17 @@ class ExtensionPagePlatform extends PlatformBase { // Support tracing for bricks run in the sidebar and clearing logs in Page Editor/Extension Console. See PanelBody.tsx override get debugger(): PlatformProtocol["debugger"] { return { - async clear(componentId: UUID): Promise { - await Promise.all([ - traces.clear(componentId), - clearModComponentDebugLogs(componentId), - ]); + async clear( + componentId: UUID, + { logValues }: { logValues: boolean }, + ): Promise { + const clearPromises = [traces.clear(componentId)]; + + if (logValues) { + clearPromises.push(clearModComponentDebugLogs(componentId)); + } + + await Promise.all(clearPromises); }, traces: { enter: traces.addEntry, diff --git a/src/platform/platformTypes/debuggerProtocol.ts b/src/platform/platformTypes/debuggerProtocol.ts index e2c2ede835..cda83a2f9b 100644 --- a/src/platform/platformTypes/debuggerProtocol.ts +++ b/src/platform/platformTypes/debuggerProtocol.ts @@ -39,7 +39,10 @@ export interface DebuggerProtocol { * * @param componentId the mod component id */ - clear: (componentId: UUID) => Promise; + clear: ( + componentId: UUID, + { logValues }: { logValues: boolean }, + ) => Promise; traces: TraceProtocol; } diff --git a/src/runtime/reducePipeline.ts b/src/runtime/reducePipeline.ts index 7786ca89eb..950523ef4e 100644 --- a/src/runtime/reducePipeline.ts +++ b/src/runtime/reducePipeline.ts @@ -257,6 +257,14 @@ type RunBrickOptions = CommonOptions & { trace: TraceMetadata; }; +async function getLogValues( + logValues: RunBrickOptions["logValues"] | undefined, +): Promise { + const globalLoggingConfig = await getLoggingConfig(); + + return logValues ?? globalLoggingConfig.logValues ?? false; +} + /** * Get the lexical environment for running a pipeline. Currently, we're just tracking on the pipeline arg itself. * https://en.wikipedia.org/wiki/Closure_(computer_programming) @@ -456,11 +464,9 @@ async function renderBrickArg( ): Promise { const { config, type } = resolvedConfig; - const globalLoggingConfig = await getLoggingConfig(); - const { // If logValues not provided explicitly, default to the global setting - logValues = globalLoggingConfig.logValues ?? false, + logValues = await getLogValues(options.logValues), logger, explicitArg, explicitDataFlow, @@ -637,7 +643,6 @@ async function applyReduceDefaults({ Partial, "modComponentRef" >): Promise { - const globalLoggingConfig = await getLoggingConfig(); const logger = providedLogger ?? new ConsoleLogger(); return { @@ -652,7 +657,7 @@ async function applyReduceDefaults({ explicitDataFlow: false, extendModVariable: false, // If logValues not provided explicitly, default to the global setting - logValues: logValues ?? globalLoggingConfig.logValues ?? false, + logValues: await getLogValues(logValues), // For stylistic consistency, default here instead of destructured parameters branches: [], // NOTE: do not set runId here. It should be set by the starter brick explicitly, or implicitly generated @@ -960,6 +965,7 @@ export async function reduceModComponentPipeline( // `await` promise to avoid race condition where the calls here delete entries from this call to reducePipeline await platform.debugger.clear( partialOptions.modComponentRef.modComponentId, + { logValues: await getLogValues(partialOptions.logValues) }, ); } catch { // NOP From 824de8ee64c75e497bd861df703a53b607efa89f Mon Sep 17 00:00:00 2001 From: Graham Langford Date: Thu, 3 Oct 2024 10:45:04 -0500 Subject: [PATCH 2/2] adds tests --- .../contentScriptPlatform.test.ts | 54 ++++++++++++--- .../extensionPagePlatform.test.ts | 65 +++++++++++++++++++ 2 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 src/extensionPages/extensionPagePlatform.test.ts diff --git a/src/contentScript/contentScriptPlatform.test.ts b/src/contentScript/contentScriptPlatform.test.ts index 83784de218..86409f3221 100644 --- a/src/contentScript/contentScriptPlatform.test.ts +++ b/src/contentScript/contentScriptPlatform.test.ts @@ -24,29 +24,39 @@ import { InteractiveLoginRequiredError } from "@/errors/authErrors"; import { waitForEffect } from "@/testUtils/testHelpers"; import { deferLogin } from "@/contentScript/integrations/deferredLoginController"; import pDefer from "p-defer"; -import { performConfiguredRequestInBackground } from "@/background/messenger/api"; +import { + clearModComponentDebugLogs, + performConfiguredRequestInBackground, + traces, +} from "@/background/messenger/api"; import { API_PATHS } from "@/data/service/urlPaths"; +import { modComponentFactory } from "@/testUtils/factories/modComponentFactories"; jest.mock("@/contentScript/integrations/deferredLoginController"); jest.mock("@/background/messenger/api", () => ({ performConfiguredRequestInBackground: jest.fn().mockResolvedValue({}), + clearModComponentDebugLogs: jest.fn(), + traces: { + clear: jest.fn(), + }, })); const deferLoginMock = jest.mocked(deferLogin); - const backgroundRequestMock = jest.mocked(performConfiguredRequestInBackground); +const clearModComponentDebugLogsMock = jest.mocked(clearModComponentDebugLogs); +const tracesClearMock = jest.mocked(traces.clear); -beforeEach(() => { - setPlatform(contentScriptPlatform); -}); +describe("contentScriptPlatform", () => { + beforeEach(() => { + setPlatform(contentScriptPlatform); + }); -afterEach(async () => { - jest.clearAllMocks(); - await TEST_deleteFeatureFlagsCache(); -}); + afterEach(async () => { + jest.clearAllMocks(); + await TEST_deleteFeatureFlagsCache(); + }); -describe("contentScriptPlatform", () => { it("makes non-interactive successful call", async () => { appApiMock.onGet(API_PATHS.FEATURE_FLAGS).reply(200, { flags: [] }); @@ -119,4 +129,28 @@ describe("contentScriptPlatform", () => { await expect(requestPromise).resolves.toBeObject(); }); + + describe("debugger", () => { + it("clears traces and clears logs when logValues is true", async () => { + const componentId = modComponentFactory().id; + await contentScriptPlatform.debugger.clear(componentId, { + logValues: true, + }); + + expect(tracesClearMock).toHaveBeenCalledExactlyOnceWith(componentId); + expect(clearModComponentDebugLogsMock).toHaveBeenCalledExactlyOnceWith( + componentId, + ); + }); + + it("clears traces and skips clearing logs when logValues is false", async () => { + const componentId = modComponentFactory().id; + await contentScriptPlatform.debugger.clear(componentId, { + logValues: false, + }); + + expect(tracesClearMock).toHaveBeenCalledExactlyOnceWith(componentId); + expect(clearModComponentDebugLogsMock).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/extensionPages/extensionPagePlatform.test.ts b/src/extensionPages/extensionPagePlatform.test.ts new file mode 100644 index 0000000000..90855b0f1d --- /dev/null +++ b/src/extensionPages/extensionPagePlatform.test.ts @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import extensionPagePlatform from "@/extensionPages/extensionPagePlatform"; +import { setPlatform } from "@/platform/platformContext"; +import { modComponentFactory } from "@/testUtils/factories/modComponentFactories"; +import { clearModComponentDebugLogs, traces } from "@/background/messenger/api"; + +jest.mock("@/background/messenger/api", () => ({ + clearModComponentDebugLogs: jest.fn(), + traces: { + clear: jest.fn(), + }, +})); + +const tracesClearMock = jest.mocked(traces.clear); +const clearModComponentDebugLogsMock = jest.mocked(clearModComponentDebugLogs); + +describe("extensionPagePlatform", () => { + beforeEach(() => { + setPlatform(extensionPagePlatform); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("debugger", () => { + it("clears traces and clears logs when logValues is true", async () => { + const componentId = modComponentFactory().id; + await extensionPagePlatform.debugger.clear(componentId, { + logValues: true, + }); + + expect(tracesClearMock).toHaveBeenCalledExactlyOnceWith(componentId); + expect(clearModComponentDebugLogsMock).toHaveBeenCalledExactlyOnceWith( + componentId, + ); + }); + + it("clears traces and skips clearing logs when logValues is false", async () => { + const componentId = modComponentFactory().id; + await extensionPagePlatform.debugger.clear(componentId, { + logValues: false, + }); + + expect(tracesClearMock).toHaveBeenCalledExactlyOnceWith(componentId); + expect(clearModComponentDebugLogsMock).not.toHaveBeenCalled(); + }); + }); +});