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

fix: remove duplicates from activeMocks() and pendingMocks() #2356

Merged

Conversation

mbargiel
Copy link
Contributor

@mbargiel mbargiel commented May 21, 2022

The interceptorScopes() function is used by activeMocks() and pendingMocks() to retrieve the list of all scopes. That list is built by concatenating the scopes of every interceptor. When multiple interceptors are registered on the same scope, the scope will be returned multiple times, causing activeMocks() and pendingMocks() to include duplicates.

This PR fixes the issue by ensuring there is no duplicate returned by interceptorScopes().

Fixes #2352 (and #600, which also reported this issue)

Copy link
Contributor

@mikicho mikicho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@mikicho
Copy link
Contributor

mikicho commented Feb 16, 2024

I was unable to merge the master to this PR to resolve the conflict, so I will wait for the contributor to do it. If not, I will open a new PR.

@mbargiel
Copy link
Contributor Author

Woah, that's quite the necro 😅 I'll see if I can find some time to remember what this was about and solve the merge conflict. (Note: I have two other PRs fixing other bugs that also have merge conflicts.)

@mbargiel mbargiel force-pushed the feature/fix-2352-remove-extraneous-active-mocks branch from 6e01cf0 to 0b6ab12 Compare February 17, 2024 16:00
@mbargiel
Copy link
Contributor Author

Well, it turns out just rebasing worked without any conflict, so here you go, updated PR :) I also locally verified that the new test I added fails on its own (ie running tests at c366ed1 results in my new test failing, then running tests on 0b6ab12 (branch head) passes).

@gr2m
Copy link
Member

gr2m commented Feb 17, 2024

Fixed the formatting and also confirmed that test is indeed failing on c366ed1

Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 💐

@gr2m gr2m merged commit 7e957b3 into nock:main Feb 17, 2024
23 checks passed
Copy link

🎉 This PR is included in version 13.5.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Quadratic number of interceptors reported by activeMocks
3 participants