Skip to content

Commit fb31435

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
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 <bmeurer@chromium.org> Reviewed-by: Changhao Han <changhaohan@chromium.org>
1 parent 5cb9f98 commit fb31435

24 files changed

+223
-70
lines changed

.eslintrc.js

Lines changed: 1 addition & 0 deletions
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',

front_end/models/trace/ModelImpl.test.ts

Lines changed: 2 additions & 2 deletions
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() {

front_end/models/trace/lantern/core/NetworkAnalyzer.test.ts

Lines changed: 1 addition & 1 deletion
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', () => {

front_end/models/trace/lantern/graph/BaseNode.test.ts

Lines changed: 6 additions & 6 deletions
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
});

front_end/models/trace/lantern/graph/PageDependencyGraph.test.ts

Lines changed: 1 addition & 1 deletion
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]);

front_end/models/trace/lantern/metrics/FirstContentfulPaint.test.ts

Lines changed: 2 additions & 2 deletions
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 () => {

front_end/models/trace/lantern/metrics/Interactive.test.ts

Lines changed: 4 additions & 4 deletions
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
});

front_end/models/trace/lantern/metrics/LargestContentfulPaint.test.ts

Lines changed: 2 additions & 2 deletions
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
});

front_end/models/trace/lantern/simulation/ConnectionPool.test.ts

Lines changed: 15 additions & 15 deletions
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
});

front_end/models/trace/lantern/simulation/Simulator.test.ts

Lines changed: 1 addition & 1 deletion
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
});

0 commit comments

Comments
 (0)