Skip to content

Commit 5cb9f98

Browse files
bmeurerDevtools-frontend LUCI CQ
bmeurer
authored and
Devtools-frontend LUCI CQ
committed
Disallow equality assertions with boolean, null, and undefined.
Require the more descriptive `assert.isTrue`, `assert.isFalse`, `assert.isNull`, `assert.isUndefined`, and friends instead, which also produce a more meaningful error message than the generic `assert.strictEqual`, `assert.deepEqual`, and friends. Bug: 386335487 Change-Id: Ic58a07381196c358a43857b53364d74076e9c7e8 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6113833 Reviewed-by: Changhao Han <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]>
1 parent e5afea2 commit 5cb9f98

File tree

84 files changed

+878
-605
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

84 files changed

+878
-605
lines changed

‎.eslintrc.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,9 @@ module.exports = {
293293

294294
'rulesdir/check-test-definitions' : 'error',
295295
'rulesdir/compare-arrays-with-assert-deepequal' : 'error',
296-
'rulesdir/no-assert-equal' : 'error',
297296
'rulesdir/no-assert-deep-strict-equal' : 'error',
297+
'rulesdir/no-assert-equal' : 'error',
298+
'rulesdir/no-assert-equal-boolean-null-undefined' : 'error',
298299
'rulesdir/no-repeated-tests' : 'error',
299300
'rulesdir/no-screenshot-test-outside-perf-panel' : 'error',
300301
'rulesdir/prefer-assert-length-of' : 'error',

‎front_end/core/common/CharacterIdMap.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,6 @@ describe('CharacterIdMap class', () => {
4141

4242
it('returns null when trying to convert a character that does not exist in the Map', () => {
4343
const characterIdMap = new CharacterIdMap();
44-
assert.strictEqual(characterIdMap.fromChar('!'), null, 'character was not converted correctly');
44+
assert.isNull(characterIdMap.fromChar('!'), 'character was not converted correctly');
4545
});
4646
});

‎front_end/core/common/ColorUtils.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ describe('ColorUtils', () => {
9595
});
9696

9797
it('is able to find APCA threshold by font size and weight', () => {
98-
assert.deepEqual(Common.ColorUtils.getAPCAThreshold('11px', '100'), null);
98+
assert.isNull(Common.ColorUtils.getAPCAThreshold('11px', '100'));
9999
assert.deepEqual(Common.ColorUtils.getAPCAThreshold('121px', '100'), 60);
100-
assert.deepEqual(Common.ColorUtils.getAPCAThreshold('16px', '100'), null);
100+
assert.isNull(Common.ColorUtils.getAPCAThreshold('16px', '100'));
101101
assert.deepEqual(Common.ColorUtils.getAPCAThreshold('16px', '400'), 90);
102102
assert.deepEqual(Common.ColorUtils.getAPCAThreshold('16px', '900'), 50);
103103
});

‎front_end/core/common/Console.test.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe('Console', () => {
1515
assert.lengthOf(messages, 1);
1616
assert.strictEqual(messages[0].text, 'Foo');
1717
assert.strictEqual(messages[0].level, Common.Console.MessageLevel.INFO);
18-
assert.strictEqual(messages[0].show, true);
18+
assert.isTrue(messages[0].show);
1919
});
2020

2121
it('stores messages', () => {
@@ -47,7 +47,7 @@ describe('Console', () => {
4747
console.log('Lorem Ipsum');
4848
const messages = console.messages();
4949
assert.lengthOf(messages, 1);
50-
assert.strictEqual(messages[0].show, false); // Infos don't popup the Console panel by default
50+
assert.isFalse(messages[0].show); // Infos don't popup the Console panel by default
5151
assert.strictEqual(messages[0].level, Common.Console.MessageLevel.INFO);
5252
});
5353
});
@@ -58,7 +58,7 @@ describe('Console', () => {
5858
console.warn('Lorem Ipsum');
5959
const messages = console.messages();
6060
assert.lengthOf(messages, 1);
61-
assert.strictEqual(messages[0].show, false); // Warnings don't popup the Console panel by default
61+
assert.isFalse(messages[0].show); // Warnings don't popup the Console panel by default
6262
assert.strictEqual(messages[0].level, Common.Console.MessageLevel.WARNING);
6363
});
6464
});
@@ -69,7 +69,7 @@ describe('Console', () => {
6969
console.error('Lorem Ipsum');
7070
const messages = console.messages();
7171
assert.lengthOf(messages, 1);
72-
assert.strictEqual(messages[0].show, true); // Errors popup the Console panel by default
72+
assert.isTrue(messages[0].show); // Errors popup the Console panel by default
7373
assert.strictEqual(messages[0].level, Common.Console.MessageLevel.ERROR);
7474
});
7575

