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

Remove request json parsing from the worker file #201

Merged
merged 4 commits into from
Jun 11, 2020
Merged

Remove request json parsing from the worker file #201

merged 4 commits into from
Jun 11, 2020

Conversation

mouhannad-sh
Copy link
Contributor

Fixes #200

Do we need to add any tests to cover the case of empty body responses ? if so, can you guide me to where I need to start ?

Thanks

@kettanaito
Copy link
Member

kettanaito commented Jun 7, 2020

Thank you for your work, @mouhannad-sh! Looks great.

Yes, let's add at least one basic test that asserts an explicit empty string body ("") and Content-Type: application/json. There's an existing test/rest-api/body.test.ts test suite that asserts different body scenarios, I'd suggest to add this one where you think it makes a logical sense.

The test performs a request with a certain options, and has a request handler in the mocks file that returns any request body as a response body.

Take a look at how test actions/assertions are written for more details, I'm sure it'd be straightforward. Let me know if you have any questions.

@kettanaito
Copy link
Member

I've rebased your branch and will provide the tests, if you don't mind.

@kettanaito
Copy link
Member

Hey, I've took this pull request as an opportunity to structure the body.test.ts test suite better. Feel free to take a look at how it's now, and at that new integration test I've added for a request with a Content-Type: application/json and no body. I've verified it fails without your change and passes with it.

Copy link
Member

@kettanaito kettanaito 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 for these changes, you make the library much better! You rock.

@kettanaito kettanaito merged commit e5ea723 into mswjs:master Jun 11, 2020
@mouhannad-sh
Copy link
Contributor Author

Thanks a lot @kettanaito you're a life saver 🔥

@sumitj18s
Copy link

Hey guys, I like this library, can you please release the current Master as we are stuck at #200 . Thanks so much!

@kettanaito
Copy link
Member

@sumitj18s thanks for the feedback. The fix is released in 0.19.1. Make sure to follow the worker file update instructions in your browser's console.

@sumitj18s
Copy link

Thanks @kettanaito a lot for such prompt release & response!

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

Successfully merging this pull request may close these issues.

Uncaught (in promise) SyntaxError: Unexpected end of input
3 participants