Skip to content

Commit fb31435

Browse files
bmeurerDevtools-frontend LUCI CQ
authored and
Devtools-frontend LUCI CQ
committed
Consistently prefer assert.isOk and assert.isNotOk.
`assert.ok` is an alias for `assert.isOk`, and similarly `assert.notOk` is an alias for `assert.isNotOk`. For consistency with other assertions such as `assert.isNull` and `assert.isNotNull`, we enforce the use of the slightly longer form here as well. Bug: 386335487 Change-Id: If845a5675a78598d01b30532985239f74f701db7 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6120409 Commit-Queue: Benedikt Meurer <[email protected]> Reviewed-by: Changhao Han <[email protected]>
1 parent 5cb9f98 commit fb31435

24 files changed

+223
-70
lines changed

Diff for: .eslintrc.js

+1
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ module.exports = {
298298
'rulesdir/no-assert-equal-boolean-null-undefined' : 'error',
299299
'rulesdir/no-repeated-tests' : 'error',
300300
'rulesdir/no-screenshot-test-outside-perf-panel' : 'error',
301+
'rulesdir/prefer-assert-is-ok' : 'error',
301302
'rulesdir/prefer-assert-length-of' : 'error',
302303
'rulesdir/trace-engine-test-timeouts' : 'error',
303304
'@typescript-eslint/no-non-null-assertion' : 'off',

Diff for: front_end/models/trace/ModelImpl.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ describeWithEnvironment('TraceModel', function() {
1717
});
1818

1919
await TraceLoader.rawEvents(this, 'basic.json.gz').then(events => model.parse(events));
20-
assert.ok(events.includes('PROGRESS_UPDATE'));
21-
assert.ok(events.includes('COMPLETE'));
20+
assert.isOk(events.includes('PROGRESS_UPDATE'));
21+
assert.isOk(events.includes('COMPLETE'));
2222
});
2323

