From fc8f12f4cb4edf0c1bb48deb87754d790b2df6ea Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Wed, 12 Feb 2025 14:24:06 +0100 Subject: [PATCH] [cleanup] Use default (empty) host configuration for unit tests. Each unit test should properly configure `Root.Runtime.hostConfig` and not rely on some global defaults (for tests). Bug: 396033932 Change-Id: I6462a6e80a8c687e9fc1f8a652f5ecc7e350b50c Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6257768 Commit-Queue: Alex Rudenko Reviewed-by: Alex Rudenko Commit-Queue: Benedikt Meurer Auto-Submit: Benedikt Meurer --- .../settings-experiments-features.md | 2 +- front_end/core/host/AidaClient.test.ts | 18 ++++- front_end/core/host/InspectorFrontendHost.ts | 2 +- .../explain/components/ConsoleInsight.test.ts | 6 +- front_end/testing/EnvironmentHelpers.ts | 68 ++----------------- front_end/testing/test_setup.ts | 5 +- 6 files changed, 33 insertions(+), 68 deletions(-) diff --git a/docs/contributing/settings-experiments-features.md b/docs/contributing/settings-experiments-features.md index fb6d5001eb..6e306da881 100644 --- a/docs/contributing/settings-experiments-features.md +++ b/docs/contributing/settings-experiments-features.md @@ -133,7 +133,7 @@ out/Default/chrome --enable-features="DevToolsNewFeature:string_param/foo/double * Update the type definition in [`Runtime.ts`](https://crsrc.org/c/third_party/devtools-frontend/src/front_end/core/root/Runtime.ts). * Update the dummy value returned by `getHostConfig` in [`InspectorFrontendHost.ts`](https://crsrc.org/c/third_party/devtools-frontend/src/front_end/core/host/InspectorFrontendHost.ts). -* For tests, update the `HOST_CONFIG` in [`EnvironmentHelpers.ts`](https://crsrc.org/c/third_party/devtools-frontend/src/front_end/testing/EnvironmentHelpers.ts). * Access the host config via `Root.Runtime.hostConfig`. +* In unit tests, make sure to assign the expected configuration using `Object.assign(Root.Runtime.hostConfig, { foo: bar })`. Please refer to this [example CL](https://crrev.com/c/5626314). diff --git a/front_end/core/host/AidaClient.test.ts b/front_end/core/host/AidaClient.test.ts index dfb5a00fab..68532e59be 100644 --- a/front_end/core/host/AidaClient.test.ts +++ b/front_end/core/host/AidaClient.test.ts @@ -11,7 +11,11 @@ const TEST_MODEL_ID = 'testModelId'; describeWithEnvironment('AidaClient', () => { it('adds no model temperature if console insights is not enabled', () => { - Object.assign(Root.Runtime.hostConfig, {}); + Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, + }); const request = Host.AidaClient.AidaClient.buildConsoleInsightsRequest('foo'); assert.deepEqual(request, { current_message: {parts: [{text: 'foo'}], role: Host.AidaClient.Role.USER}, @@ -23,6 +27,9 @@ describeWithEnvironment('AidaClient', () => { it('adds a model temperature', () => { Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, devToolsConsoleInsights: { enabled: true, temperature: 0.5, @@ -42,6 +49,9 @@ describeWithEnvironment('AidaClient', () => { it('adds a model temperature of 0', () => { Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, devToolsConsoleInsights: { enabled: true, temperature: 0, @@ -61,6 +71,9 @@ describeWithEnvironment('AidaClient', () => { it('ignores a negative model temperature', () => { Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, devToolsConsoleInsights: { enabled: true, temperature: -1, @@ -77,6 +90,9 @@ describeWithEnvironment('AidaClient', () => { it('adds a model id and temperature', () => { Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, devToolsConsoleInsights: { enabled: true, modelId: TEST_MODEL_ID, diff --git a/front_end/core/host/InspectorFrontendHost.ts b/front_end/core/host/InspectorFrontendHost.ts index f9f81dd36a..4b9c8cc8cf 100644 --- a/front_end/core/host/InspectorFrontendHost.ts +++ b/front_end/core/host/InspectorFrontendHost.ts @@ -403,7 +403,7 @@ export class InspectorFrontendHostStub implements InspectorFrontendHostAPI { }); } - getHostConfig(callback: (arg0: Root.Runtime.HostConfig) => void): void { + getHostConfig(callback: (hostConfig: Root.Runtime.HostConfig) => void): void { const result: Root.Runtime.HostConfig = { aidaAvailability: { enabled: true, diff --git a/front_end/panels/explain/components/ConsoleInsight.test.ts b/front_end/panels/explain/components/ConsoleInsight.test.ts index 9c988d6eff..35a7ba7228 100644 --- a/front_end/panels/explain/components/ConsoleInsight.test.ts +++ b/front_end/panels/explain/components/ConsoleInsight.test.ts @@ -191,7 +191,11 @@ describeWithEnvironment('ConsoleInsight', () => { }); const reportsRating = (positive: boolean) => async () => { - Object.assign(Root.Runtime.hostConfig, {}); + Object.assign(Root.Runtime.hostConfig, { + aidaAvailability: { + disallowLogging: false, + }, + }); const actionTaken = sinon.stub(Host.userMetrics, 'actionTaken'); const aidaClient = getTestAidaClient(); component = new Explain.ConsoleInsight( diff --git a/front_end/testing/EnvironmentHelpers.ts b/front_end/testing/EnvironmentHelpers.ts index 742a03fa80..a300c98f19 100644 --- a/front_end/testing/EnvironmentHelpers.ts +++ b/front_end/testing/EnvironmentHelpers.ts @@ -501,65 +501,9 @@ export function expectConsoleLogs(expectedLogs: {warn?: string[], log?: string[] }); } -/** - * The default host configuration used for unit tests. - */ -export const HOST_CONFIG: Readonly = Object.freeze({ - aidaAvailability: { - disallowLogging: false, - enterprisePolicyValue: 0, - }, - devToolsConsoleInsights: { - enabled: false, - modelId: '', - temperature: -1, - }, - devToolsFreestyler: { - modelId: '', - temperature: -1, - enabled: false, - }, - devToolsAiAssistanceNetworkAgent: { - modelId: '', - temperature: -1, - enabled: false, - }, - devToolsAiAssistanceFileAgent: { - modelId: '', - temperature: -1, - enabled: false, - }, - devToolsAiAssistancePerformanceAgent: { - modelId: '', - temperature: -1, - enabled: false, - insightsEnabled: false, - }, - devToolsImprovedWorkspaces: { - enabled: false, - }, - devToolsVeLogging: { - enabled: true, - testing: false, - }, - devToolsWellKnown: { - enabled: false, - }, - devToolsPrivacyUI: { - enabled: false, - }, - devToolsEnableOriginBoundCookies: { - portBindingEnabled: false, - schemeBindingEnabled: false, - }, - devToolsAnimationStylesInStylesTab: { - enabled: false, - }, - isOffTheRecord: false, - thirdPartyCookieControls: { - thirdPartyCookieRestrictionEnabled: false, - thirdPartyCookieMetadataEnabled: true, - thirdPartyCookieHeuristicsEnabled: true, - managedBlockThirdPartyCookies: 'Unset', - }, -}); +export function resetHostConfig() { + for (const key of Object.keys(Root.Runtime.hostConfig)) { + // @ts-expect-error + delete Root.Runtime.hostConfig[key]; + } +} diff --git a/front_end/testing/test_setup.ts b/front_end/testing/test_setup.ts index 5d03dcb03e..0081cf23b5 100644 --- a/front_end/testing/test_setup.ts +++ b/front_end/testing/test_setup.ts @@ -15,7 +15,7 @@ import * as Timeline from '../panels/timeline/timeline.js'; import * as ThemeSupport from '../ui/legacy/theme_support/theme_support.js'; import {cleanTestDOM, setupTestDOM} from './DOMHelpers.js'; -import {createFakeSetting, HOST_CONFIG} from './EnvironmentHelpers.js'; +import {createFakeSetting, resetHostConfig} from './EnvironmentHelpers.js'; import { checkForPendingActivity, startTrackingAsyncActivity, @@ -23,7 +23,7 @@ import { } from './TrackAsyncOperations.js'; beforeEach(async () => { - Object.assign(Root.Runtime.hostConfig, HOST_CONFIG); + resetHostConfig(); await setupTestDOM(); // Ensure that no trace data leaks between tests when testing the trace engine. for (const handler of Object.values(Trace.Handlers.ModelHandlers)) { @@ -51,6 +51,7 @@ afterEach(async () => { } await cleanTestDOM(); await checkForPendingActivity(); + resetHostConfig(); sinon.restore(); stopTrackingAsyncActivity(); // Clear out any Sinon stubs or spies between individual tests.