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

Add an ability to clear pending interceptors in MockAgent #3737

Open
stanislav-halyn opened this issue Oct 15, 2024 · 9 comments
Open

Add an ability to clear pending interceptors in MockAgent #3737

stanislav-halyn opened this issue Oct 15, 2024 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers mocks Issues/PRs related to mocks feature

Comments

@stanislav-halyn
Copy link

This would solve...

I'm migrating from nock to undici mocks and after each test, I clear all active mocks, so that they do not interact with other test cases. This solves the problem when mocks from a failed test might interact with other test cases, making debugging difficult.

The implementation should look like...

It would be great to be able to call a cleanMocks method on MockPool or MockAgent to clean all pending mocks.

I have also considered...

I have considered creating a separate MockAgent instance for every test case, but I'm not sure it's a good idea since it can have a performance overhead and a lof of boilerplate code for cleaning up the mock agent.

I also considered using .assertNoPendingInterceptors() after each test case, but it leads to all tests failing if any of them haven't used a mocked request.

Additional context

It's the same feature nock has.

@stanislav-halyn stanislav-halyn added the enhancement New feature or request label Oct 15, 2024
@stanislav-halyn
Copy link
Author

stanislav-halyn commented Oct 15, 2024

As far as I understand the code, the implementation would be quite straightforward: I need to clean the this[kDispatches] variable in MockPool and MockClient.
If this feature gets approved, I can try to implement it.

@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina mcollina added the good first issue Good for newcomers label Oct 15, 2024
@stanislav-halyn
Copy link
Author

Sure, I will give it a try

@metcoder95 metcoder95 added the mocks Issues/PRs related to mocks feature label Oct 16, 2024
@metcoder95
Copy link
Member

I have considered creating a separate MockAgent instance for every test case, but I'm not sure it's a good idea since it can have a performance overhead and a lof of boilerplate code for cleaning up the mock agent.

Not to disregard, I agree the feature might be beneficial; but I wouldn't discard this option as adds the most isolation to your tests cases, reducing the surface of side-effects between them than can lead to flaky or weak tests.

@stanislav-halyn
Copy link
Author

@metcoder95 thanks for your answer!
My main concerns were:

  • If I call setGlobalDispatcher(mockAgent) with newly created mockAgent in each test case, won't it interact with other test cases? What if we want to call the tests concurrently?
  • won't mockAgent leave any memory leaks of I don't mockAgent.close after each test case ?

For context, I use setGlobalDispatcher in my app fetcher.
Also, I have a test file for each fastify router I have and for each test file I generate a mock agent

@mcollina
Copy link
Member

If I call setGlobalDispatcher(mockAgent) with newly created mockAgent in each test case, won't it interact with other test cases? What if we want to call the tests concurrently?

Yes.

won't mockAgent leave any memory leaks of I don't mockAgent.close after each test case ?

I don't think so

@stanislav-halyn
Copy link
Author

@mcollina thanks for the answer!
I guess it still would be beneficial to have this feature in the mock agent

@mcollina
Copy link
Member

Agreed!

@mouhannad-sh
Copy link

@stanislav-halyn I came across the same issue and I ended up writing this into my test suite setup

export const TestSuiteMockAgent = new MockAgent();
export const clearInterceptors = () => {
  if (!TestSuiteMockAgent.pendingInterceptors()?.length) return;
  const kClients = Object.getOwnPropertySymbols(TestSuiteMockAgent).find((s) => s.toString() === "Symbol(clients)");
  if (!kClients) return;
  // @ts-ignore
  const clients = [...TestSuiteMockAgent[kClients].values()];
  for (const client of clients) {
    const kDispatches = Object.getOwnPropertySymbols(client).find((s) => s.toString() === "Symbol(dispatches)");
    client[kDispatches] = [];
  }
};

I'm happy to open a PR soon and extend the mockAgent.

I'm basically clearing the dispatches array in the agent's clients, @mcollina are there any other properties / internals that also need updating ?

Potentially we could also add a selector as an argument to pick which interceptors to clear. can be useful in some sophisticated testing scenarios

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers mocks Issues/PRs related to mocks feature
Projects
None yet
Development

No branches or pull requests

6 participants
@mcollina @metcoder95 @mouhannad-sh @stanislav-halyn and others