2424
it('supports parsing a generic trace that has no browser specific details', async function() {

Diff for: front_end/models/trace/lantern/core/NetworkAnalyzer.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('NetworkAnalyzer', () => {
5757

5858
function assertCloseEnough(valueA, valueB, threshold = 1) {
5959
const message = `${valueA} was not close enough to ${valueB}`;
60-
assert.ok(Math.abs(valueA - valueB) < threshold, message);
60+
assert.isOk(Math.abs(valueA - valueB) < threshold, message);
6161
}
6262

6363
describe('#estimateIfConnectionWasReused', () => {

Diff for: front_end/models/trace/lantern/graph/BaseNode.test.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,8 @@ describe('BaseNode', () => {
137137
const networkNode = new NetworkNode({});
138138
networkNode.setIsMainDocument(true);
139139

140-
assert.ok(node.cloneWithoutRelationships().isMainDocument());
141-
assert.ok(networkNode.cloneWithoutRelationships().isMainDocument());
140+
assert.isOk(node.cloneWithoutRelationships().isMainDocument());
141+
assert.isOk(networkNode.cloneWithoutRelationships().isMainDocument());
142142
});
143143
});
144144

@@ -226,10 +226,10 @@ describe('BaseNode', () => {
226226
clone.traverse(node => clonedIdMap.set(node.id, node));
227227

228228
assert.strictEqual(clonedIdMap.size, 6);
229-
assert.ok(clonedIdMap.has('F'), 'did not include target node');
230-
assert.ok(clonedIdMap.has('E'), 'did not include dependency');
231-
assert.ok(clonedIdMap.has('B'), 'did not include branched dependency');
232-
assert.ok(clonedIdMap.has('C'), 'did not include branched dependency');
229+
assert.isOk(clonedIdMap.has('F'), 'did not include target node');
230+
assert.isOk(clonedIdMap.has('E'), 'did not include dependency');
231+
assert.isOk(clonedIdMap.has('B'), 'did not include branched dependency');
232+
assert.isOk(clonedIdMap.has('C'), 'did not include branched dependency');
233233
assert.isUndefined(clonedIdMap.get('G'));
234234
assert.isUndefined(clonedIdMap.get('H'));
235235
});

Diff for: front_end/models/trace/lantern/graph/PageDependencyGraph.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ describe('PageDependencyGraph', () => {
7171
const networkNodeOutput = PageDependencyGraph.getNetworkNodeOutput(networkRequests);
7272
for (let i = 0; i < networkRequests.length; i++) {
7373
const node = networkNodeOutput.nodes[i];
74-
assert.ok(node, `did not create node at index ${i}`);
74+
assert.isOk(node, `did not create node at index ${i}`);
7575
assert.strictEqual(node.id, i + 1);
7676
assert.strictEqual(node.type, 'network');
7777
assert.strictEqual(node.request, networkRequests[i]);

Diff for: front_end/models/trace/lantern/metrics/FirstContentfulPaint.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ describe('Metrics: Lantern FCP', () => {
3333
optimisticNodeTimings: 4,
3434
pessimisticNodeTimings: 4,
3535
});
36-
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
37-
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
36+
assert.isOk(result.optimisticGraph, 'should have created optimistic graph');
37+
assert.isOk(result.pessimisticGraph, 'should have created pessimistic graph');
3838
});
3939

4040
it('should handle negative request networkEndTime', async () => {

Diff for: front_end/models/trace/lantern/metrics/Interactive.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ describe('Metrics: Lantern TTI', () => {
3737
});
3838
assert.strictEqual(result.optimisticEstimate.nodeTimings.size, 14);
3939
assert.strictEqual(result.pessimisticEstimate.nodeTimings.size, 31);
40-
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
41-
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
40+
assert.isOk(result.optimisticGraph, 'should have created optimistic graph');
41+
assert.isOk(result.pessimisticGraph, 'should have created pessimistic graph');
4242
});
4343

4444
it('should compute predicted value on iframes with substantial layout', async () => {
@@ -62,7 +62,7 @@ describe('Metrics: Lantern TTI', () => {
6262
pessimistic: 2386,
6363
timing: 2379,
6464
});
65-
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
66-
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
65+
assert.isOk(result.optimisticGraph, 'should have created optimistic graph');
66+
assert.isOk(result.pessimisticGraph, 'should have created pessimistic graph');
6767
});
6868
});

Diff for: front_end/models/trace/lantern/metrics/LargestContentfulPaint.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('Metrics: Lantern LCP', () => {
3535
optimisticNodeTimings: 8,
3636
pessimisticNodeTimings: 9,
3737
});
38-
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
39-
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
38+
assert.isOk(result.optimisticGraph, 'should have created optimistic graph');
39+
assert.isOk(result.pessimisticGraph, 'should have created pessimistic graph');
4040
});
4141
});

Diff for: front_end/models/trace/lantern/simulation/ConnectionPool.test.ts

+15-15
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,29 @@ describe('ConnectionPool', () => {
6161
const recordA = request({url: 'https://example.com'});
6262
const pool = new ConnectionPool([recordA], simulationOptions({rtt, throughput}));
6363
const connection = pool.connectionsByOrigin.get('https://example.com')[0];
64-
assert.ok(connection.ssl, 'should have set connection TLS');
64+
assert.isOk(connection.ssl, 'should have set connection TLS');
6565
});
6666

6767
it('should set H2 properly', () => {
6868
const recordA = request({protocol: 'h2'});
6969
const pool = new ConnectionPool([recordA], simulationOptions({rtt, throughput}));
7070
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
71-
assert.ok(connection.isH2(), 'should have set HTTP/2');
71+
assert.isOk(connection.isH2(), 'should have set HTTP/2');
7272
assert.lengthOf(pool.connectionsByOrigin.get('http://example.com'), 1);
7373
});
7474

7575
it('should set origin-specific RTT properly', () => {
7676
const additionalRttByOrigin = new Map([['http://example.com', 63]]);
7777
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput, additionalRttByOrigin}));
7878
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
79-
assert.ok(connection.rtt, rtt + 63);
79+
assert.isOk(connection.rtt, rtt + 63);
8080
});
8181

8282
it('should set origin-specific server latency properly', () => {
8383
const serverResponseTimeByOrigin = new Map([['http://example.com', 63]]);
8484
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput, serverResponseTimeByOrigin}));
8585
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
86-
assert.ok(connection.serverLatency, 63);
86+
assert.isOk(connection.serverLatency, 63);
8787
});
8888
});
8989

