Skip to content

Commit 3a1777b

Browse files
authored
Replace @fetch-mock/jest with plain fetch-mock (#5575)
This makes the tests check what they were thinking they were checking, and gives better error messages when they fail. --- Most of our `toHaveFetched` checks weren't actually checking anything. Only the checks in shorten-url.test.ts worked. These were of the following shape: ```ts expect(window.fetch).toHaveFetched({ body: { longUrl: expectedLongUrl } }); ``` So they were passing a plain object literal (not an `expect.objectContaining`), and they were using an actual object in the `body` property, which fetch-mock compares against the JSON.parse'd body. This is the right way to use fetch-mock's "body matcher" and it worked. The check in profile-store.test.ts also worked, but only by accident: ```ts expect(window.fetch).toHaveFetched(endpointUrl, expect.anything()); ``` We could have just removed the second argument there. The uses in receive-profile.test.ts were not checking anything, and neither were the uses in AppLocalizationProvider.test.tsx. Here's the use of toHaveFetched in in receive-profile.test.ts: ```ts expect(window.fetch).toHaveFetched( 'https://symbolication.services.mozilla.com/symbolicate/v5', expect.objectContaining({ body: expect.stringMatching(/memoryMap.*firefox/), }) ); ``` If we wanted to keep using toHaveFetched here, one option would be to use fetch-mock's "body" matcher, and do something like this: ```ts expect(window.fetch).toHaveFetched( 'https://symbolication.services.mozilla.com/symbolicate/v5', { body: { job: [...] }, matchPartialBody: true, } ); ``` or in reality probably something more like this: ```ts expect(window.fetch).toHaveFetched( 'https://symbolication.services.mozilla.com/symbolicate/v5', { matcherFunction: (callLog) => { return (callLog.options.body as string).match(/memoryMap.*firefox/) !== null } } ); ``` But then the other problem is that, if these checks fail, the error messages are not all that useful. For example, if I misspell 'firefox' as 'forefix' in the example above, the error message I get is: `fetch should have been called with https://symbolication.services.mozilla.com/symbolicate/v5 and {}` Overall, my conclusion is that, if I can't use things like expect.objectContaining or expect.stringMatching with `@fetch-mock/jest`, then `@fetch-mock/jest` is not very useful and we're better off using plain fetch-mock. Accessing `lastCall()` directly lets us use these Jest matchers and produces more useful errors.
2 parents cc2349c + faeb038 commit 3a1777b

File tree

8 files changed

+107
-56
lines changed

8 files changed

+107
-56
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@
115115
"@babel/preset-react": "^7.27.1",
116116
"@babel/preset-typescript": "^7.27.1",
117117
"@eslint/js": "^9.34.0",
118-
"@fetch-mock/jest": "^0.2.16",
119118
"@testing-library/dom": "^10.4.1",
120119
"@testing-library/jest-dom": "^6.8.0",
121120
"@testing-library/react": "^16.3.0",
@@ -158,6 +157,7 @@
158157
"eslint-plugin-testing-library": "^7.6.6",
159158
"espree": "^10.4.0",
160159
"fake-indexeddb": "^6.1.0",
160+
"fetch-mock": "^12.5.3",
161161
"file-loader": "^6.2.0",
162162
"glob": "^11.0.3",
163163
"globals": "^16.3.0",
@@ -207,7 +207,7 @@
207207
"tsx"
208208
],
209209
"transformIgnorePatterns": [
210-
"/node_modules/(?!(query-string|decode-uri-component|iongraph-web|split-on-first|filter-obj|@fetch-mock/jest|fetch-mock)/)"
210+
"/node_modules/(?!(query-string|decode-uri-component|iongraph-web|split-on-first|filter-obj|fetch-mock)/)"
211211
],
212212
"moduleNameMapper": {
213213
"\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga|ftl)$": "<rootDir>/src/test/fixtures/mocks/file-mock.ts",

src/test/components/AppLocalizationProvider.test.tsx

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -161,17 +161,23 @@ describe('AppLocalizationProvider', () => {
161161

162162
expect(await screen.findByText(translatedText('de'))).toBeInTheDocument();
163163
expect(document.documentElement).toHaveAttribute('lang', 'de');
164-
expect(window.fetch).toHaveFetched('/locales/de/app.ftl', {
165-
// @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why
166-
credentials: 'include',
167-
mode: 'no-cors',
168-
});
169-
expect(window.fetch).toHaveFetched('/locales/en-US/app.ftl', {
170-
// @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why
171-
credentials: 'include',
172-
mode: 'no-cors',
173-
});
174-
expect(window.fetch).toHaveFetchedTimes(2);
164+
expect(
165+
window.fetchMock.callHistory.lastCall('/locales/de/app.ftl')?.options
166+
).toEqual(
167+
expect.objectContaining({
168+
credentials: 'include',
169+
mode: 'no-cors',
170+
})
171+
);
172+
expect(
173+
window.fetchMock.callHistory.lastCall('/locales/en-US/app.ftl')?.options
174+
).toEqual(
175+
expect.objectContaining({
176+
credentials: 'include',
177+
mode: 'no-cors',
178+
})
179+
);
180+
expect(window.fetchMock.callHistory.callLogs.length).toBe(2);
175181
});
176182

177183
it('falls back properly on en-US if the primary locale lacks a string', async () => {
@@ -193,16 +199,22 @@ describe('AppLocalizationProvider', () => {
193199
await screen.findByText(translatedText('en-US'))
194200
).toBeInTheDocument();
195201
expect(document.documentElement).toHaveAttribute('lang', 'de');
196-
expect(window.fetch).toHaveFetched('/locales/de/app.ftl', {
197-
// @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why
198-
credentials: 'include',
199-
mode: 'no-cors',
200-
});
201-
expect(window.fetch).toHaveFetched('/locales/en-US/app.ftl', {
202-
// @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why
203-
credentials: 'include',
204-
mode: 'no-cors',
205-
});
206-
expect(window.fetch).toHaveFetchedTimes(2);
202+
expect(
203+
window.fetchMock.callHistory.lastCall('/locales/de/app.ftl')?.options
204+
).toEqual(
205+
expect.objectContaining({
206+
credentials: 'include',
207+
mode: 'no-cors',
208+
})
209+
);
210+
expect(
211+
window.fetchMock.callHistory.lastCall('/locales/en-US/app.ftl')?.options
212+
).toEqual(
213+
expect.objectContaining({
214+
credentials: 'include',
215+
mode: 'no-cors',
216+
})
217+
);
218+
expect(window.fetchMock.callHistory.callLogs.length).toBe(2);
207219
});
208220
});

