Modify multipart body and calculate Content-Length in the right order

[honzajavorek]

🚀 Why this change?

Current implementation of multipart request bodies is broken. See #786.

In #734 Dredd's custom implementation of HTTP requesting was replaced with the request library. There is some special handling of multipart requests, solving apiaryio/api-blueprint#401, which modifies the body payload and also modifies the Content-Length header accordingly. This wasn't updated with introduction of the request library. That resulted in the Content-Length being sent with incorrect number, causing the server under test to hang, waiting for more bytes.

My changes makes the code dealing with apiaryio/api-blueprint#401 a lot simpler and more functional (no side effects). It removes any modifications to Content-Length and instead it changes the order in which the body gets modified and the header gets calculated.

📝 Related issues and Pull Request

• Use the request library instead of custom implementation #734
• Multipart request bodies need to use \r\n instead of only \n api-blueprint#401
• Multipart form requests need to use \r\n instead of only \n  #74
• Multipart form requests need to use \r\n instead of only \n #74 #118

Fixes #786.

✅ What didn't I forget?

• To write docs - N/A
• To write tests
• To put Conventional Changelog prefixes in front of all my commits and run npm run lint

[michalholasek]

You can join these two returns into one - return false if not contentType else contentType.indexOf('multipart') > -1.

[honzajavorek]

Addressed in 3055aba

[michalholasek]

Honestly, I think that change made in 3055aba is worse than the original two returns. I think that solution from above reads a little bit better, yours has a little bit of Yoda tongue in it 🙂

[honzajavorek]

Ha, I though that was typo of yours, I did not know that's valid CoffeeScript 😄

return false if not contentType else contentType.indexOf('multipart') > -1

won't compile. I'll change it to normal if/else 😏

[honzajavorek]

Addressed in a4da5dd

[michalholasek]

[michalholasek]

Just one nitpick, otherwise I think it is good to go 👍

[honzajavorek]

The drop in coverage is caused by "statistics". There's less lines in the transaction runner now, which means the uncovered part takes larger percentage of it. The drop is thus caused by Dredd consisting of less lines in overall, not by adding uncovered code:

[honzajavorek]

Got verbal approval from @michalholasek of the coverage drop explanation, merging.