@@ -79,8 +79,8 @@ describe('Console', () => {
7979
console.error('Baz', true);
8080
const messages = console.messages();
8181
assert.lengthOf(messages, 2);
82-
assert.strictEqual(messages[0].show, false);
83-
assert.strictEqual(messages[1].show, true);
82+
assert.isFalse(messages[0].show);
83+
assert.isTrue(messages[1].show);
8484
});
8585
});
8686
});

‎front_end/core/common/ParsedURL.test.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,17 @@ describe('Parsed URL', () => {
159159

160160
it('checks that URL is valid', () => {
161161
const urlTest = 'http://www.example.com#?test';
162-
assert.strictEqual(ParsedURL.isValidUrlString(urlTest), true, 'URL validation was incorrect');
162+
assert.isTrue(ParsedURL.isValidUrlString(urlTest), 'URL validation was incorrect');
163163
});
164164

165165
it('checks that file:// URL is valid', () => {
166166
const urlTest = 'file:///usr/lib';
167-
assert.strictEqual(ParsedURL.isValidUrlString(urlTest), true, 'URL validation was incorrect');
167+
assert.isTrue(ParsedURL.isValidUrlString(urlTest), 'URL validation was incorrect');
168168
});
169169

170170
it('checks that "://" is not a valid URL', () => {
171171
const urlTest = '://';
172-
assert.strictEqual(ParsedURL.isValidUrlString(urlTest), false, 'URL validation was incorrect');
172+
assert.isFalse(ParsedURL.isValidUrlString(urlTest), 'URL validation was incorrect');
173173
});
174174

175175
it('converts URL with a hash to a URL without a hash', () => {
@@ -358,7 +358,7 @@ describe('Parsed URL', () => {
358358
const hrefTest = 'www.example.com';
359359
const baseUrlTest = 'www.example.com' as Platform.DevToolsPath.UrlString;
360360
const completeUrl = ParsedURL.completeURL(baseUrlTest, hrefTest);
361-
assert.strictEqual(completeUrl, null, 'complete URL is not returned correctly');
361+
assert.isNull(completeUrl, 'complete URL is not returned correctly');
362362
});
363363

364364
it('uses the completeURL function to return the href if the base URL is a data URL', () => {
@@ -421,7 +421,7 @@ describe('Parsed URL', () => {
421421
const splitResult = ParsedURL.splitLineAndColumn(stringTest);
422422
assert.strictEqual(splitResult.url, 'http://www.example.com/foo.js', 'URL is not correct');
423423
assert.strictEqual(splitResult.lineNumber, 14, 'line number is incorrect');
424-
assert.strictEqual(splitResult.columnNumber, undefined, 'column number is incorrect');
424+
assert.isUndefined(splitResult.columnNumber, 'column number is incorrect');
425425
});
426426

427427
it('uses the splitLineAndColumn function to return the line and column numbers if the URL contains them', () => {

‎front_end/core/common/Progress.test.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('Composite Progress Bar', () => {
6464
const composite = new CompositeProgress(indicator);
6565
const subProgress = composite.createSubProgress();
6666

67-
assert.strictEqual(indicator.getTitle, undefined);
67+
assert.isUndefined(indicator.getTitle);
6868
assert.strictEqual(indicator.getWorkCompleted, 0);
6969
assert.strictEqual(indicator.getTotalWork, 1);
7070

@@ -98,7 +98,7 @@ describe('Composite Progress Bar', () => {
9898
// Creates a sub progress with the weight of 3
9999
const subProgress2 = composite.createSubProgress(3);
100100

101-
assert.strictEqual(indicator.getTitle, undefined);
101+
assert.isUndefined(indicator.getTitle);
102102
assert.strictEqual(indicator.getWorkCompleted, 0);
103103
assert.strictEqual(indicator.getTotalWork, 1);
104104

@@ -154,17 +154,17 @@ describe('Composite Progress Bar', () => {
154154
const composite1 = new CompositeProgress(subProgress01);
155155
const subProgress11 = composite1.createSubProgress(10); // Weight should have no effect
156156

157-
assert.strictEqual(indicator.getTitle, undefined);
157+
assert.isUndefined(indicator.getTitle);
158158
assert.strictEqual(indicator.getWorkCompleted, 0);
159159
assert.strictEqual(indicator.getTotalWork, 1);
160160

161161
subProgress11.setWorked(10);
162-
assert.strictEqual(indicator.getTitle, undefined);
162+
assert.isUndefined(indicator.getTitle);
163163
assert.strictEqual(indicator.getWorkCompleted, 0);
164164
assert.strictEqual(indicator.getTotalWork, 1);
165165

166166
subProgress11.setTotalWork(100);
167-
assert.strictEqual(indicator.getTitle, undefined);
167+
assert.isUndefined(indicator.getTitle);
168168
assert.strictEqual(indicator.getWorkCompleted, 0.1);
169169
assert.strictEqual(indicator.getTotalWork, 1);
170170

@@ -188,14 +188,14 @@ describe('Composite Progress Bar', () => {
188188
const composite = new CompositeProgress(indicator);
189189
const subProgress = composite.createSubProgress();
190190

191-
assert.strictEqual(indicator.getTitle, undefined);
191+
assert.isUndefined(indicator.getTitle);
192192
assert.strictEqual(indicator.getWorkCompleted, 0);
193193
assert.strictEqual(indicator.getTotalWork, 1);
194194
assert.strictEqual(subProgress.getWorked(), 0);
195195

196196
subProgress.incrementWorked();
197197

198-
assert.strictEqual(indicator.getTitle, undefined);
198+
assert.isUndefined(indicator.getTitle);
199199
assert.strictEqual(indicator.getWorkCompleted, 0);
200200
assert.strictEqual(indicator.getTotalWork, 1);
201201
assert.strictEqual(subProgress.getWorked(), 1);

‎front_end/core/common/ResolverBase.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,15 @@ describe('ResolverBase', () => {
6262
throw new Error('This should not get called');
6363
});
6464
resolver.assertIsListening();
65-
assert.strictEqual(obj, null);
65+
assert.isNull(obj);
6666
resolver.clear();
6767
});
6868

6969
it('should resolve a previously unknown object when it becomes available', async () => {
7070
const resolver = new ResolverTestImpl();
7171
const waitForCall = new Promise<TestClass>(resolve => {
7272
const obj = resolver.tryGet(id, resolve);
73-
assert.strictEqual(obj, null);
73+
assert.isNull(obj);
7474
});
7575
resolver.assertIsListening();
7676
resolver.onResolve(id, testObj);

‎front_end/core/common/ResourceType.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describeWithEnvironment('ResourceType class', () => {
3434
assert.strictEqual(
3535
resourceType.category().shortTitle(), 'Category Test Short Title',
3636
'category short title was not set correctly');
37-
assert.strictEqual(resourceType.isTextType(), true, 'resource type was not set correctly');
37+
assert.isTrue(resourceType.isTextType(), 'resource type was not set correctly');
3838
});
3939

4040
it('is able to return a document resource from the string "text/html"', () => {
@@ -411,13 +411,13 @@ describeWithEnvironment('ResourceType class', () => {
411411

412412
it('treats a Ping as Other', () => {
413413
const resourceType = resourceTypes.Ping;
414-
assert.strictEqual(resourceType.isTextType(), false, 'A ping is not a text type');
414+
assert.isFalse(resourceType.isTextType(), 'A ping is not a text type');
415415
assert.strictEqual(resourceType.canonicalMimeType(), '', 'A ping does not have an associated mime type');
416416
});
417417

418418
it('treats a CSPViolationsReport as Other', () => {
419419
const resourceType = resourceTypes.CSPViolationReport;
420-
assert.strictEqual(resourceType.isTextType(), false, 'A ping is not a text type');
420+
assert.isFalse(resourceType.isTextType(), 'A ping is not a text type');
421421
assert.strictEqual(resourceType.canonicalMimeType(), '', 'A ping does not have an associated mime type');
422422
});
423423
});

‎front_end/core/dom_extension/DOMExtension.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('DataGrid', () => {
4747
*/
4848

4949
let node: HTMLElement = component1;
50-
assert.strictEqual(node.nodeValue, null, 'root node value is incorrect');
50+
assert.isNull(node.nodeValue, 'root node value is incorrect');
5151
assert.strictEqual(node.nodeName, 'DIV', 'root node name is incorrect');
5252
assert.strictEqual(node.className, 'component1', 'root node class is incorrect');
5353

‎front_end/core/platform/StringUtilities.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ describe('StringUtilities', () => {
249249
describe('createSearchRegex', () => {
250250
it('returns a case sensitive regex if the call states it is case sensitive', () => {
251251
const regex = Platform.StringUtilities.createSearchRegex('foo', true, false);
252-
assert.strictEqual(regex.ignoreCase, false);
252+
assert.isFalse(regex.ignoreCase);
253253
assert.strictEqual(regex.source, 'foo');
254254
});
255255

‎front_end/core/sdk/AnimationModel.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ describeWithMockConnection('AnimationModel', () => {
129129
},
130130
});
131131
assert.strictEqual(animationImpl.name(), 'animation-name');
132-
assert.strictEqual(animationImpl.paused(), false);
132+
assert.isFalse(animationImpl.paused());
133133
assert.strictEqual(animationImpl.playState(), 'running');
134134
assert.strictEqual(animationImpl.playbackRate(), 1);
135135
assert.strictEqual(animationImpl.startTime(), 0);
@@ -160,7 +160,7 @@ describeWithMockConnection('AnimationModel', () => {
160160
});
161161

162162
assert.strictEqual(animationImpl.name(), 'updated-name');
163-
assert.strictEqual(animationImpl.paused(), true);
163+
assert.isTrue(animationImpl.paused());
164164
assert.strictEqual(animationImpl.playState(), 'paused');
165165
assert.strictEqual(animationImpl.playbackRate(), 2);
166166
assert.strictEqual(animationImpl.startTime(), 100);
@@ -209,7 +209,7 @@ describeWithMockConnection('AnimationModel', () => {
209209
},
210210
});
211211
assert.strictEqual(animationImpl.name(), 'animation-name');
212-
assert.strictEqual(animationImpl.paused(), false);
212+
assert.isFalse(animationImpl.paused());
213213
assert.strictEqual(animationImpl.playState(), 'running');
214214
assert.strictEqual(animationImpl.playbackRate(), 1);
215215
assert.strictEqual(animationImpl.startTime(), 100); // in pixels
@@ -245,7 +245,7 @@ describeWithMockConnection('AnimationModel', () => {
245245
});
246246

247247
assert.strictEqual(animationImpl.name(), 'updated-name');
248-
assert.strictEqual(animationImpl.paused(), true);
248+
assert.isTrue(animationImpl.paused());
249249
assert.strictEqual(animationImpl.playState(), 'paused');
250250
assert.strictEqual(animationImpl.playbackRate(), 2);
251251
assert.strictEqual(animationImpl.startTime(), 0); // in pixels

0 commit comments

Comments
 (0)