ref(test): Refactor Node integration tests to prevent nock leaks.#5579
ref(test): Refactor Node integration tests to prevent nock leaks.#5579AbhiPrasad merged 5 commits intomasterfrom
Conversation
AbhiPrasad
left a comment
There was a problem hiding this comment.
Can we update the README with these details?
IMO not the end of the world for us to have this explicit passing of server and scope. If we think it's too messy, we could also choose to create a class that manages this, and then have the url/server/scope be internal vars to that class. What do you think about that?
If we choose to do the class method, let's first merge this in to unblock Tim, and then come back to it.
| "@remix-run/dev": "^1.6.5", | ||
| "@types/react": "^17.0.47", | ||
| "@types/react-dom": "^17.0.17", | ||
| "nock": "^13.1.0", |
There was a problem hiding this comment.
did we have to add this dep?
There was a problem hiding this comment.
TS falls back to the global installation of nock, which seems to have a slight difference between what we use in node-integration-tests. Had to add this (the same version with node-integration-tests) to resolve it.
| count: number, | ||
| method: 'get' | 'post' = 'get', | ||
| endServer: boolean = true, |
There was a problem hiding this comment.
Now that there are three options, can we pass this in as an options obj (2nd arg)?
Yes, I think it would make it much cleaner and easier/safer to use. 👍 |
AbhiPrasad
left a comment
There was a problem hiding this comment.
Awesome, thanks for taking a look at this @onurtemizkan!
Currently, Node integration tests initialize nock interceptors on server initialization and end them after a certain timeout. This recently created problems with asynchronous event processors, and caused random cases of envelope leaks between tests.
To resolve it, this approach returns
http.Serverandnock.Scopeinstances back to the test, to eventually be provided to a helper that either collects envelopes or makes requests. The responsibility of ending those two instances is moved to the collection helpers likegetEnvelopeRequestorgetAPIResponse.Tests that do not use those helpers should close the server and nock interceptors manually.
This way, we ensure that we do not lose the context of the server and the nock interceptors between tests.
Needs to be tested on #5512