From 0f4bf6180e29d3f59a9a69ea90f8424b33d46a49 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 27 Aug 2025 11:09:53 -0400 Subject: [PATCH] Replace @fetch-mock/jest with plain fetch-mock. 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. 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. --- package.json | 4 +- .../AppLocalizationProvider.test.tsx | 56 +++++++++++-------- src/test/setup.ts | 5 +- src/test/store/receive-profile.test.ts | 28 +++++++--- src/test/unit/profile-store.test.ts | 4 +- src/test/unit/shorten-url.test.ts | 24 +++++--- src/types/globals/Window.d.ts | 4 +- yarn.lock | 38 +++++++++---- 8 files changed, 107 insertions(+), 56 deletions(-) diff --git a/package.json b/package.json index 522473826e..10089f1388 100644 --- a/package.json +++ b/package.json @@ -115,7 +115,6 @@ "@babel/preset-react": "^7.27.1", "@babel/preset-typescript": "^7.27.1", "@eslint/js": "^9.34.0", - "@fetch-mock/jest": "^0.2.16", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^6.8.0", "@testing-library/react": "^16.3.0", @@ -158,6 +157,7 @@ "eslint-plugin-testing-library": "^7.6.6", "espree": "^10.4.0", "fake-indexeddb": "^6.1.0", + "fetch-mock": "^12.5.3", "file-loader": "^6.2.0", "glob": "^11.0.3", "globals": "^16.3.0", @@ -207,7 +207,7 @@ "tsx" ], "transformIgnorePatterns": [ - "/node_modules/(?!(query-string|decode-uri-component|iongraph-web|split-on-first|filter-obj|@fetch-mock/jest|fetch-mock)/)" + "/node_modules/(?!(query-string|decode-uri-component|iongraph-web|split-on-first|filter-obj|fetch-mock)/)" ], "moduleNameMapper": { "\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga|ftl)$": "/src/test/fixtures/mocks/file-mock.ts", diff --git a/src/test/components/AppLocalizationProvider.test.tsx b/src/test/components/AppLocalizationProvider.test.tsx index ba2532ad8e..cfb191c77c 100644 --- a/src/test/components/AppLocalizationProvider.test.tsx +++ b/src/test/components/AppLocalizationProvider.test.tsx @@ -161,17 +161,23 @@ describe('AppLocalizationProvider', () => { expect(await screen.findByText(translatedText('de'))).toBeInTheDocument(); expect(document.documentElement).toHaveAttribute('lang', 'de'); - expect(window.fetch).toHaveFetched('/locales/de/app.ftl', { - // @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why - credentials: 'include', - mode: 'no-cors', - }); - expect(window.fetch).toHaveFetched('/locales/en-US/app.ftl', { - // @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why - credentials: 'include', - mode: 'no-cors', - }); - expect(window.fetch).toHaveFetchedTimes(2); + expect( + window.fetchMock.callHistory.lastCall('/locales/de/app.ftl')?.options + ).toEqual( + expect.objectContaining({ + credentials: 'include', + mode: 'no-cors', + }) + ); + expect( + window.fetchMock.callHistory.lastCall('/locales/en-US/app.ftl')?.options + ).toEqual( + expect.objectContaining({ + credentials: 'include', + mode: 'no-cors', + }) + ); + expect(window.fetchMock.callHistory.callLogs.length).toBe(2); }); it('falls back properly on en-US if the primary locale lacks a string', async () => { @@ -193,16 +199,22 @@ describe('AppLocalizationProvider', () => { await screen.findByText(translatedText('en-US')) ).toBeInTheDocument(); expect(document.documentElement).toHaveAttribute('lang', 'de'); - expect(window.fetch).toHaveFetched('/locales/de/app.ftl', { - // @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why - credentials: 'include', - mode: 'no-cors', - }); - expect(window.fetch).toHaveFetched('/locales/en-US/app.ftl', { - // @ts-expect-error fetch-mock's TypeScript types for toHaveFetched don't know about `credentials`, not sure why - credentials: 'include', - mode: 'no-cors', - }); - expect(window.fetch).toHaveFetchedTimes(2); + expect( + window.fetchMock.callHistory.lastCall('/locales/de/app.ftl')?.options + ).toEqual( + expect.objectContaining({ + credentials: 'include', + mode: 'no-cors', + }) + ); + expect( + window.fetchMock.callHistory.lastCall('/locales/en-US/app.ftl')?.options + ).toEqual( + expect.objectContaining({ + credentials: 'include', + mode: 'no-cors', + }) + ); + expect(window.fetchMock.callHistory.callLogs.length).toBe(2); }); }); diff --git a/src/test/setup.ts b/src/test/setup.ts index 97486f458a..06124c018c 100644 --- a/src/test/setup.ts +++ b/src/test/setup.ts @@ -9,7 +9,7 @@ import '@testing-library/jest-dom'; import 'jest-extended'; // This installs jest matchers as a side effect as well. -import fetchMock from '@fetch-mock/jest'; +import fetchMock from 'fetch-mock'; import crypto from 'crypto'; import { NodeWorker, __shutdownWorkers } from './fixtures/node-worker'; @@ -48,7 +48,8 @@ afterEach(() => { jest.useRealTimers(); // Do the same with fetch mocks - fetchMock.mockReset(); + fetchMock.removeRoutes(); + fetchMock.clearHistory(); }); expect.extend({ diff --git a/src/test/store/receive-profile.test.ts b/src/test/store/receive-profile.test.ts index cc1854bf85..8af03b4340 100644 --- a/src/test/store/receive-profile.test.ts +++ b/src/test/store/receive-profile.test.ts @@ -813,8 +813,11 @@ describe('actions/receive-profile', function () { expect.any(String) ); - expect(window.fetch).toHaveFetched( - 'https://symbolication.services.mozilla.com/symbolicate/v5', + expect( + window.fetchMock.callHistory.lastCall( + 'https://symbolication.services.mozilla.com/symbolicate/v5' + )?.options + ).toEqual( expect.objectContaining({ body: expect.stringMatching(/memoryMap.*firefox/), }) @@ -828,8 +831,11 @@ describe('actions/receive-profile', function () { await createBrowserConnection('Firefox/123.0'); await dispatch(retrieveProfileFromBrowser(browserConnectionStatus)); - expect(window.fetch).toHaveFetched( - 'https://symbolication.services.mozilla.com/symbolicate/v5', + expect( + window.fetchMock.callHistory.lastCall( + 'https://symbolication.services.mozilla.com/symbolicate/v5' + )?.options + ).toEqual( expect.objectContaining({ body: expect.stringMatching(/memoryMap.*firefox/), }) @@ -917,8 +923,11 @@ describe('actions/receive-profile', function () { const store = blankStore(); await store.dispatch(retrieveProfileFromStore('FAKEHASH')); - expect(window.fetch).toHaveLastFetched( - 'https://symbolication.services.mozilla.com/symbolicate/v5', + expect( + window.fetchMock.callHistory.lastCall( + 'https://symbolication.services.mozilla.com/symbolicate/v5' + )?.options + ).toEqual( expect.objectContaining({ body: expect.stringMatching(/memoryMap.*libxul/), }) @@ -1414,8 +1423,11 @@ describe('actions/receive-profile', function () { payload: profile, }); - expect(window.fetch).toHaveFetched( - 'https://symbolication.services.mozilla.com/symbolicate/v5', + expect( + window.fetchMock.callHistory.lastCall( + 'https://symbolication.services.mozilla.com/symbolicate/v5' + )?.options + ).toEqual( expect.objectContaining({ body: expect.stringMatching(/memoryMap.*firefox/), }) diff --git a/src/test/unit/profile-store.test.ts b/src/test/unit/profile-store.test.ts index 0f4bb0578d..fb901ca318 100644 --- a/src/test/unit/profile-store.test.ts +++ b/src/test/unit/profile-store.test.ts @@ -228,6 +228,8 @@ describe('profile deletion', () => { profileToken: PROFILE_TOKEN, jwtToken: JWT_TOKEN, }); - expect(window.fetch).toHaveFetched(endpointUrl, expect.anything()); + expect( + window.fetchMock.callHistory.lastCall(endpointUrl) + ).not.toBeUndefined(); }); }); diff --git a/src/test/unit/shorten-url.test.ts b/src/test/unit/shorten-url.test.ts index 6266e85947..6191f9822b 100644 --- a/src/test/unit/shorten-url.test.ts +++ b/src/test/unit/shorten-url.test.ts @@ -65,9 +65,11 @@ describe('shortenUrl', () => { const shortUrl = await shortenUrl(longUrl); expect(shortUrl).toBe(expectedShortUrl); - // @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do - // not recognize the body property, not sure why - expect(window.fetch).toHaveFetched({ body: { longUrl } }); + expect(window.fetchMock.callHistory.lastCall()?.options).toEqual( + expect.objectContaining({ + body: JSON.stringify({ longUrl }), + }) + ); }); it('changes the requested url if is not the main URL', async () => { @@ -84,9 +86,11 @@ describe('shortenUrl', () => { const shortUrl = await shortenUrl(longUrl); expect(shortUrl).toBe(expectedShortUrl); - // @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do - // not recognize the body property, not sure why - expect(window.fetch).toHaveFetched({ body: { longUrl: expectedLongUrl } }); + expect(window.fetchMock.callHistory.lastCall()?.options).toEqual( + expect.objectContaining({ + body: JSON.stringify({ longUrl: expectedLongUrl }), + }) + ); }); }); @@ -110,9 +114,11 @@ describe('expandUrl', () => { const longUrl = await expandUrl(shortUrl); expect(longUrl).toBe(returnedLongUrl); - // @ts-expect-error TODO: fetch-mock's TypeScript types for toHaveFetched do - // not recognize the body property, not sure why - expect(window.fetch).toHaveFetched({ body: { shortUrl } }); + expect(window.fetchMock.callHistory.lastCall()?.options).toEqual( + expect.objectContaining({ + body: JSON.stringify({ shortUrl }), + }) + ); }); it('forwards errors', async () => { diff --git a/src/types/globals/Window.d.ts b/src/types/globals/Window.d.ts index ee055eaac7..583727ee12 100644 --- a/src/types/globals/Window.d.ts +++ b/src/types/globals/Window.d.ts @@ -4,7 +4,7 @@ import type { SymbolTableAsTuple } from '../../profile-logic/symbol-store-db'; import type { GoogleAnalytics } from '../../utils/analytics'; -import type FetchMockJest from '@fetch-mock/jest'; +import type FetchMock from 'fetch-mock'; declare global { // Because this type isn't an existing Global type, but still it's useful to @@ -36,6 +36,6 @@ declare global { persistTooltips?: boolean; // Test-only - fetchMock: typeof FetchMockJest; + fetchMock: typeof FetchMock; } } diff --git a/yarn.lock b/yarn.lock index a6c7424304..19d2320d5e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1279,13 +1279,6 @@ "@eslint/core" "^0.15.2" levn "^0.4.1" -"@fetch-mock/jest@^0.2.16": - version "0.2.16" - resolved "https://registry.yarnpkg.com/@fetch-mock/jest/-/jest-0.2.16.tgz#8c556edbb539101c156e2c115a95a4e33f03ce44" - integrity sha512-i/fuyWSxR5b5FowhmqJL4SPt7GuV2j7NnM/KnPK7fwucEUNuVGTrUXam8I4lXKNiBSYYpHsyExem5n/WmQMwjA== - dependencies: - fetch-mock "^12.5.3" - "@firefox-devtools/react-contextmenu@^5.2.3": version "5.2.3" 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: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + +string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -11446,7 +11448,7 @@ stringify-object@^3.3.0: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -11460,6 +11462,13 @@ strip-ansi@^0.3.0: dependencies: ansi-regex "^0.2.1" +strip-ansi@^6.0.0, strip-ansi@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + strip-ansi@^7.0.1, strip-ansi@^7.1.0: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -12975,7 +12984,16 @@ workbox-window@7.3.0, workbox-window@^7.3.0: "@types/trusted-types" "^2.0.2" workbox-core "7.3.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + +wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==