mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.97k stars 519 forks source link

test(browser): add failing tests for xhr upload events #2262

Closed gmarcos87 closed 1 month ago

gmarcos87 commented 2 months ago

This PR shows that the upload events in xhr are not being fired correctly.

kettanaito commented 2 months ago

Thanks for submitting this, @gmarcos87!

If you experience this in the browser, then I'm afraid there's little we can do. MSW uses a Service Worker to intercept XHR in the browser, so if the request doesn't fire its upload events, it's either an issue with the request or the way how the Service Worker API handles responses to XHR.

We've implemented the proper upload event support to XHR in Node.js though! (if using msw/node). Those fixes are not a part of MSW just yet.

kettanaito commented 2 months ago

I'm fairly confident this is going to be fixed by https://github.com/mswjs/interceptors/releases/tag/v0.34.2.

gmarcos87 commented 2 months ago

I'm fairly confident this is going to be fixed by https://github.com/mswjs/interceptors/releases/tag/v0.34.2.

Thank you very much @kettanaito! Tomorrow I will test the release on my source code and see if that fixes the tests.

kettanaito commented 2 months ago

@gmarcos87, a new version of MSW will release today containing that fix. Let me know how it goes!

gmarcos87 commented 2 months ago

Ok, I tested it and it works! But... It works the first time I run the tests, if I do it in “watch” mode the following re-runs of the tests don't trigger the callback. I have updated both msw and re-generated the service worker.

kettanaito commented 2 months ago

@gmarcos87, are these browser or Node.js tests? Please share a reproduction repo. This may have something to do with your test's setup.

kettanaito commented 1 month ago

I will close this one. We can afford not to test XHR upload in the browser because we rely on the Service Worker API in the browser. If it supports XHR uploads, which I think it does, we are fine. If it doesn't, we are also fine (just need to mention that in the docs).

No test is necessary for this. We have an extensive test suit for XHR in Node.js that uses a different interception algorithm. Thanks for working on this nonetheless.