Skip to content

Commit

Permalink
test: Fix nock compatibility with fake timers (#24805)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

The `nock` library is not compatible with the fake timers we use in unit
tests because it uses the Node.js `timers` API. This API is not mocked
correctly by the version of Jest we are using.

Jest uses `@sinon/fake-timers` internally, which didn't support mocking
the Node.js `timers` API until v11.0.0 (see
sinonjs/fake-timers#467) This package is updated
in Jest as part of the v30 release, which is currently under
development.

To workaround this problem in the meantime, the `nock` package has been
updated and patched to use global timers rather than the `timers` API.
Global timers are mocked correctly.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24805?quickstart=1)

## **Related issues**

This was required for some unit tests that I will be submitting in a
different PR, intended for #24503

## **Manual testing steps**

N/A

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
Gudahtt authored May 28, 2024
1 parent 1fe471d commit 9971cb5
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 7 deletions.
45 changes: 45 additions & 0 deletions .yarn/patches/nock-npm-13.5.4-2c4f77b249.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
diff --git a/lib/common.js b/lib/common.js
index 336bc4d376d07306d6adf79b8e73cffd4dfff4f7..271d50d42a5a370440fd93de00072ddfdd2fcf4b 100644
--- a/lib/common.js
+++ b/lib/common.js
@@ -1,7 +1,17 @@
'use strict'

const debug = require('debug')('nock.common')
-const timers = require('timers')
+/**
+ * PATCH NOTES:
+ *
+ * Replace Node.js `timers` module with global timers, because Jest/Sinon fake timers do not work
+ * with the Node.js timers module in the version we are using.
+ *
+ * This is resolved in `@sinon/fake-timers@11`, which is introduced in the upcoming `jest@30`
+ * release.
+ *
+ * TODO: Remove this patch after updating to `jest@30`.
+ */
const url = require('url')
const util = require('util')

@@ -598,16 +608,16 @@ const intervals = []
const immediates = []

const wrapTimer =
- (timer, ids) =>
+ (timerName, ids) =>
(...args) => {
- const id = timer(...args)
+ const id = globalThis[timerName](...args)
ids.push(id)
return id
}

-const setTimeout = wrapTimer(timers.setTimeout, timeouts)
-const setInterval = wrapTimer(timers.setInterval, intervals)
-const setImmediate = wrapTimer(timers.setImmediate, immediates)
+const setTimeout = wrapTimer('setTimeout', timeouts)
+const setInterval = wrapTimer('setInterval', intervals)
+const setImmediate = wrapTimer('setImmediate', immediates)

function clearTimer(clear, ids) {
while (ids.length) {
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@
"mocha": "^10.2.0",
"mocha-junit-reporter": "^2.2.1",
"mockttp": "^3.10.1",
"nock": "^13.2.9",
"nock": "patch:nock@npm%3A13.5.4#~/.yarn/patches/nock-npm-13.5.4-2c4f77b249.patch",
"node-fetch": "^2.6.1",
"nyc": "^15.1.0",
"postcss": "^8.4.32",
Expand Down
22 changes: 16 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -25341,7 +25341,7 @@ __metadata:
mocha-junit-reporter: "npm:^2.2.1"
mockttp: "npm:^3.10.1"
nanoid: "npm:^2.1.6"
nock: "npm:^13.2.9"
nock: "patch:nock@npm%3A13.5.4#~/.yarn/patches/nock-npm-13.5.4-2c4f77b249.patch"
node-fetch: "npm:^2.6.1"
nyc: "npm:^15.1.0"
pify: "npm:^5.0.0"
Expand Down Expand Up @@ -26619,15 +26619,25 @@ __metadata:
languageName: node
linkType: hard

"nock@npm:^13.2.9":
version: 13.2.9
resolution: "nock@npm:13.2.9"
"nock@npm:13.5.4":
version: 13.5.4
resolution: "nock@npm:13.5.4"
dependencies:
debug: "npm:^4.1.0"
json-stringify-safe: "npm:^5.0.1"
propagate: "npm:^2.0.0"
checksum: 75bad391bae4efb81b742734af5f2d87309cd93d3ca6b78372fd37946d78ccb254d79104676619866915e6734abfc1b00fee2aa42073a4843ca3c746aad35a4d
languageName: node
linkType: hard

"nock@patch:nock@npm%3A13.5.4#~/.yarn/patches/nock-npm-13.5.4-2c4f77b249.patch":
version: 13.5.4
resolution: "nock@patch:nock@npm%3A13.5.4#~/.yarn/patches/nock-npm-13.5.4-2c4f77b249.patch::version=13.5.4&hash=ef8b14"
dependencies:
debug: "npm:^4.1.0"
json-stringify-safe: "npm:^5.0.1"
lodash: "npm:^4.17.21"
propagate: "npm:^2.0.0"
checksum: 38e135b41b3ba0ce3202d4079a45642eb5721f1c1c78aea6b54c668e56cc05265100899ab67376bb99aec860456d11e4e30c5e9d5be3265c522edf3df54fa172
checksum: 195540d49cbbc42a4092fe8db80ffe646919ec5abef115be70eeffba81223331e38285bc3f37c024784a2fc5dff69b237aad1fcacc821ba815f4e5b7c57333d8
languageName: node
linkType: hard

Expand Down

0 comments on commit 9971cb5

Please sign in to comment.