Issue 75: cant test expected error responses because superagent throws #93
Conversation
farmisen
force-pushed
the
Issue-75_Cant_test_expected_error_responses_because_superagent_throws
branch
from
b8bb783
to
21c27fd
Compare
|
@farmisen this looks good from a coding perspective. I'm not familiar enough with this plugin to offer a strong opinion on whether or not we actually want to move forward with this feature; we'll need to wait for @keithamus. If he does give the green light, then we'll want the option documented in the comments, and this PR rebased into a single commit, but probably best to hold off on any of that until he pops in. Thanks! :D |
| @@ -249,7 +250,7 @@ Test.prototype.then = function (onResolve, onReject) { | |||
| var self = this; | |||
| this._promise = new Test.Promise(function (resolve, reject) { | |||
| self.end(function (err, res) { | |||
| if (err) { | |||
| if (err && self.options.badStatusCausesError) { | |||
There was a problem hiding this comment.
Not sure I like this code right here - because bad statuses aren't the only cause of failure (for example timeouts, dns failures, connection closures all produce errors) - some of those errors are genuinely errors and should always fail. This should check the res.status as well.
|
@farmisen You should check out my comment in #75 here: #75 (comment). This might be a better direction to go in, as it would also support |
#75
Gave a shot at implementing @keithamus suggestion.