From a0852cd5f699fd57a027202e9a76127b7b041033 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 30 Jan 2026 21:32:14 +0100 Subject: [PATCH 1/2] chore: remove unused sinon dependency from 4 packages Remove sinon from devDependencies in packages where it was declared but not actually used: - app-metadata-controller - approval-controller - core-backend - selected-network-controller --- packages/app-metadata-controller/package.json | 1 - packages/approval-controller/package.json | 1 - packages/core-backend/package.json | 1 - packages/selected-network-controller/package.json | 1 - yarn.lock | 4 ---- 5 files changed, 8 deletions(-) diff --git a/packages/app-metadata-controller/package.json b/packages/app-metadata-controller/package.json index 91dc7d7dc69..e671cc423c1 100644 --- a/packages/app-metadata-controller/package.json +++ b/packages/app-metadata-controller/package.json @@ -57,7 +57,6 @@ "@types/jest": "^27.5.2", "deepmerge": "^4.2.2", "jest": "^27.5.1", - "sinon": "^9.2.4", "ts-jest": "^27.1.5", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", diff --git a/packages/approval-controller/package.json b/packages/approval-controller/package.json index 01c0e2d8227..9fc71354a44 100644 --- a/packages/approval-controller/package.json +++ b/packages/approval-controller/package.json @@ -60,7 +60,6 @@ "@types/jest": "^27.5.2", "deepmerge": "^4.2.2", "jest": "^27.5.1", - "sinon": "^9.2.4", "ts-jest": "^27.1.5", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", diff --git a/packages/core-backend/package.json b/packages/core-backend/package.json index d8f6756aae9..35c6423cc07 100644 --- a/packages/core-backend/package.json +++ b/packages/core-backend/package.json @@ -63,7 +63,6 @@ "@types/jest": "^27.5.2", "deepmerge": "^4.2.2", "jest": "^27.5.1", - "sinon": "^9.2.4", "ts-jest": "^27.1.5", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", diff --git a/packages/selected-network-controller/package.json b/packages/selected-network-controller/package.json index 55214eb4e65..3119dec6502 100644 --- a/packages/selected-network-controller/package.json +++ b/packages/selected-network-controller/package.json @@ -65,7 +65,6 @@ "jest": "^27.5.1", "lodash": "^4.17.21", "nock": "^13.3.1", - "sinon": "^9.2.4", "ts-jest": "^27.1.5", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", diff --git a/yarn.lock b/yarn.lock index 068ece3ca4a..9481254f45b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2633,7 +2633,6 @@ __metadata: "@types/jest": "npm:^27.5.2" deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" - sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.5" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0" @@ -2655,7 +2654,6 @@ __metadata: deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" nanoid: "npm:^3.3.8" - sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.5" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0" @@ -3104,7 +3102,6 @@ __metadata: "@types/jest": "npm:^27.5.2" deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" - sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.5" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0" @@ -4838,7 +4835,6 @@ __metadata: jest: "npm:^27.5.1" lodash: "npm:^4.17.21" nock: "npm:^13.3.1" - sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.5" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0" From c6965924252659a31cbd93b6b25b7f3a00045f6b Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Fri, 30 Jan 2026 22:01:15 +0100 Subject: [PATCH 2/2] chore: replace sinon with jest in phishing-controller Migrate all test files from sinon fake timers to Jest fake timers and manual time control pattern for async test compatibility. --- eslint-suppressions.json | 6 - packages/phishing-controller/package.json | 1 - .../src/BulkTokenScan.test.ts | 3 +- .../src/CacheManager.test.ts | 30 ++- .../src/PhishingController.test.ts | 221 +++++++++++------- .../phishing-controller/src/utils.test.ts | 25 +- yarn.lock | 1 - 7 files changed, 164 insertions(+), 123 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 0dae1f432de..2975d509f5b 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1439,9 +1439,6 @@ } }, "packages/phishing-controller/src/PhishingController.test.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 1 - }, "jest/unbound-method": { "count": 7 } @@ -1483,9 +1480,6 @@ "packages/phishing-controller/src/utils.test.ts": { "@typescript-eslint/explicit-function-return-type": { "count": 2 - }, - "import-x/namespace": { - "count": 5 } }, "packages/phishing-controller/src/utils.ts": { diff --git a/packages/phishing-controller/package.json b/packages/phishing-controller/package.json index 74a97480567..e7fe2363399 100644 --- a/packages/phishing-controller/package.json +++ b/packages/phishing-controller/package.json @@ -65,7 +65,6 @@ "deepmerge": "^4.2.2", "jest": "^27.5.1", "nock": "^13.3.1", - "sinon": "^9.2.4", "ts-jest": "^27.1.5", "typedoc": "^0.24.8", "typedoc-plugin-missing-exports": "^2.0.0", diff --git a/packages/phishing-controller/src/BulkTokenScan.test.ts b/packages/phishing-controller/src/BulkTokenScan.test.ts index 9755d0218ce..d79e4d6d28a 100644 --- a/packages/phishing-controller/src/BulkTokenScan.test.ts +++ b/packages/phishing-controller/src/BulkTokenScan.test.ts @@ -6,7 +6,6 @@ import type { MockAnyNamespace, } from '@metamask/messenger'; import nock, { cleanAll } from 'nock'; -import sinon from 'sinon'; import { PhishingController, @@ -118,7 +117,7 @@ describe('PhishingController - Bulk Token Scanning', () => { }); afterEach(() => { - sinon.restore(); + jest.restoreAllMocks(); cleanAll(); consoleErrorSpy.mockRestore(); consoleWarnSpy.mockRestore(); diff --git a/packages/phishing-controller/src/CacheManager.test.ts b/packages/phishing-controller/src/CacheManager.test.ts index 0418112cbf3..6178c23fee0 100644 --- a/packages/phishing-controller/src/CacheManager.test.ts +++ b/packages/phishing-controller/src/CacheManager.test.ts @@ -1,19 +1,16 @@ -import sinon from 'sinon'; - import { CacheManager } from './CacheManager'; import * as utils from './utils'; describe('CacheManager', () => { - let clock: sinon.SinonFakeTimers; - let updateStateSpy: sinon.SinonSpy; + let updateStateSpy: jest.Mock; let cache: CacheManager<{ value: string }>; beforeEach(() => { - clock = sinon.useFakeTimers(); - sinon - .stub(utils, 'fetchTimeNow') - .callsFake(() => Math.floor(Date.now() / 1000)); - updateStateSpy = sinon.spy(); + jest.useFakeTimers(); + jest + .spyOn(utils, 'fetchTimeNow') + .mockImplementation(() => Math.floor(Date.now() / 1000)); + updateStateSpy = jest.fn(); cache = new CacheManager<{ value: string }>({ cacheTTL: 300, // 5 minutes maxCacheSize: 3, @@ -22,7 +19,8 @@ describe('CacheManager', () => { }); afterEach(() => { - sinon.restore(); + jest.useRealTimers(); + jest.restoreAllMocks(); }); describe('constructor', () => { @@ -69,7 +67,7 @@ describe('CacheManager', () => { cache.set('key1', { value: 'value1' }); // Fast forward time past TTL - clock.tick(301 * 1000); + jest.advanceTimersByTime(301 * 1000); expect(cache.get('key1')).toBeUndefined(); }); @@ -89,7 +87,7 @@ describe('CacheManager', () => { it('should call updateState when adding entries', () => { cache.set('key1', { value: 'value1' }); - expect(updateStateSpy.calledOnce).toBe(true); + expect(updateStateSpy).toHaveBeenCalledTimes(1); }); it('should evict oldest entries when cache exceeds max size', () => { @@ -118,9 +116,9 @@ describe('CacheManager', () => { it('should call updateState when deleting entries', () => { cache.set('key1', { value: 'value1' }); - updateStateSpy.resetHistory(); + updateStateSpy.mockClear(); cache.delete('key1'); - expect(updateStateSpy.calledOnce).toBe(true); + expect(updateStateSpy).toHaveBeenCalledTimes(1); }); }); @@ -135,9 +133,9 @@ describe('CacheManager', () => { it('should call updateState', () => { cache.set('key1', { value: 'value1' }); - updateStateSpy.resetHistory(); + updateStateSpy.mockClear(); cache.clear(); - expect(updateStateSpy.calledOnce).toBe(true); + expect(updateStateSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 42a286794f7..e25087b5e07 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -7,7 +7,6 @@ import type { } from '@metamask/messenger'; import { strict as assert } from 'assert'; import nock, { cleanAll, isDone, pendingMocks } from 'nock'; -import sinon from 'sinon'; import { ListNames, @@ -41,6 +40,7 @@ import { AddressScanResultType, } from './types'; import { getHostnameFromUrl } from './utils'; +import * as utils from './utils'; const controllerName = 'PhishingController'; @@ -106,7 +106,9 @@ function setupMessenger(): { * @param options - The Phishing Controller options. * @returns The constructed Phishing Controller. */ -function getPhishingController(options?: Partial) { +function getPhishingController( + options?: Partial, +): PhishingController { const { messenger } = setupMessenger(); return new PhishingController({ messenger, @@ -115,8 +117,15 @@ function getPhishingController(options?: Partial) { } describe('PhishingController', () => { + beforeEach(() => { + jest + .spyOn(utils, 'fetchTimeNow') + .mockImplementation(() => Math.floor(Date.now() / 1000)); + }); + afterEach(() => { - sinon.restore(); + jest.useRealTimers(); + jest.restoreAllMocks(); cleanAll(); }); @@ -225,7 +234,7 @@ describe('PhishingController', () => { }); it('should not re-request when an update is in progress', async () => { - const clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); const nockScope = nock(PHISHING_CONFIG_BASE_URL) .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .delay(500) // delay promise resolution to generate "pending" state that lasts long enough to test. @@ -263,7 +272,7 @@ describe('PhishingController', () => { ], }, }); - clock.tick(1000 * 10); + jest.advanceTimersByTime(1000 * 10); const pendingUpdate = controller.updateHotlist(); expect(controller.isHotlistOutOfDate()).toBe(true); @@ -277,7 +286,15 @@ describe('PhishingController', () => { describe('maybeUpdateState', () => { let nockScope: nock.Scope; + let currentTime: number; + + const advanceTime = (seconds: number): void => { + currentTime += seconds; + }; + beforeEach(() => { + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); nockScope = nock(PHISHING_CONFIG_BASE_URL) .get(METAMASK_STALELIST_FILE) .reply(200, { @@ -318,11 +335,10 @@ describe('PhishingController', () => { }); it('should not have stalelist be out of date immediately after maybeUpdateState is called', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isStalelistOutOfDate()).toBe(true); await controller.maybeUpdateState(); expect(controller.isStalelistOutOfDate()).toBe(false); @@ -330,36 +346,33 @@ describe('PhishingController', () => { }); it('should not be out of date after maybeUpdateStalelist is called but before refresh interval has passed', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isStalelistOutOfDate()).toBe(true); await controller.maybeUpdateState(); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isStalelistOutOfDate()).toBe(false); expect(nockScope.isDone()).toBe(true); }); it('should still be out of date while update is in progress', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); // do not wait const maybeUpdatePhisingListPromise = controller.maybeUpdateState(); expect(controller.isStalelistOutOfDate()).toBe(true); await maybeUpdatePhisingListPromise; expect(controller.isStalelistOutOfDate()).toBe(false); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isStalelistOutOfDate()).toBe(true); expect(nockScope.isDone()).toBe(true); }); it('should call update only if it is out of date, otherwise it should not call update', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); @@ -383,7 +396,7 @@ describe('PhishingController', () => { type: PhishingDetectorResultType.All, }); - clock.tick(1000 * 10); + advanceTime(10); await controller.maybeUpdateState(); expect( @@ -425,12 +438,23 @@ describe('PhishingController', () => { }, ], }); - const clock = sinon.useFakeTimers(50); + currentTime = 60; const controller = getPhishingController({ hotlistRefreshInterval: 10, stalelistRefreshInterval: 50, + state: { + phishingLists: [], + whitelist: [], + whitelistPaths: {}, + hotlistLastFetched: 0, + stalelistLastFetched: 60, // recent, so stalelist is NOT out of date + c2DomainBlocklistLastFetched: 60, + urlScanCache: {}, + }, }); - clock.tick(1000 * 10); + advanceTime(10); + // At currentTime=70: stalelist (70-60=10 <= 50) NOT out of date, hotlist (70-0=70 > 10) IS out of date + expect(controller.isStalelistOutOfDate()).toBe(false); expect(controller.isHotlistOutOfDate()).toBe(true); await controller.maybeUpdateState(); expect(controller.isHotlistOutOfDate()).toBe(false); @@ -444,11 +468,10 @@ describe('PhishingController', () => { recentlyRemoved: [], lastFetchedAt: 1, }); - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(true); await controller.maybeUpdateState(); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(false); @@ -509,8 +532,7 @@ describe('PhishingController', () => { }); // Force the stalelist to be out of date and trigger update - const clock = sinon.useFakeTimers(); - clock.tick(1000 * 10); + advanceTime(10); await controller.maybeUpdateState(); @@ -531,14 +553,22 @@ describe('PhishingController', () => { name: ListNames.MetaMask, }, ]); - - clock.restore(); }); }); describe('isStalelistOutOfDate', () => { + let currentTime: number; + + const advanceTime = (seconds: number): void => { + currentTime += seconds; + }; + + beforeEach(() => { + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); + }); + it('should not be out of date upon construction', () => { - sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); @@ -547,31 +577,28 @@ describe('PhishingController', () => { }); it('should not be out of date after some of the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isStalelistOutOfDate()).toBe(false); }); it('should be out of date after the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isStalelistOutOfDate()).toBe(true); }); it('should be out of date if the refresh interval has passed and an update is in progress', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); const pendingUpdate = controller.updateStalelist(); expect(controller.isStalelistOutOfDate()).toBe(true); @@ -581,7 +608,6 @@ describe('PhishingController', () => { }); it('should not be out of date if the phishing lists were just updated', async () => { - sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); @@ -591,31 +617,39 @@ describe('PhishingController', () => { }); it('should not be out of date if the phishing lists were recently updated', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); await controller.updateStalelist(); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isStalelistOutOfDate()).toBe(false); }); it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ stalelistRefreshInterval: 10, }); await controller.updateStalelist(); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isStalelistOutOfDate()).toBe(true); }); }); describe('isHotlistOutOfDate', () => { + let currentTime: number; + + const advanceTime = (seconds: number): void => { + currentTime += seconds; + }; + + beforeEach(() => { + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); + }); + it('should not be out of date upon construction', () => { - sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); @@ -624,27 +658,24 @@ describe('PhishingController', () => { }); it('should not be out of date after some of the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isHotlistOutOfDate()).toBe(false); }); it('should be out of date after the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isHotlistOutOfDate()).toBe(true); }); it('should be out of date if the refresh interval has passed and an update is in progress', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, state: { @@ -663,7 +694,7 @@ describe('PhishingController', () => { ], }, }); - clock.tick(1000 * 10); + advanceTime(10); const pendingUpdate = controller.updateHotlist(); expect(controller.isHotlistOutOfDate()).toBe(true); @@ -673,7 +704,6 @@ describe('PhishingController', () => { }); it('should not be out of date if the phishing lists were just updated', async () => { - sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); @@ -683,31 +713,39 @@ describe('PhishingController', () => { }); it('should not be out of date if the phishing lists were recently updated', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); await controller.updateHotlist(); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isHotlistOutOfDate()).toBe(false); }); it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); await controller.updateHotlist(); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isHotlistOutOfDate()).toBe(true); }); }); describe('isC2DomainBlocklistOutOfDate', () => { + let currentTime: number; + + const advanceTime = (seconds: number): void => { + currentTime += seconds; + }; + + beforeEach(() => { + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); + }); + it('should not be out of date upon construction', () => { - sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); @@ -716,31 +754,28 @@ describe('PhishingController', () => { }); it('should not be out of date after some of the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(false); }); it('should be out of date after the refresh interval has passed', () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(true); }); it('should be out of date if the refresh interval has passed and an update is in progress', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); - clock.tick(1000 * 10); + advanceTime(10); const pendingUpdate = controller.updateC2DomainBlocklist(); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(true); @@ -750,7 +785,6 @@ describe('PhishingController', () => { }); it('should not be out of date if the C2 domain blocklist was just updated', async () => { - sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); @@ -760,30 +794,28 @@ describe('PhishingController', () => { }); it('should not be out of date if the C2 domain blocklist was recently updated', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); await controller.updateC2DomainBlocklist(); - clock.tick(1000 * 5); + advanceTime(5); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(false); }); it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { - const clock = sinon.useFakeTimers(); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); await controller.updateC2DomainBlocklist(); - clock.tick(1000 * 10); + advanceTime(10); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(true); }); }); it('should be able to change the stalelistRefreshInterval', async () => { - sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); const controller = getPhishingController({ stalelistRefreshInterval: 10 }); controller.setStalelistRefreshInterval(0); @@ -791,7 +823,7 @@ describe('PhishingController', () => { }); it('should be able to change the hotlistRefreshInterval', async () => { - sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); const controller = getPhishingController({ hotlistRefreshInterval: 10, }); @@ -801,7 +833,7 @@ describe('PhishingController', () => { }); it('should be able to change the c2DomainBlocklistRefreshInterval', async () => { - sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); const controller = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); @@ -1424,8 +1456,15 @@ describe('PhishingController', () => { }); describe('updateStalelist', () => { + let currentTime: number; + + beforeEach(() => { + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); + }); + it('should update lists with addition to hotlist', async () => { - sinon.useFakeTimers(2); + currentTime = 2; const exampleBlockedUrl = 'example-blocked-website.com'; const exampleRequestBlockedHash = '0415f1f12f07ddc4ef7e229da747c6c53a6a6474fbaf295a35d984ec0ece9455'; @@ -1482,7 +1521,7 @@ describe('PhishingController', () => { }); it('should update lists with removal diff from hotlist', async () => { - sinon.useFakeTimers(2); + currentTime = 2; const exampleBlockedUrl = 'example-blocked-website.com'; const exampleRequestBlockedHash = '0415f1f12f07ddc4ef7e229da747c6c53a6a6474fbaf295a35d984ec0ece9455'; @@ -1693,7 +1732,7 @@ describe('PhishingController', () => { describe('an update is in progress', () => { it('should not fetch phishing lists again', async () => { - const clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); const nockScope = nock(PHISHING_CONFIG_BASE_URL) .get(METAMASK_STALELIST_FILE) .delay(100) @@ -1715,7 +1754,7 @@ describe('PhishingController', () => { const firstPromise = controller.updateStalelist(); const secondPromise = controller.updateStalelist(); - clock.tick(1000 * 100); + jest.advanceTimersByTime(1000 * 100); await firstPromise; await secondPromise; @@ -1726,7 +1765,7 @@ describe('PhishingController', () => { }); it('should wait until the in-progress update has completed', async () => { - const clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); nock(PHISHING_CONFIG_BASE_URL) .get(METAMASK_STALELIST_FILE) .delay(100) @@ -1747,7 +1786,7 @@ describe('PhishingController', () => { const controller = getPhishingController(); const firstPromise = controller.updateStalelist(); const secondPromise = controller.updateStalelist(); - clock.tick(1000 * 99); + jest.advanceTimersByTime(1000 * 99); await expect(secondPromise).toNeverResolve(); @@ -2619,7 +2658,6 @@ describe('PhishingController', () => { describe('scanUrl', () => { let controller: PhishingController; - let clock: sinon.SinonFakeTimers; const testUrl: string = 'https://example.com'; const mockResponse: PhishingDetectionScanResult = { hostname: 'example.com', @@ -2628,7 +2666,7 @@ describe('PhishingController', () => { beforeEach(() => { controller = getPhishingController(); - clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); }); it('should return the scan result', async () => { @@ -2677,7 +2715,7 @@ describe('PhishingController', () => { .reply(200, {}); const promise = controller.scanUrl(testUrl); - clock.tick(8000); + jest.advanceTimersByTime(8000); const response = await promise; expect(response).toMatchObject({ hostname: '', @@ -2783,7 +2821,6 @@ describe('PhishingController', () => { describe('bulkScanUrls', () => { let controller: PhishingController; - let clock: sinon.SinonFakeTimers; const testUrls: string[] = [ 'https://example1.com', 'https://example2.com', @@ -2809,11 +2846,11 @@ describe('PhishingController', () => { beforeEach(() => { controller = getPhishingController(); - clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); }); afterEach(() => { - clock.restore(); + jest.useRealTimers(); }); it('should return the scan results for multiple URLs', async () => { @@ -2896,7 +2933,7 @@ describe('PhishingController', () => { .reply(200, {}); const promise = controller.bulkScanUrls(testUrls); - clock.tick(15000); + jest.advanceTimersByTime(15000); const response = await promise; expect(response).toStrictEqual({ results: {}, @@ -3218,7 +3255,6 @@ describe('PhishingController', () => { describe('scanAddress', () => { let controller: PhishingController; - let clock: sinon.SinonFakeTimers; const testChainId = '0x1'; const testAddress = '0x1234567890123456789012345678901234567890'; const mockResponse: AddressScanResult = { @@ -3228,11 +3264,11 @@ describe('PhishingController', () => { beforeEach(() => { controller = getPhishingController(); - clock = sinon.useFakeTimers(); + jest.useFakeTimers('legacy'); }); afterEach(() => { - clock.restore(); + jest.useRealTimers(); }); it('will return the scan result for a valid address', async () => { @@ -3286,7 +3322,7 @@ describe('PhishingController', () => { .reply(200, {}); const promise = controller.scanAddress(testChainId, testAddress); - clock.tick(5000); + jest.advanceTimersByTime(5000); const response = await promise; expect(response).toMatchObject({ result_type: AddressScanResultType.ErrorResult, @@ -3417,16 +3453,21 @@ describe('PhishingController', () => { }); describe('URL Scan Cache', () => { - let clock: sinon.SinonFakeTimers; + let currentTime: number; beforeEach(() => { - clock = sinon.useFakeTimers(); + currentTime = 0; + jest.spyOn(utils, 'fetchTimeNow').mockImplementation(() => currentTime); }); afterEach(() => { - sinon.restore(); + jest.restoreAllMocks(); cleanAll(); }); + const advanceTime = (seconds: number): void => { + currentTime += seconds; + }; + it('should cache scan results and return them on subsequent calls', async () => { const testDomain = 'example.com'; @@ -3486,12 +3527,12 @@ describe('URL Scan Cache', () => { await controller.scanUrl(`https://${testDomain}`); // Before TTL expires, should use cache - clock.tick((cacheTTL - 10) * 1000); + advanceTime(cacheTTL - 10); await controller.scanUrl(`https://${testDomain}`); expect(pendingMocks()).toHaveLength(1); // One mock remaining // After TTL expires, should fetch again - clock.tick(11 * 1000); + advanceTime(11); await controller.scanUrl(`https://${testDomain}`); expect(pendingMocks()).toHaveLength(0); // All mocks used }); @@ -3526,11 +3567,11 @@ describe('URL Scan Cache', () => { // Fill the cache await controller.scanUrl(`https://${domains[0]}`); - clock.tick(1000); // Ensure different timestamps + advanceTime(1); // Ensure different timestamps await controller.scanUrl(`https://${domains[1]}`); // This should evict the oldest entry (domain1) - clock.tick(1000); + advanceTime(1); await controller.scanUrl(`https://${domains[2]}`); // Now domain1 should not be in cache and require a new fetch @@ -3602,12 +3643,12 @@ describe('URL Scan Cache', () => { controller.setUrlScanCacheTTL(newTTL); // Before new TTL expires, should use cache - clock.tick((newTTL - 10) * 1000); + advanceTime(newTTL - 10); await controller.scanUrl(`https://${testDomain}`); expect(pendingMocks()).toHaveLength(1); // One mock remaining // After new TTL expires, should fetch again - clock.tick(11 * 1000); + advanceTime(11); await controller.scanUrl(`https://${testDomain}`); expect(pendingMocks()).toHaveLength(0); // All mocks used }); @@ -3639,9 +3680,9 @@ describe('URL Scan Cache', () => { // Fill the cache to initial size await controller.scanUrl(`https://${domains[0]}`); - clock.tick(1000); // Ensure different timestamps + advanceTime(1); // Ensure different timestamps await controller.scanUrl(`https://${domains[1]}`); - clock.tick(1000); + advanceTime(1); await controller.scanUrl(`https://${domains[2]}`); // Verify initial cache size diff --git a/packages/phishing-controller/src/utils.test.ts b/packages/phishing-controller/src/utils.test.ts index e722714f2f0..3d0b3de6076 100644 --- a/packages/phishing-controller/src/utils.test.ts +++ b/packages/phishing-controller/src/utils.test.ts @@ -1,5 +1,3 @@ -import * as sinon from 'sinon'; - import { ListKeys, ListNames } from './PhishingController'; import type { PhishingListState } from './PhishingController'; import type { TokenScanResultType } from './types'; @@ -79,15 +77,24 @@ const exampleRemoveDiff = { }; describe('fetchTimeNow', () => { + afterEach(() => { + jest.useRealTimers(); + }); + it('correctly converts time from milliseconds to seconds', () => { const testTime = 1674773005000; - sinon.useFakeTimers(testTime); + jest.useFakeTimers(); + jest.setSystemTime(testTime); const result = fetchTimeNow(); expect(result).toBe(1674773005); }); }); describe('applyDiffs', () => { + afterEach(() => { + jest.useRealTimers(); + }); + it('adds a valid addition diff to the state then sets lastUpdated to be the time of the latest diff', () => { const result = applyDiffs( exampleListState, @@ -115,7 +122,8 @@ describe('applyDiffs', () => { it('does not add an addition diff to the state if it is older than the state.lastUpdated time.', () => { const testTime = 1674773005000; - sinon.useFakeTimers(testTime); + jest.useFakeTimers(); + jest.setSystemTime(testTime); const testExistingState = { ...exampleListState, lastUpdated: 1674773005 }; const result = applyDiffs( testExistingState, @@ -127,7 +135,8 @@ describe('applyDiffs', () => { it('does not remove a url from the state if the removal diff is older than the state.lastUpdated time.', () => { const testTime = 1674773005000; - sinon.useFakeTimers(testTime); + jest.useFakeTimers(); + jest.setSystemTime(testTime); const testExistingState = { ...exampleListState, lastUpdated: 1674773005, @@ -149,7 +158,8 @@ describe('applyDiffs', () => { it('does not add an addition diff to the state if it does not contain the same targetlist listkey.', () => { const testTime = 1674773005000; - sinon.useFakeTimers(testTime); + jest.useFakeTimers(); + jest.setSystemTime(testTime); const testExistingState = { ...exampleListState, lastUpdated: 1674773005 }; const result = applyDiffs( testExistingState, @@ -164,7 +174,8 @@ describe('applyDiffs', () => { it('does not remove a url from the state if it does not contain the same targetlist listkey.', () => { const testTime = 1674773005000; - sinon.useFakeTimers(testTime); + jest.useFakeTimers(); + jest.setSystemTime(testTime); const testExistingState = { ...exampleListState, lastUpdated: 1674773005, diff --git a/yarn.lock b/yarn.lock index 9481254f45b..d3dd7781d49 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4512,7 +4512,6 @@ __metadata: jest: "npm:^27.5.1" nock: "npm:^13.3.1" punycode: "npm:^2.1.1" - sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.5" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0"