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 multipart content-type headers detection #1275

Merged
merged 4 commits into from Nov 18, 2014

Conversation

simov
Copy link
Member

@simov simov commented Nov 18, 2014

Fixes #1274
Introduced in #1216 (v2.48)

There was a logical typo. I added tests for multipart content-type headers for both cases when the json option is present or not.

@simov simov changed the title Fix multipart content-type headers check Fix multipart content-type headers detection Nov 18, 2014
@simov
Copy link
Member Author

simov commented Nov 18, 2014

Added one more test for the default content-type:multipart/related header.

Added a test for chunked transfer-encoding regarding this issue #1271 just to make it more clear what's the expected behavior.

On the other note - we should probably use a test coverage tool.

@simov
Copy link
Member Author

simov commented Nov 18, 2014

I should use this on my own whenever I add new things istanbul cover node_modules/.bin/tape tests/test-*.js

master

multipart-before

multipart-mixed

multipart-after

@nylen
Copy link
Member

nylen commented Nov 18, 2014

👍

and 👍 to code coverage too. https://coveralls.io/ looks pretty nice, it will integrate with Travis and GitHub. https://github.com/cainus/node-coveralls is what they recommend for Node.js projects.

nylen added a commit that referenced this pull request Nov 18, 2014
Fix multipart content-type headers detection
@nylen nylen merged commit 7092c68 into request:master Nov 18, 2014
@simov
Copy link
Member Author

simov commented Nov 18, 2014

re: coverage tools - 👍 nice, I'll take a look at these

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

Successfully merging this pull request may close these issues.

multipart POST broke in v2.48.0
2 participants