Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MockAgent don't intercept Pool requests #996

Open
santiagoalmeidabolannos opened this issue Aug 24, 2021 · 7 comments
Open

MockAgent don't intercept Pool requests #996

santiagoalmeidabolannos opened this issue Aug 24, 2021 · 7 comments
Labels
Docs Changes related to the documentation

Comments

@santiagoalmeidabolannos

Check this example based on these docs https://undici.nodejs.org/#/docs/api/MockPool?id=example-basic-mockpool-instantiation

When

const { MockAgent, setGlobalDispatcher, Pool } = require('undici')

const mockAgent = new MockAgent()

setGlobalDispatcher(mockAgent)

const mockPool = mockAgent.get('https://jsonplaceholder.typicode.com')


mockPool.intercept({
 path: '/todos/1',
 method: 'GET'
}).reply(200, 'foo')

async function run () {
   try {
       const agent = new Pool('https://jsonplaceholder.typicode.com', {
           headersTimeout: 30e3, // 30 seconds
           maxHeaderSize: 16_384, // 16 KiB
           keepAliveMaxTimeout: 5000, // 5 seconds
           connections: 50,
           tls: { rejectUnauthorized: false },
       })

       const { statusCode, body } = await agent.request({
           path: '/todos/1',
           method: 'GET',
       })

       console.log('response received', statusCode) // response received 200

       for await (const data of body) {
           console.log('data', data.toString('utf8')) // data foo
       }
   }
   catch (error) {
       console.log(error)
   }
}

run()

The request never gets intercepted, not sure if I am missing something here

@santiagoalmeidabolannos santiagoalmeidabolannos added the bug Something isn't working label Aug 24, 2021
@mcollina
Copy link
Member

It doesn't and that's the point. Just pass in the agent instead of your pool instance.
There is no monkeypatching involved.

@SimenB
Copy link
Member

SimenB commented Aug 24, 2021

Hiya 👋

Currently working on migrating some code from node-fetch&nock to undici, and I think I'm hitting something similar to this.

I have a certificate to use, therefore I must (I think?) create a Client rather than using the request export. But then setting the global dispatcher etc doesn't seem to have an effect. Is this expected? Or is it only possible to mock requests made with the request export? None of the examples in the docs seems to use anything else.

How would I go about mocking a request which is made using a Client? From your response it seems I need to pass something in where the new Client call is. But the Client constructor doesn't take a dispatcher argument or some such, so I'm not sure how that'd help me?

(I've only tried to use Undici for about an hour now, so I assume I'm just missing something 😅)

@mcollina
Copy link
Member

I would recommend you to structure/test your code like the following:

  1. a function that build the Client (or Pool) instance, let's call it build()
  2. a function that takes a Client/Pool and use it to make that request, let's call it execute()
  3. create a MockClient/MockPool and pass it to the second function, e.g. execute(build())

On a side note we probably need a way to specify those client credentials in some global way, the Client/Pool instances are really too low level for most folks.

@mcollina mcollina added Docs Changes related to the documentation and removed bug Something isn't working labels Aug 24, 2021
@mcollina
Copy link
Member

This needs better docs.

tomi-bigpi added a commit to tomi-bigpi/mercurius that referenced this issue Mar 8, 2022
* Add constructor argument validation to `FederatedError` to ensure `errors` is an Array
* Add input validation tests for FederatedError constructor argument
* Fix `request` and `sendRequest` to wrap caught errors in array when creating new `FederatedError`
* Add new test file for `request` to ensure array is used
* Modify existing `sendRequest` test to verify array is used
* Add support for passing in undici agent as part of the federated service configuration. Required for injection during tests. See nodejs/undici#996
mcollina pushed a commit to mercurius-js/mercurius that referenced this issue Mar 9, 2022
* Add constructor argument validation to `FederatedError` to ensure `errors` is an Array
* Add input validation tests for FederatedError constructor argument
* Fix `request` and `sendRequest` to wrap caught errors in array when creating new `FederatedError`
* Add new test file for `request` to ensure array is used
* Modify existing `sendRequest` test to verify array is used
* Add support for passing in undici agent as part of the federated service configuration. Required for injection during tests. See nodejs/undici#996
@JoonasVali
Copy link

I'm facing the same issue.

In test I set the global dispatcher as mocked

  const mockAgent = new MockAgent({ connections: 1 });
  setGlobalDispatcher(mockAgent);
  mockAgent.disableNetConnect();

In the code I have a dynamic range of Clients being created as needed.

client = new Client(new URL(endpoint.url).origin, opts);

However, later, when I call...

client.request(requestOptions);

...the client does not use the global dispatcher, which has the mocked agent set, and the HTTP call goes out.

Of course I could use some factory pattern for clients, and pass different implementation of ClientFactory, based whether I'm running tests or real instance, but I thought the idea of mocking was to avoid all that.

@mcollina
Copy link
Member

The idea is to avoid monkeypatching and minimize global state.

@tastypackets
Copy link
Contributor

It'd be really awesome if someone has time to write a doc under best practices section on suggested patterns for testing with pools/clients.

For what it's worth for future searchers, this is all you need to get a mock pool:

import { MockAgent } from "undici";
const mockAgent = new MockAgent();
const mockPool = mockAgent.get("https://www.github.com");

// This mock pool needs to be called instead of the normal pool
mockPool.request({
  path: "/",
  method: "GET"
});

You'll need to setup your own pattern for passing this pool to be used during tests or getting the mock pool pre-setup if you automate setting it up on test runs. I think a few docs would have saved me a lot of time thinking the mocks weren't working right, because setGlobalDispatcher does not work with pools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Changes related to the documentation
Projects
None yet
Development

No branches or pull requests

5 participants