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: proxy events into instance var and add test script #1661
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2430,3 +2430,19 @@ test('should emit error with multiple next calls with strictNext', function(t) { | |
}); | ||
}); | ||
}); | ||
|
||
test('should have proxy event handlers as instance', function(t) { | ||
var server = restify.createServer({ | ||
handleUpgrades: false | ||
}); | ||
t.equal(server.proxyEvents.length, 7); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I expect handleUpgrades: true to be 7, and 6 for false (may be related to the && indexOf check above) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per the doc options.handleUpgrades Boolean Hook the upgrade event from the node HTTP server, pushing Connection: Upgrade requests through the regular request handling chain. (optional, default false) (http://restify.com/docs/server-api/). We are updating proxyEvents array when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I can't read. Must have been a long day. :D |
||
|
||
server = restify.createServer({ | ||
handleUpgrades: true | ||
}); | ||
|
||
t.equal(server.proxyEvents.length, 6); | ||
server.close(function() { | ||
t.end(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that proxyEvents is specific to the instance of the server being created, I don't think we even need the indexOf check right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DonutEspresso: right, I can remove it.