src/test/setup.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import '@testing-library/jest-dom';
99
import 'jest-extended';
1010

1111
// This installs jest matchers as a side effect as well.
12-
import fetchMock from '@fetch-mock/jest';
12+
import fetchMock from 'fetch-mock';
1313
import crypto from 'crypto';
1414

1515
import { NodeWorker, __shutdownWorkers } from './fixtures/node-worker';
@@ -48,7 +48,8 @@ afterEach(() => {
4848
jest.useRealTimers();
4949

5050
// Do the same with fetch mocks
51-
fetchMock.mockReset();
51+
fetchMock.removeRoutes();
52+
fetchMock.clearHistory();
5253
});
5354

5455
expect.extend({

src/test/store/receive-profile.test.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -813,8 +813,11 @@ describe('actions/receive-profile', function () {
813813
expect.any(String)
814814
);
815815

816-
expect(window.fetch).toHaveFetched(
817-
'https://symbolication.services.mozilla.com/symbolicate/v5',
816+
expect(
817+
window.fetchMock.callHistory.lastCall(
818+
'https://symbolication.services.mozilla.com/symbolicate/v5'
819+
)?.options
820+
).toEqual(
818821
expect.objectContaining({
819822
body: expect.stringMatching(/memoryMap.*firefox/),
820823
})
@@ -828,8 +831,11 @@ describe('actions/receive-profile', function () {
828831
await createBrowserConnection('Firefox/123.0');
829832
await dispatch(retrieveProfileFromBrowser(browserConnectionStatus));
830833

831-
expect(window.fetch).toHaveFetched(
832-
'https://symbolication.services.mozilla.com/symbolicate/v5',
834+
expect(
835+
window.fetchMock.callHistory.lastCall(
836+
'https://symbolication.services.mozilla.com/symbolicate/v5'
837+
)?.options
838+
).toEqual(
833839
expect.objectContaining({
834840
body: expect.stringMatching(/memoryMap.*firefox/),
835841
})
@@ -917,8 +923,11 @@ describe('actions/receive-profile', function () {
917923
const store = blankStore();
918924
await store.dispatch(retrieveProfileFromStore('FAKEHASH'));
919925

920-
expect(window.fetch).toHaveLastFetched(
921-
'https://symbolication.services.mozilla.com/symbolicate/v5',
926+
expect(
927+
window.fetchMock.callHistory.lastCall(
928+
'https://symbolication.services.mozilla.com/symbolicate/v5'
929+
)?.options
930+
).toEqual(
922931
expect.objectContaining({
923932
body: expect.stringMatching(/memoryMap.*libxul/),
924933
})
@@ -1414,8 +1423,11 @@ describe('actions/receive-profile', function () {
14141423
payload: profile,
14151424
});
14161425

1417-
expect(window.fetch).toHaveFetched(
1418-
'https://symbolication.services.mozilla.com/symbolicate/v5',
1426+
expect(
1427+
window.fetchMock.callHistory.lastCall(
1428+
'https://symbolication.services.mozilla.com/symbolicate/v5'
1429+
)?.options
1430+
).toEqual(
14191431
expect.objectContaining({
14201432
body: expect.stringMatching(/memoryMap.*firefox/),
14211433
})

src/test/unit/profile-store.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ describe('profile deletion', () => {
228228
profileToken: PROFILE_TOKEN,
229229
jwtToken: JWT_TOKEN,
230230
});
231-
expect(window.fetch).toHaveFetched(endpointUrl, expect.anything());
231+
expect(
232+
window.fetchMock.callHistory.lastCall(endpointUrl)
233+
).not.toBeUndefined();
232234
});
233235
});

src/test/unit/shorten-url.test.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,11 @@ describe('shortenUrl', () => {
6565
const shortUrl = await shortenUrl(longUrl);
6666

6767
expect(shortUrl).toBe(expectedShortUrl);
68-
// @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do
69-
// not recognize the body property, not sure why
70-
expect(window.fetch).toHaveFetched({ body: { longUrl } });
68+
expect(window.fetchMock.callHistory.lastCall()?.options).toEqual(
69+
expect.objectContaining({
70+
body: JSON.stringify({ longUrl }),
71+
})
72+
);
7173
});
7274

7375
it('changes the requested url if is not the main URL', async () => {
@@ -84,9 +86,11 @@ describe('shortenUrl', () => {
8486

8587
const shortUrl = await shortenUrl(longUrl);
8688
expect(shortUrl).toBe(expectedShortUrl);
87-
// @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do
88-
// not recognize the body property, not sure why
89-
expect(window.fetch).toHaveFetched({ body: { longUrl: expectedLongUrl } });
89+
expect(window.fetchMock.callHistory.lastCall()?.options).toEqual(
90+
expect.objectContaining({
91+
body: JSON.stringify({ longUrl: expectedLongUrl }),
92+
})
93+
);
9094
});
9195
});
9296

@@ -110,9 +114,11 @@ describe('expandUrl', () => {
110114

111115
const longUrl = await expandUrl(shortUrl);
112116
expect(longUrl).toBe(returnedLongUrl);
113-
// @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do
114-
// not recognize the body property, not sure why
115-
expect(window.fetch).toHaveFetched({ body: { shortUrl } });
117+
expect(window.fetchMock.callHistory.lastCall()?.options).toEqual(
118+
expect.objectContaining({
119+
body: JSON.stringify({ shortUrl }),
120+
})
121+
);
116122
});
117123

118124
it('forwards errors', async () => {

src/types/globals/Window.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import type { SymbolTableAsTuple } from '../../profile-logic/symbol-store-db';
66
import type { GoogleAnalytics } from '../../utils/analytics';
7-
import type FetchMockJest from '@fetch-mock/jest';
7+
import type FetchMock from 'fetch-mock';
88

99
declare global {
1010
// Because this type isn't an existing Global type, but still it's useful to
@@ -36,6 +36,6 @@ declare global {
3636
persistTooltips?: boolean;
3737

3838
// Test-only
39-
fetchMock: typeof FetchMockJest;
39+
fetchMock: typeof FetchMock;
4040
}
4141
}

yarn.lock

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,13 +1279,6 @@
12791279
"@eslint/core" "^0.15.2"
12801280
levn "^0.4.1"
12811281

1282-
"@fetch-mock/jest@^0.2.16":
1283-
version "0.2.16"
1284-
resolved "https://registry.yarnpkg.com/@fetch-mock/jest/-/jest-0.2.16.tgz#8c556edbb539101c156e2c115a95a4e33f03ce44"
1285-
integrity sha512-i/fuyWSxR5b5FowhmqJL4SPt7GuV2j7NnM/KnPK7fwucEUNuVGTrUXam8I4lXKNiBSYYpHsyExem5n/WmQMwjA==
1286-
dependencies:
1287-
fetch-mock "^12.5.3"
1288-
12891282
"@firefox-devtools/react-contextmenu@^5.2.3":
12901283
version "5.2.3"
12911284
resolved "https://registry.yarnpkg.com/@firefox-devtools/react-contextmenu/-/react-contextmenu-5.2.3.tgz#84fe061597a896ab66917914b8b975c40bd730e9"
@@ -11324,7 +11317,16 @@ string-length@^4.0.2:
1132411317
char-regex "^1.0.2"
1132511318
strip-ansi "^6.0.0"
1132611319

11327-
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
11320+
"string-width-cjs@npm:string-width@^4.2.0":
11321+
version "4.2.3"
11322+
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
11323+
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
11324+
dependencies:
11325+
emoji-regex "^8.0.0"
11326+
is-fullwidth-code-point "^3.0.0"
11327+
strip-ansi "^6.0.1"
11328+
11329+
string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
1132811330
version "4.2.3"
1132911331
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
1133011332
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@@ -11446,7 +11448,7 @@ stringify-object@^3.3.0:
1144611448
is-obj "^1.0.1"
1144711449
is-regexp "^1.0.0"
1144811450

11449-
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
11451+
"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
1145011452
version "6.0.1"
1145111453
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
1145211454
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@@ -11460,6 +11462,13 @@ strip-ansi@^0.3.0:
1146011462
dependencies:
1146111463
ansi-regex "^0.2.1"
1146211464

11465+
strip-ansi@^6.0.0, strip-ansi@^6.0.1:
11466+
version "6.0.1"
11467+
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
11468+
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
11469+
dependencies:
11470+
ansi-regex "^5.0.1"
11471+
1146311472
strip-ansi@^7.0.1, strip-ansi@^7.1.0:
1146411473
version "7.1.0"
1146511474
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
@@ -12975,7 +12984,16 @@ [email protected], workbox-window@^7.3.0:
1297512984
"@types/trusted-types" "^2.0.2"
1297612985
workbox-core "7.3.0"
1297712986

12978-
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
12987+
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
12988+
version "7.0.0"
12989+
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
12990+
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
12991+
dependencies:
12992+
ansi-styles "^4.0.0"
12993+
string-width "^4.1.0"
12994+
strip-ansi "^6.0.0"
12995+
12996+
wrap-ansi@^7.0.0:
1297912997
version "7.0.0"
1298012998
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
1298112999
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==

0 commit comments

Comments
 (0)