Skip one-time request handlers inside the response reducer so it handles a scenario with parallel requests #583
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 443eb76:
|
There was a problem hiding this comment.
Hey, @fnmunhoz. Thank you for an amazing contribution!
Everything looks good. There's one comment left regarding the bookId assertions in the responses. Also, please, rebase your branch against the latest master to get the response awaiting fix so there would be no need to include query parameters in 2 requests to the same URL. Those will be awaited properly now by default.
fnmunhoz
changed the title
…les a scenario with multiple request promises
fnmunhoz
force-pushed
the
feature/one-time-handler-parallel-requests
branch
from
98f9641
to
443eb76
Compare
There was a problem hiding this comment.
The changes look great! Thank you for working on this, @fnmunhoz! It's an exemplary contribution and we're happy to receive it from you. You're awesome!
|
Yay! Thanks for all your help in the review @kettanaito and @timdeschryver 🚀 I learned a lot about MSW, and I hope I can contribute more soon! I appreciate the fantastic work you have been doing for this project. It makes my automated tests at work way better. 🤓 |
|
We will be happy to collaborate more with you, @fnmunhoz! Feel free to tackle any issues you find interesting. |
I came across an issue where a
one-timerequest handler is used more than once.After some investigation, I created an automated test where it fails (actually, it failed only with
setupServerand worked fine withsetupWorker).Apparently, the problem is related to multiple requests running in parallel (i.e., not awaited).
The test passes by moving the request handler "skip process" to inside the response reducer function.
I've also tried the MSW build with the change on the project where I initially identified the problem, and it also worked.
Please review and see if it's a viable solution or if you have other suggestions!
Ps: I've read the contribution guidelines and hope to have followed them, but I'm happy to update the PR if I missed something.