Skip to content

Commit fc8f12f

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[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 <[email protected]> Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]> Auto-Submit: Benedikt Meurer <[email protected]>
1 parent 36d8356 commit fc8f12f

File tree

6 files changed

+33
-68
lines changed

6 files changed

+33
-68
lines changed

docs/contributing/settings-experiments-features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ out/Default/chrome --enable-features="DevToolsNewFeature:string_param/foo/double
133133
134134
* Update the type definition in [`Runtime.ts`](https://crsrc.org/c/third_party/devtools-frontend/src/front_end/core/root/Runtime.ts).
135135
* 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).
136-
* For tests, update the `HOST_CONFIG` in [`EnvironmentHelpers.ts`](https://crsrc.org/c/third_party/devtools-frontend/src/front_end/testing/EnvironmentHelpers.ts).
137136
* Access the host config via `Root.Runtime.hostConfig`.
137+
* In unit tests, make sure to assign the expected configuration using `Object.assign(Root.Runtime.hostConfig, { foo: bar })`.
138138
139139
Please refer to this [example CL](https://crrev.com/c/5626314).

front_end/core/host/AidaClient.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ const TEST_MODEL_ID = 'testModelId';
1111

1212
describeWithEnvironment('AidaClient', () => {
1313
it('adds no model temperature if console insights is not enabled', () => {
14-
Object.assign(Root.Runtime.hostConfig, {});
14+
Object.assign(Root.Runtime.hostConfig, {
15+
aidaAvailability: {
16+
disallowLogging: false,
17+
},
18+
});
1519
const request = Host.AidaClient.AidaClient.buildConsoleInsightsRequest('foo');
1620
assert.deepEqual(request, {
1721
current_message: {parts: [{text: 'foo'}], role: Host.AidaClient.Role.USER},
@@ -23,6 +27,9 @@ describeWithEnvironment('AidaClient', () => {
2327

2428
it('adds a model temperature', () => {
2529
Object.assign(Root.Runtime.hostConfig, {
30+
aidaAvailability: {
31+
disallowLogging: false,
32+
},
2633
devToolsConsoleInsights: {
2734
enabled: true,
2835
temperature: 0.5,
@@ -42,6 +49,9 @@ describeWithEnvironment('AidaClient', () => {
4249

4350
it('adds a model temperature of 0', () => {
4451
Object.assign(Root.Runtime.hostConfig, {
52+
aidaAvailability: {
53+
disallowLogging: false,
54+
},
4555
devToolsConsoleInsights: {
4656
enabled: true,
4757
temperature: 0,
@@ -61,6 +71,9 @@ describeWithEnvironment('AidaClient', () => {
6171

6272
it('ignores a negative model temperature', () => {
6373
Object.assign(Root.Runtime.hostConfig, {
74+
aidaAvailability: {
75+
disallowLogging: false,
76+
},
6477
devToolsConsoleInsights: {
6578
enabled: true,
6679
temperature: -1,
@@ -77,6 +90,9 @@ describeWithEnvironment('AidaClient', () => {
7790

7891
it('adds a model id and temperature', () => {
7992
Object.assign(Root.Runtime.hostConfig, {
93+
aidaAvailability: {
94+
disallowLogging: false,
95+
},
8096
devToolsConsoleInsights: {
8197
enabled: true,
8298
modelId: TEST_MODEL_ID,

front_end/core/host/InspectorFrontendHost.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ export class InspectorFrontendHostStub implements InspectorFrontendHostAPI {
403403
});
404404
}
405405

406-
getHostConfig(callback: (arg0: Root.Runtime.HostConfig) => void): void {
406+
getHostConfig(callback: (hostConfig: Root.Runtime.HostConfig) => void): void {
407407
const result: Root.Runtime.HostConfig = {
408408
aidaAvailability: {
409409
enabled: true,

front_end/panels/explain/components/ConsoleInsight.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,11 @@ describeWithEnvironment('ConsoleInsight', () => {
191191
});
192192

193193
const reportsRating = (positive: boolean) => async () => {
194-
Object.assign(Root.Runtime.hostConfig, {});
194+
Object.assign(Root.Runtime.hostConfig, {
195+
aidaAvailability: {
196+
disallowLogging: false,
197+
},
198+
});
195199
const actionTaken = sinon.stub(Host.userMetrics, 'actionTaken');
196200
const aidaClient = getTestAidaClient();
197201
component = new Explain.ConsoleInsight(

front_end/testing/EnvironmentHelpers.ts

Lines changed: 6 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -501,65 +501,9 @@ export function expectConsoleLogs(expectedLogs: {warn?: string[], log?: string[]
501501
});
502502
}
503503

504-
/**
505-
* The default host configuration used for unit tests.
506-
*/
507-
export const HOST_CONFIG: Readonly<Root.Runtime.HostConfig> = Object.freeze({
508-
aidaAvailability: {
509-
disallowLogging: false,
510-
enterprisePolicyValue: 0,
511-
},
512-
devToolsConsoleInsights: {
513-
enabled: false,
514-
modelId: '',
515-
temperature: -1,
516-
},
517-
devToolsFreestyler: {
518-
modelId: '',
519-
temperature: -1,
520-
enabled: false,
521-
},
522-
devToolsAiAssistanceNetworkAgent: {
523-
modelId: '',
524-
temperature: -1,
525-
enabled: false,
526-
},
527-
devToolsAiAssistanceFileAgent: {
528-
modelId: '',
529-
temperature: -1,
530-
enabled: false,
531-
},
532-
devToolsAiAssistancePerformanceAgent: {
533-
modelId: '',
534-
temperature: -1,
535-
enabled: false,
536-
insightsEnabled: false,
537-
},
538-
devToolsImprovedWorkspaces: {
539-
enabled: false,
540-
},
541-
devToolsVeLogging: {
542-
enabled: true,
543-
testing: false,
544-
},
545-
devToolsWellKnown: {
546-
enabled: false,
547-
},
548-
devToolsPrivacyUI: {
549-
enabled: false,
550-
},
551-
devToolsEnableOriginBoundCookies: {
552-
portBindingEnabled: false,
553-
schemeBindingEnabled: false,
554-
},
555-
devToolsAnimationStylesInStylesTab: {
556-
enabled: false,
557-
},
558-
isOffTheRecord: false,
559-
thirdPartyCookieControls: {
560-
thirdPartyCookieRestrictionEnabled: false,
561-
thirdPartyCookieMetadataEnabled: true,
562-
thirdPartyCookieHeuristicsEnabled: true,
563-
managedBlockThirdPartyCookies: 'Unset',
564-
},
565-
});
504+
export function resetHostConfig() {
505+
for (const key of Object.keys(Root.Runtime.hostConfig)) {
506+
// @ts-expect-error
507+
delete Root.Runtime.hostConfig[key];
508+
}
509+
}

front_end/testing/test_setup.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ import * as Timeline from '../panels/timeline/timeline.js';
1515
import * as ThemeSupport from '../ui/legacy/theme_support/theme_support.js';
1616

1717
import {cleanTestDOM, setupTestDOM} from './DOMHelpers.js';
18-
import {createFakeSetting, HOST_CONFIG} from './EnvironmentHelpers.js';
18+
import {createFakeSetting, resetHostConfig} from './EnvironmentHelpers.js';
1919
import {
2020
checkForPendingActivity,
2121
startTrackingAsyncActivity,
2222
stopTrackingAsyncActivity,
2323
} from './TrackAsyncOperations.js';
2424

2525
beforeEach(async () => {
26-
Object.assign(Root.Runtime.hostConfig, HOST_CONFIG);
26+
resetHostConfig();
2727
await setupTestDOM();
2828
// Ensure that no trace data leaks between tests when testing the trace engine.
2929
for (const handler of Object.values(Trace.Handlers.ModelHandlers)) {
@@ -51,6 +51,7 @@ afterEach(async () => {
5151
}
5252
await cleanTestDOM();
5353
await checkForPendingActivity();
54+
resetHostConfig();
5455
sinon.restore();
5556
stopTrackingAsyncActivity();
5657
// Clear out any Sinon stubs or spies between individual tests.

0 commit comments

Comments
 (0)