eugef / node-mocks-http

Mock 'http' objects for testing Express routing functions
Other
755 stars 134 forks source link

mockResponse.cookie does not follow the builder pattern #129

Closed danallen88 closed 7 years ago

danallen88 commented 7 years ago

If my code under test has something like res.status(200).cookie('name', 'value').send('stuff'), then the send method will never get called (and consequently the end event will not fire) because the mockResponse.cookie method does not support chaining.

mockResponse.cookie = function(name, value, opt) {

        mockResponse.cookies[name] = {
            value: value,
            options: opt
        };

    };

I suspect this could be fixed by ending the method with return this;

howardabrams commented 7 years ago

Interesting idea. We would probably want all other functions to do that too.

Can't think of what would be the downside to this. This shouldn't break backward compatibility, would it?

danallen88 commented 7 years ago

I wouldn't think it would break backward compatibility. We would just want to be careful to only implement it for express methods that support chaining. If I get some free time I'd be happy to submit a PR.

edit: After some further analysis of the methods supported in this project, I think only cookie() and clearCookie() are eligible for non-breaking changes. vary() looks like it can be chained per the express docs, but it internally calls setHeader() which just returns the value being set. Since setHeader() and the other methods from node's http module don't have any documentation on whether or not they support chaining, I'm hesitant to try and make any changes there. Otherwise everything else appears to at some point call end() or emit the 'end' event so I don't think those are candidates for chaining.