@@ -106,17 +106,17 @@ describe('ConnectionPool', () => {
106106
it('should allocate at least 6 connections', () => {
107107
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput}));
108108
for (let i = 0; i < 6; i++) {
109-
assert.ok(pool.acquire(request()), `did not find connection for ${i}th request`);
109+
assert.isOk(pool.acquire(request()), `did not find connection for ${i}th request`);
110110
}
111111
});
112112

113113
it('should allocate all connections', () => {
114114
const records = new Array(7).fill(undefined, 0, 7).map(() => request());
115115
const pool = new ConnectionPool(records, simulationOptions({rtt, throughput}));
116116
const connections = records.map(request => pool.acquire(request));
117-
assert.ok(connections[0], 'did not find connection for 1st request');
118-
assert.ok(connections[5], 'did not find connection for 6th request');
119-
assert.ok(connections[6], 'did not find connection for 7th request');
117+
assert.isOk(connections[0], 'did not find connection for 1st request');
118+
assert.isOk(connections[5], 'did not find connection for 6th request');
119+
assert.isOk(connections[6], 'did not find connection for 7th request');
120120
});
121121

122122
it('should be oblivious to connection reuse', () => {
@@ -125,16 +125,16 @@ describe('ConnectionPool', () => {
125125
const pool = new ConnectionPool([coldRecord, warmRecord], simulationOptions({rtt, throughput}));
126126
pool.connectionReusedByRequestId.set(warmRecord.requestId, true);
127127

128-
assert.ok(pool.acquire(coldRecord), 'should have acquired connection');
129-
assert.ok(pool.acquire(warmRecord), 'should have acquired connection');
128+
assert.isOk(pool.acquire(coldRecord), 'should have acquired connection');
129+
assert.isOk(pool.acquire(warmRecord), 'should have acquired connection');
130130
pool.release(coldRecord);
131131

132132
for (const connection of pool.connectionsByOrigin.get('http://example.com')) {
133133
connection.setWarmed(true);
134134
}
135135

136-
assert.ok(pool.acquire(coldRecord), 'should have acquired connection');
137-
assert.ok(pool.acquireActiveConnectionFromRequest(warmRecord), 'should have acquired connection');
136+
assert.isOk(pool.acquire(coldRecord), 'should have acquired connection');
137+
assert.isOk(pool.acquireActiveConnectionFromRequest(warmRecord), 'should have acquired connection');
138138
});
139139

140140
it('should acquire in order of warmness', () => {
@@ -175,13 +175,13 @@ describe('ConnectionPool', () => {
175175
requests.forEach(request => pool.acquire(request));
176176

177177
assert.lengthOf(pool.connectionsInUse(), 6);
178-
assert.ok(!pool.acquire(requests[6]), 'had connection that is in use');
178+
assert.isOk(!pool.acquire(requests[6]), 'had connection that is in use');
179179

180180
pool.release(requests[0]);
181181
assert.lengthOf(pool.connectionsInUse(), 5);
182182

183-
assert.ok(pool.acquire(requests[6]), 'could not reissue released connection');
184-
assert.ok(!pool.acquire(requests[0]), 'had connection that is in use');
183+
assert.isOk(pool.acquire(requests[6]), 'could not reissue released connection');
184+
assert.isOk(!pool.acquire(requests[0]), 'had connection that is in use');
185185
});
186186
});
187187
});

Diff for: front_end/models/trace/lantern/simulation/Simulator.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('DependencyGraph/Simulator', () => {
7070

7171
function assertNodeTiming(result, node, assertions) {
7272
const timing = result.nodeTimings.get(node);
73-
assert.ok(timing, 'missing node timing information');
73+
assert.isOk(timing, 'missing node timing information');
7474
Object.keys(assertions).forEach(key => {
7575
assert.strictEqual(timing[key], assertions[key]);
7676
});

Diff for: front_end/models/trace/lantern/simulation/TCPConnection.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe('TCPConnection', () => {
1212
const rtt = 150;
1313
const throughput = 1600 * 1024;
1414
const connection = new TCPConnection(rtt, throughput);
15-
assert.ok(connection);
15+
assert.isOk(connection);
1616
assert.strictEqual(connection.rtt, rtt);
1717
});
1818
});

Diff for: front_end/panels/ai_assistance/components/ChatView.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ css
5959
const elem = renderToElem(linkCase, linklessRenderer);
6060
assert.lengthOf(elem.querySelectorAll('a, x-link, devtools-link'), 0);
6161
assert.isFalse(['<a', '<x-link', '<devtools-link'].some(tagName => elem.outerHTML.includes(tagName)));
62-
assert.ok(elem.textContent?.includes('( https://z.com )'), linkCase);
62+
assert.isOk(elem.textContent?.includes('( https://z.com )'), linkCase);
6363
}
6464
});
6565

