Skip to content

Commit 9f4fd2c

Browse files
bmeurerDevtools-frontend LUCI CQ
bmeurer
authored and
Devtools-frontend LUCI CQ
committed
[eslint] Prefer sinon.assert.calledWith() and friends.
This extends the `sinon.assert` ESLint rule to also check for the following helpers: - `sinon.assert.alwaysCalledWith(spy, x, y)` - `sinon.assert.alwaysCalledWithExactly(spy, x, y)` - `sinon.assert.alwaysCalledWithMatch(spy, match)` - `sinon.assert.calledWith(spy, x, y)` - `sinon.assert.calledWithExactly(spy, x, y)` - `sinon.assert.calledOnceWithExactly(spy, x, y)` - `sinon.assert.neverCalledWith(spy, x, y)` - `sinon.assert.calledOn(spy, obj)' - `sinon.assert.alwaysCalledOn(spy, obj)` - `sinon.assert.neverCalledWith(spy, x, y)` - `sinon.assert.calledWithMatch(spy, match)` - `sinon.assert.calledOnceWithMatch(spy, match)` Using the `sinon.assert` APIs provides us with much more actionable and human readable error messages. Bug: 397260638 Change-Id: I1115c36f2840550ca6cfca78d0d9f3f4306ed3e1 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6448978 Commit-Queue: Nikolay Vitkov <nvitkov@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Auto-Submit: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Nikolay Vitkov <nvitkov@chromium.org>
1 parent 19958d5 commit 9f4fd2c

File tree

48 files changed

+383
-218
lines changed

Some content is hidden

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

48 files changed

+383
-218
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describeWithMockConnection('CPUThrottlingManager', () => {
3333
manager.setHardwareConcurrency(5);
3434

3535
sinon.assert.calledOnce(cdpStub);
36-
assert.isTrue(cdpStub.calledWithExactly({hardwareConcurrency: 5}));
36+
sinon.assert.calledWithExactly(cdpStub, {hardwareConcurrency: 5});
3737
});
3838

3939
it('does not set concurrency to 0 or negative numbers', async () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ describeWithMockConnection('ConsoleMessage', () => {
161161
runtimeModel.dispatchEventToListeners(
162162
SDK.RuntimeModel.Events.ConsoleAPICalled, {...consoleAPICall, timestamp: 123457.000});
163163
sinon.assert.calledTwice(addMessage);
164-
assert.isTrue(addMessage.secondCall.calledWith(sinon.match({messageText: 'log me'})));
164+
sinon.assert.calledWith(addMessage.secondCall, sinon.match({messageText: 'log me'}));
165165
});
166166

167167
it('clears when main frame global object cleared', async () => {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('CookieParser', () => {
101101
size: 83,
102102
},
103103
]);
104-
assert.isTrue(stub.calledOnceWithExactly('Failed getting cookie attribute: Discard'));
104+
sinon.assert.calledOnceWithExactly(stub, 'Failed getting cookie attribute: Discard');
105105
});
106106

107107
it('handles multiple cookies with an invalid attribute', () => {
@@ -121,7 +121,7 @@ describe('CookieParser', () => {
121121
size: 83,
122122
},
123123
]);
124-
assert.isTrue(stub.calledOnceWithExactly('Failed getting cookie attribute: Discard'));
124+
sinon.assert.calledOnceWithExactly(stub, 'Failed getting cookie attribute: Discard');
125125
});
126126

127127
describe('handles the SameSite attribute', () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ describeWithMockConnection('EmulationModel', () => {
2222

2323
sinon.assert.calledOnce(spySetTouchEmulationEnabled);
2424
sinon.assert.calledOnce(spySetEmitTouchEventsForMouse);
25-
assert.isTrue(spySetTouchEmulationEnabled.calledWith({enabled: true, maxTouchPoints: 1}));
25+
sinon.assert.calledWith(spySetTouchEmulationEnabled, {enabled: true, maxTouchPoints: 1});
2626
});
2727

2828
it('should `emulateTouch` not enable touch emulation when `setTouchEmulationAllowed` is called with false',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,7 @@ describeWithMockConnection('InterceptedRequest', () => {
819819
sinon.assert.notCalled(fulfillRequestSpy);
820820
await multitargetNetworkManager.requestIntercepted(interceptedRequest);
821821
await fulfilledRequest;
822-
assert.isTrue(fulfillRequestSpy.calledOnceWithExactly(expectedOverriddenResponse));
822+
sinon.assert.calledOnceWithExactly(fulfillRequestSpy, expectedOverriddenResponse);
823823
assert.deepEqual(networkRequest.setCookieHeaders, expectedSetCookieHeaders);
824824
fulfillRequestSpy.resetHistory();
825825
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,11 @@ describeWithEnvironment('OverlayPersistentHighlighter', () => {
222222

223223
await highlighter.restoreHighlightsForDocument();
224224

225-
assert(stubbedCallbacks.onGridOverlayStateChanged.calledWith(
226-
{nodeId: PATH_TO_NODE_ID_FOR_TEST[paths[0]], enabled: true}));
225+
sinon.assert.calledWith(
226+
stubbedCallbacks.onGridOverlayStateChanged, {nodeId: PATH_TO_NODE_ID_FOR_TEST[paths[0]], enabled: true});
227227
assert(highlighter.isGridHighlighted(PATH_TO_NODE_ID_FOR_TEST[paths[0]]));
228-
assert(stubbedCallbacks.onFlexOverlayStateChanged.calledWith(
229-
{nodeId: PATH_TO_NODE_ID_FOR_TEST[paths[1]], enabled: true}));
228+
sinon.assert.calledWith(
229+
stubbedCallbacks.onFlexOverlayStateChanged, {nodeId: PATH_TO_NODE_ID_FOR_TEST[paths[1]], enabled: true});
230230
assert(highlighter.isFlexHighlighted(PATH_TO_NODE_ID_FOR_TEST[paths[1]]));
231231
});
232232
});

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
7474
cacheStorageModel.enable();
7575
manager?.storageBucketCreatedOrUpdated({bucketInfo: testStorageBucketInfo});
7676

77-
assert.isTrue(trackCacheSpy.calledOnceWithExactly({storageKey: testKey}));
77+
sinon.assert.calledOnceWithExactly(trackCacheSpy, {storageKey: testKey});
7878
});
7979
});
8080

@@ -85,7 +85,7 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
8585
manager?.storageBucketCreatedOrUpdated({bucketInfo: testStorageBucketInfo});
8686
manager?.storageBucketDeleted({bucketId: testStorageBucketInfo.id});
8787

88-
assert.isTrue(untrackCacheSpy.calledOnceWithExactly({storageKey: testKey}));
88+
sinon.assert.calledOnceWithExactly(untrackCacheSpy, {storageKey: testKey});
8989
});
9090

9191
it('detaches storage key event listeners on dispose', () => {
@@ -118,7 +118,7 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
118118

119119
void cacheStorageModel.refreshCacheNames();
120120

121-
assert.isTrue(requestCacheNamesSpy.calledWithExactly({storageBucket: testStorageBucket}));
121+
sinon.assert.calledWithExactly(requestCacheNamesSpy, {storageBucket: testStorageBucket});
122122
await cacheAddedPromise;
123123
});
124124

@@ -129,9 +129,10 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
129129
cacheStorageModel.cacheStorageContentUpdated(
130130
{origin: '', storageKey: testKey, bucketId: testStorageBucketInfo.id, cacheName: 'test-cache'});
131131

132-
assert.isTrue(dispatcherSpy.calledOnceWithExactly(
132+
sinon.assert.calledOnceWithExactly(
133+
dispatcherSpy,
133134
SDK.ServiceWorkerCacheModel.Events.CACHE_STORAGE_CONTENT_UPDATED as unknown as sinon.SinonMatcher,
134-
{storageBucket: testStorageBucket, cacheName: 'test-cache'}));
135+
{storageBucket: testStorageBucket, cacheName: 'test-cache'});
135136
});
136137

137138
it('requests cache names on cacheStorageListUpdated', async () => {
@@ -141,7 +142,7 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
141142

142143
cacheStorageModel.cacheStorageListUpdated({origin: '', storageKey: testKey, bucketId: testStorageBucketInfo.id});
143144

144-
assert.isTrue(requestCacheNamesSpy.calledWithExactly({storageBucket: testStorageBucket}));
145+
sinon.assert.calledWithExactly(requestCacheNamesSpy, {storageBucket: testStorageBucket});
145146
});
146147

147148
it('gets caches added for storage key', async () => {
@@ -194,6 +195,6 @@ describeWithMockConnection('ServiceWorkerCacheModel', () => {
194195
manager?.storageBucketCreatedOrUpdated({bucketInfo: testStorageBucketInfo});
195196
cacheStorageModel.enable();
196197

197-
assert.isTrue(trackCacheSpy.calledOnceWithExactly({storageKey: testKey}));
198+
sinon.assert.calledOnceWithExactly(trackCacheSpy, {storageKey: testKey});
198199
});
199200
});

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,11 @@ describe('SourceMapManager', () => {
134134
sinon.stub(SDK.PageResourceLoader.PageResourceLoader.instance(), 'loadResource').resolves({content});
135135
sourceMapManager.attachSourceMap(client, sourceURL, sourceMappingURL);
136136
assert.strictEqual(sourceMapWillAttach.callCount, 1, 'SourceMapWillAttach events');
137-
assert.isTrue(sourceMapWillAttach.calledWith(sinon.match.hasNested('data.client', client)));
137+
sinon.assert.calledWith(sourceMapWillAttach, sinon.match.hasNested('data.client', client));
138138
const sourceMap = await sourceMapManager.sourceMapForClientPromise(client);
139139
assert.strictEqual(sourceMapAttached.callCount, 1, 'SourceMapAttached events');
140-
assert.isTrue(sourceMapAttached.calledWith(sinon.match.hasNested('data.client', client)));
141-
assert.isTrue(sourceMapAttached.calledWith(sinon.match.hasNested('data.sourceMap', sourceMap)));
140+
sinon.assert.calledWith(sourceMapAttached, sinon.match.hasNested('data.client', client));
141+
sinon.assert.calledWith(sourceMapAttached, sinon.match.hasNested('data.sourceMap', sourceMap));
142142
assert.isTrue(sourceMapAttached.calledAfter(sourceMapWillAttach));
143143
});
144144

@@ -153,10 +153,10 @@ describe('SourceMapManager', () => {
153153
sinon.stub(SDK.PageResourceLoader.PageResourceLoader.instance(), 'loadResource').rejects('Error');
154154
sourceMapManager.attachSourceMap(client, sourceURL, sourceMappingURL);
155155
assert.strictEqual(sourceMapWillAttach.callCount, 1, 'SourceMapWillAttach events');
156-
assert.isTrue(sourceMapWillAttach.calledWith(sinon.match.hasNested('data.client', client)));
156+
sinon.assert.calledWith(sourceMapWillAttach, sinon.match.hasNested('data.client', client));
157157
await sourceMapManager.sourceMapForClientPromise(client);
158158
assert.strictEqual(sourceMapFailedToAttach.callCount, 1, 'SourceMapFailedToAttach events');
159-
assert.isTrue(sourceMapFailedToAttach.calledWith(sinon.match.hasNested('data.client', client)));
159+
sinon.assert.calledWith(sourceMapFailedToAttach, sinon.match.hasNested('data.client', client));
160160
assert.isTrue(sourceMapFailedToAttach.calledAfter(sourceMapWillAttach));
161161
});
162162

@@ -171,11 +171,11 @@ describe('SourceMapManager', () => {
171171
sinon.stub(SDK.PageResourceLoader.PageResourceLoader.instance(), 'loadResource').resolves({content});
172172
sourceMapManager.attachSourceMap(client, sourceURL, sourceMappingURL);
173173
sourceMapManager.detachSourceMap(client);
174-
assert.isTrue(sourceMapFailedToAttach.calledWith(sinon.match.hasNested('data.client', client)));
174+
sinon.assert.calledWith(sourceMapFailedToAttach, sinon.match.hasNested('data.client', client));
175175
sourceMapManager.attachSourceMap(client, sourceURL, sourceMappingURL);
176176
await sourceMapManager.sourceMapForClientPromise(client);
177177
assert.strictEqual(sourceMapAttached.callCount, 1, 'SourceMapAttached events');
178-
assert.isTrue(sourceMapAttached.calledWith(sinon.match.hasNested('data.client', client)));
178+
sinon.assert.calledWith(sourceMapAttached, sinon.match.hasNested('data.client', client));
179179
assert.isTrue(sourceMapAttached.calledAfter(sourceMapFailedToAttach));
180180
});
181181

@@ -223,7 +223,7 @@ describe('SourceMapManager', () => {
223223
assert.strictEqual(loadResource.callCount, 0, 'loadResource calls');
224224
await sourceMapManager.sourceMapForClientPromise(client);
225225
assert.strictEqual(sourceMapFailedToAttach.callCount, 1, 'SourceMapFailedToAttach events');
226-
assert.isTrue(sourceMapFailedToAttach.calledWith(sinon.match.hasNested('data.client', client)));
226+
sinon.assert.calledWith(sourceMapFailedToAttach, sinon.match.hasNested('data.client', client));
227227
});
228228
});
229229

@@ -246,8 +246,8 @@ describe('SourceMapManager', () => {
246246
const sourceMap = await sourceMapManager.sourceMapForClientPromise(client);
247247
sourceMapManager.detachSourceMap(client);
248248
assert.strictEqual(sourceMapDetached.callCount, 1, 'SourceMapDetached events');
249-
assert.isTrue(sourceMapDetached.calledWith(sinon.match.hasNested('data.client', client)));
250-
assert.isTrue(sourceMapDetached.calledWith(sinon.match.hasNested('data.sourceMap', sourceMap)));
249+
sinon.assert.calledWith(sourceMapDetached, sinon.match.hasNested('data.client', client));
250+
sinon.assert.calledWith(sourceMapDetached, sinon.match.hasNested('data.sourceMap', sourceMap));
251251
});
252252

253253
it('triggers the correct lifecycle events when disabled', async () => {
@@ -281,7 +281,7 @@ describe('SourceMapManager', () => {
281281
sourceMapManager.setEnabled(false);
282282

283283
assert.strictEqual(sourceMapFailedToAttach.callCount, 1, 'SourceMapFailedToAttach events');
284-
assert.isTrue(sourceMapFailedToAttach.calledWith(sinon.match.hasNested('data.client', client)));
284+
sinon.assert.calledWith(sourceMapFailedToAttach, sinon.match.hasNested('data.client', client));
285285
});
286286

287287
it('triggers the correct lifecycle events when disabling once attached', async () => {
@@ -297,8 +297,8 @@ describe('SourceMapManager', () => {
297297
sourceMapManager.setEnabled(false);
298298

299299
assert.strictEqual(sourceMapDetached.callCount, 1, 'SourceMapDetached events');
300-
assert.isTrue(sourceMapDetached.calledWith(sinon.match.hasNested('data.client', client)));
301-
assert.isTrue(sourceMapDetached.calledWith(sinon.match.hasNested('data.sourceMap', sourceMap)));
300+
sinon.assert.calledWith(sourceMapDetached, sinon.match.hasNested('data.client', client));
301+
sinon.assert.calledWith(sourceMapDetached, sinon.match.hasNested('data.sourceMap', sourceMap));
302302
});
303303

304304
it('triggers the correct lifecycle events when re-enabling', async () => {
@@ -324,11 +324,11 @@ describe('SourceMapManager', () => {
324324
assert.strictEqual(sourceMapDetached.callCount, 0, 'SourceMapDetached events');
325325
assert.strictEqual(sourceMapFailedToAttach.callCount, 0, 'SourceMapFailedToAttach events');
326326
assert.strictEqual(sourceMapWillAttach.callCount, 1, 'SourceMapWillAttach events');
327-
assert.isTrue(sourceMapWillAttach.calledWith(sinon.match.hasNested('data.client', client)));
327+
sinon.assert.calledWith(sourceMapWillAttach, sinon.match.hasNested('data.client', client));
328328
assert.isTrue(sourceMapAttached.calledAfter(sourceMapWillAttach));
329329
assert.strictEqual(sourceMapAttached.callCount, 1, 'SourceMapAttached events');
330-
assert.isTrue(sourceMapAttached.calledWith(sinon.match.hasNested('data.client', client)));
331-
assert.isTrue(sourceMapAttached.calledWith(sinon.match.hasNested('data.sourceMap', sourceMap)));
330+
sinon.assert.calledWith(sourceMapAttached, sinon.match.hasNested('data.client', client));
331+
sinon.assert.calledWith(sourceMapAttached, sinon.match.hasNested('data.sourceMap', sourceMap));
332332
});
333333
});
334334
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ describeWithMockConnection('StorageBucketsModel', () => {
146146

147147
sinon.assert.notCalled(setStorageBucketTrackingSpy);
148148
storageKeyManager.updateStorageKeys(new Set([storageKey]));
149-
assert.isTrue(setStorageBucketTrackingSpy.calledOnceWithExactly({storageKey, enable: true}));
149+
sinon.assert.calledOnceWithExactly(setStorageBucketTrackingSpy, {storageKey, enable: true});
150150
});
151151

152152
it('adds buckets for storage keys that already have been added', async () => {
@@ -179,7 +179,7 @@ describeWithMockConnection('StorageBucketsModel', () => {
179179
storageKeyManager.updateStorageKeys(new Set([storageKey]));
180180
storageKeyManager.updateStorageKeys(new Set([]));
181181
sinon.assert.callCount(setStorageBucketTrackingSpy, 2);
182-
assert.isTrue(setStorageBucketTrackingSpy.secondCall.calledWithExactly({storageKey, enable: false}));
182+
sinon.assert.calledWithExactly(setStorageBucketTrackingSpy.secondCall, {storageKey, enable: false});
183183
});
184184

185185
it('removes all buckets for removed storage key', async () => {
@@ -378,6 +378,6 @@ describeWithMockConnection('StorageBucketsModel', () => {
378378
});
379379

380380
storageBucketsModel.deleteBucket(bucket);
381-
assert.isTrue(setStorageBucketTrackingSpy.calledOnceWithExactly({bucket}));
381+
sinon.assert.calledOnceWithExactly(setStorageBucketTrackingSpy, {bucket});
382382
});
383383
});

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describeWithMockConnection('TargetManager', () => {
3535
assert.isTrue(observer.targetAdded.calledOnceWith(target1));
3636
const target2 = createTarget();
3737
sinon.assert.calledTwice(observer.targetAdded);
38-
assert.isTrue(observer.targetAdded.calledWith(target2));
38+
sinon.assert.calledWith(observer.targetAdded, target2);
3939
target2.dispose('YOLO!');
4040
assert.isTrue(observer.targetRemoved.calledOnceWith(target2));
4141

@@ -51,7 +51,7 @@ describeWithMockConnection('TargetManager', () => {
5151
assert.isTrue(observer.modelAdded.calledOnceWith(resourceTreeModel(target1)));
5252
const target2 = createTarget();
5353
sinon.assert.calledTwice(observer.modelAdded);
54-
assert.isTrue(observer.modelAdded.calledWith(resourceTreeModel(target2)));
54+
sinon.assert.calledWith(observer.modelAdded, resourceTreeModel(target2));
5555
target2.dispose('YOLO!');
5656
assert.isTrue(observer.modelRemoved.calledOnceWith(resourceTreeModel(target2)));
5757

@@ -70,12 +70,12 @@ describeWithMockConnection('TargetManager', () => {
7070

7171
resourceTreeModel(target1).dispatchEventToListeners(WillReloadPage);
7272
sinon.assert.calledOnce(listener);
73-
assert.isTrue(listener.calledOn(thisObject));
73+
sinon.assert.calledOn(listener, thisObject);
7474

7575
const target2 = createTarget();
7676
resourceTreeModel(target2).dispatchEventToListeners(WillReloadPage);
7777
sinon.assert.calledTwice(listener);
78-
assert.isTrue(listener.calledOn(thisObject));
78+
sinon.assert.calledOn(listener, thisObject);
7979

8080
targetManager.removeModelListener(SDK.ResourceTreeModel.ResourceTreeModel, WillReloadPage, listener, thisObject);
8181
resourceTreeModel(target1).dispatchEventToListeners(WillReloadPage);
@@ -96,7 +96,7 @@ describeWithMockConnection('TargetManager', () => {
9696

9797
const subtarget1 = createTarget({parentTarget: target1});
9898
sinon.assert.calledTwice(observer.targetAdded);
99-
assert.isTrue(observer.targetAdded.calledWith(subtarget1));
99+
sinon.assert.calledWith(observer.targetAdded, subtarget1);
100100
});
101101

102102
it('allows observing models in scope', () => {
@@ -113,7 +113,7 @@ describeWithMockConnection('TargetManager', () => {
113113

114114
const subtarget1 = createTarget({parentTarget: target1});
115115
sinon.assert.calledTwice(observer.modelAdded);
116-
assert.isTrue(observer.modelAdded.calledWith(resourceTreeModel(subtarget1)));
116+
sinon.assert.calledWith(observer.modelAdded, resourceTreeModel(subtarget1));
117117
});
118118

119119
it('calls second observers even if the first is changing the scope', () => {
@@ -144,7 +144,7 @@ describeWithMockConnection('TargetManager', () => {
144144

145145
resourceTreeModel(target1).dispatchEventToListeners(WillReloadPage);
146146
sinon.assert.calledOnce(listener);
147-
assert.isTrue(listener.calledOn(thisObject));
147+
sinon.assert.calledOn(listener, thisObject);
148148

149149
const target2 = createTarget();
150150
resourceTreeModel(target2).dispatchEventToListeners(WillReloadPage);

0 commit comments

Comments
 (0)