search

eugef / node-mocks-http

Mock 'http' objects for testing Express routing functions
Other
753 stars 133 forks source link

Feature/add accepts language #188

Closed AndyOGo closed 5 years ago

AndyOGo commented 5 years ago

fixes #187 fixes #128

eugef commented 5 years ago

@AndyOGo thanks for the PR, please review and fix few issues.

AndyOGo commented 5 years ago

@eugef Thanks for your quick review. I fixed them, please see my changes.

PS: some how I can't request a new review on Github

AndyOGo commented 5 years ago

@eugef @howardabrams I made suggested changes, may I ask you to review again please? I'm looking forward to your feedback. Thank you.

AndyOGo commented 5 years ago

@eugef @howardabrams hmh, I'm a bit confused. Just found existing implementations of this PR here: https://github.com/howardabrams/node-mocks-http/blob/master/lib/express/mock-request.js#L34-L53

Express is exported here, gues it was already working but was usage issue 🤔 https://github.com/howardabrams/node-mocks-http/blob/master/lib/http-mock.js#L34

eugef commented 5 years ago

Personally I never use express-specific mocks, so your PR is still relevant

AndyOGo commented 5 years ago

Me neither, they aren't documented too. Glad my PR is still relevant.

But just recently I worked on .isXhr #189 greped through this repo for .isXhr and found all of it and more already implemented and exported as httpMocks.express. I couldn't find unit tests for it though 🤔 Definitely a lot of work has already been done for express