@@ -90,7 +90,7 @@ css
9090
assert.isFalse(['<a', '<x-link', '<devtools-link', '<img', '<devtools-markdown-image'].some(
9191
tagName => elem.outerHTML.includes(tagName)));
9292

93-
assert.ok(elem.textContent?.includes('( https://z.com/i.png )'), imageCase);
93+
assert.isOk(elem.textContent?.includes('( https://z.com/i.png )'), imageCase);
9494
}
9595
});
9696
});

Diff for: front_end/panels/recorder/RecorderController.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describeWithEnvironment('RecorderController', () => {
5757
const createRecordingView = controller.shadowRoot?.querySelector(
5858
'devtools-create-recording-view',
5959
);
60-
assert.ok(createRecordingView);
60+
assert.isOk(createRecordingView);
6161
createRecordingView?.dispatchEvent(
6262
new Components.CreateRecordingView.RecordingCancelledEvent(),
6363
);
@@ -74,7 +74,7 @@ describeWithEnvironment('RecorderController', () => {
7474
const recordingView = controller.shadowRoot?.querySelector(
7575
'devtools-recording-view',
7676
);
77-
assert.ok(recordingView);
77+
assert.isOk(recordingView);
7878
recordingView?.dispatchEvent(event);
7979
await coordinator.done();
8080
}

Diff for: front_end/panels/recorder/components/CreateRecordingView.test.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ describeWithEnvironment('CreateRecordingView', () => {
3030
const input = view.shadowRoot?.querySelector(
3131
'#user-flow-name',
3232
) as HTMLInputElement;
33-
assert.ok(input);
33+
assert.isOk(input);
3434
const button = view.shadowRoot?.querySelector(
3535
'devtools-control-button',
3636
) as Components.ControlButton.ControlButton;
37-
assert.ok(button);
37+
assert.isOk(button);
3838
const onceClicked = new Promise<Components.CreateRecordingView.RecordingStartedEvent>(
3939
resolve => {
4040
view.addEventListener('recordingstarted', resolve, {once: true});
@@ -78,11 +78,11 @@ describeWithEnvironment('CreateRecordingView', () => {
7878
let input = view.shadowRoot?.querySelector(
7979
'#selector-attribute',
8080
) as HTMLInputElement;
81-
assert.ok(input);
81+
assert.isOk(input);
8282
const button = view.shadowRoot?.querySelector(
8383
'devtools-control-button',
8484
) as Components.ControlButton.ControlButton;
85-
assert.ok(button);
85+
assert.isOk(button);
8686
const onceClicked = new Promise<Components.CreateRecordingView.RecordingStartedEvent>(
8787
resolve => {
8888
view.addEventListener('recordingstarted', resolve, {once: true});
@@ -96,7 +96,7 @@ describeWithEnvironment('CreateRecordingView', () => {
9696
input = view.shadowRoot?.querySelector(
9797
'#selector-attribute',
9898
) as HTMLInputElement;
99-
assert.ok(input);
99+
assert.isOk(input);
100100
assert.strictEqual(input.value, 'data-custom-attribute');
101101
});
102102

@@ -110,7 +110,7 @@ describeWithEnvironment('CreateRecordingView', () => {
110110
const button = view.shadowRoot?.querySelector(
111111
'devtools-control-button',
112112
) as Components.ControlButton.ControlButton;
113-
assert.ok(button);
113+
assert.isOk(button);
114114
const onceClicked = new Promise<Components.CreateRecordingView.RecordingStartedEvent>(
115115
resolve => {
116116
view.addEventListener('recordingstarted', resolve, {once: true});

Diff for: front_end/panels/recorder/components/RecordingListView.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ describeWithEnvironment('RecordingListView', () => {
2222
view.recordings = [{storageName: 'storage-test', name: 'test'}];
2323
await coordinator.done();
2424
const recording = view.shadowRoot?.querySelector('.row') as HTMLDivElement;
25-
assert.ok(recording);
25+
assert.isOk(recording);
2626
const eventSent = new Promise<Components.RecordingListView.OpenRecordingEvent>(
2727
resolve => {
2828
view.addEventListener('openrecording', resolve, {once: true});
@@ -41,7 +41,7 @@ describeWithEnvironment('RecordingListView', () => {
4141
const deleteButton = view.shadowRoot?.querySelector(
4242
'.delete-recording-button',
4343
) as HTMLButtonElement;
44-
assert.ok(deleteButton);
44+
assert.isOk(deleteButton);
4545
const eventSent = new Promise<Components.RecordingListView.DeleteRecordingEvent>(
4646
resolve => {
4747
view.addEventListener('deleterecording', resolve, {once: true});
@@ -60,7 +60,7 @@ describeWithEnvironment('RecordingListView', () => {
6060
const deleteButton = view.shadowRoot?.querySelector(
6161
'.delete-recording-button',
6262
) as HTMLDivElement;
63-
assert.ok(deleteButton);
63+
assert.isOk(deleteButton);
6464
let forceResolve: Function|undefined;
6565
const eventSent = new Promise<Components.RecordingListView.OpenRecordingEvent>(
6666
resolve => {

Diff for: front_end/panels/recorder/components/RecordingView.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,15 @@ describeWithEnvironment('RecordingView', () => {
9898
const button = view.shadowRoot?.querySelector(
9999
'.show-code',
100100
) as HTMLDivElement;
101-
assert.ok(button);
101+
assert.isOk(button);
102102
dispatchClickEvent(button);
103103
}
104104

105105
function clickHideCode(view: Components.RecordingView.RecordingView) {
106106
const button = view.shadowRoot?.querySelector(
107107
'[title="Hide code"]',
108108
) as HTMLDivElement;
109-
assert.ok(button);
109+
assert.isOk(button);
110110
dispatchClickEvent(button);
111111
}
112112

@@ -122,7 +122,7 @@ describeWithEnvironment('RecordingView', () => {
122122
const menu = view.shadowRoot?.querySelector(
123123
'devtools-select-menu',
124124
) as Menus.SelectMenu.SelectMenu;
125-
assert.ok(menu);
125+
assert.isOk(menu);
126126

127127
const event = new Menus.SelectMenu.SelectMenuItemSelectedEvent(Models.ConverterIds.ConverterIds.REPLAY);
128128
menu.dispatchEvent(event);

Diff for: front_end/ui/components/split_view/SplitView.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe('SplitView', () => {
1919
const resizer = view.shadowRoot?.querySelector(
2020
'#resizer',
2121
) as HTMLDivElement;
22-
assert.ok(resizer);
22+
assert.isOk(resizer);
2323

2424
assert.strictEqual(
2525
view.style.getPropertyValue('--current-main-area-size'),
@@ -59,7 +59,7 @@ describe('SplitView', () => {
5959
const resizer = view.shadowRoot?.querySelector(
6060
'#resizer',
6161
) as HTMLDivElement;
62-
assert.ok(resizer);
62+
assert.isOk(resizer);
6363

6464
view.style.width = '600px';
6565
view.style.height = '800px';
@@ -81,7 +81,7 @@ describe('SplitView', () => {
8181
const resizer = view.shadowRoot?.querySelector(
8282
'#resizer',
8383
) as HTMLDivElement;
84-
assert.ok(resizer);
84+
assert.isOk(resizer);
8585

8686
await coordinator.done({waitForWork: true});
8787

@@ -99,7 +99,7 @@ describe('SplitView', () => {
9999
const resizer = view.shadowRoot?.querySelector(
100100
'#resizer',
101101
) as HTMLDivElement;
102-
assert.ok(resizer);
102+
assert.isOk(resizer);
103103

104104
view.style.width = '600px';
105105
view.style.height = '550px';

0 commit comments

Comments
 (0)