Skip to content

Commit 3e89f93

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[common] Cleanup SettingRegistration tests
We don't need a full `setupSettings` call. And we don't need a shared setup just for 2 tests. Also, use a local registration for the host config test, as it doesn't check `registerSettingExtension` behavior. [email protected] Bug: None Change-Id: I3751a9f84d08cfee6511aa0cf73779baf180924b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7137799 Auto-Submit: Simon Zünd <[email protected]> Commit-Queue: Simon Zünd <[email protected]> Commit-Queue: Kim-Anh Tran <[email protected]> Reviewed-by: Kim-Anh Tran <[email protected]>
1 parent 8fbcba5 commit 3e89f93

File tree

1 file changed

+22
-42
lines changed

1 file changed

+22
-42
lines changed

front_end/core/common/SettingRegistration.test.ts

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,22 @@
55
import {
66
updateHostConfig,
77
} from '../../testing/EnvironmentHelpers.js';
8-
import {setupSettings} from '../../testing/SettingsHelpers.js';
9-
import * as i18n from '../i18n/i18n.js';
108

119
import * as Common from './common.js';
1210

13-
const settingName = 'mock-setting';
14-
const settingTitle = 'Mock setting';
15-
const enableTitle = 'Enable mock setting';
16-
const disableTitle = 'Disable mock setting';
17-
1811
describe('SettingRegistration', () => {
19-
// const enum `SettingCategory` not available in top level scope, thats why
20-
// its initialized here.
21-
const settingCategory = Common.Settings.SettingCategory.CONSOLE;
22-
23-
beforeEach(() => {
24-
Common.Settings.registerSettingsForTest(
25-
[{
26-
category: settingCategory,
27-
title: i18n.i18n.lockedLazyString(settingTitle),
28-
settingType: Common.Settings.SettingType.BOOLEAN,
29-
settingName,
30-
defaultValue: false,
31-
options: [
32-
{
33-
value: true,
34-
title: i18n.i18n.lockedLazyString(enableTitle),
35-
},
36-
{
37-
value: false,
38-
title: i18n.i18n.lockedLazyString(disableTitle),
39-
},
40-
],
41-
}],
42-
true);
12+
beforeEach(() => Common.Settings.resetSettings());
13+
afterEach(() => Common.Settings.resetSettings());
4314

44-
setupSettings(false);
45-
});
15+
const settingName = 'mock-setting'; // Moved into a variable to prevent KnownContextValue linter to pick it up.
4616

4717
it('throws an error when trying to register a duplicated setting name', () => {
18+
Common.Settings.registerSettingExtension({
19+
settingName,
20+
settingType: Common.Settings.SettingType.BOOLEAN,
21+
defaultValue: false,
22+
});
23+
4824
assert.throws(() => {
4925
Common.Settings.registerSettingExtension({
5026
settingName,
@@ -55,7 +31,14 @@ describe('SettingRegistration', () => {
5531
});
5632

5733
it('deletes a registered setting using its name', () => {
34+
Common.Settings.registerSettingExtension({
35+
settingName,
36+
settingType: Common.Settings.SettingType.BOOLEAN,
37+
defaultValue: false,
38+
});
39+
5840
const removalResult = Common.Settings.maybeRemoveSettingExtension(settingName);
41+
5942
assert.isTrue(removalResult);
6043
assert.doesNotThrow(() => {
6144
Common.Settings.registerSettingExtension({
@@ -67,33 +50,30 @@ describe('SettingRegistration', () => {
6750
});
6851

6952
it('can handle settings with condition which depends on host config', () => {
70-
const configSettingName = 'mock-setting-with-host-config';
7153
updateHostConfig({
7254
devToolsConsoleInsights: {
7355
modelId: 'mockModel',
7456
temperature: -1,
7557
enabled: true,
7658
},
7759
});
78-
Common.Settings.registerSettingExtension({
79-
settingName: configSettingName,
60+
const settingRegistrations: Common.SettingRegistration.SettingRegistration[] = [{
61+
settingName,
8062
settingType: Common.Settings.SettingType.BOOLEAN,
8163
defaultValue: false,
8264
condition: config => {
8365
return config?.devToolsConsoleInsights?.enabled === true;
8466
},
85-
});
86-
assert.throws(() => Common.Settings.Settings.instance().moduleSetting(configSettingName));
67+
}];
8768

8869
const dummyStorage = new Common.Settings.SettingsStorage({});
89-
Common.Settings.Settings.instance({
90-
forceNew: true,
70+
const settings = new Common.Settings.Settings({
9171
syncedStorage: dummyStorage,
9272
globalStorage: dummyStorage,
9373
localStorage: dummyStorage,
94-
settingRegistrations: Common.SettingRegistration.getRegisteredSettings(),
74+
settingRegistrations,
9575
});
96-
const setting = Common.Settings.Settings.instance().moduleSetting(configSettingName);
76+
const setting = settings.moduleSetting(settingName);
9777
assert.isNotNull(setting);
9878
assert.isFalse(setting.get());
9979
});

0 commit comments

Comments
 (0)