-
Notifications
You must be signed in to change notification settings - Fork 7
Tidy DataQueryEndpoints tests #239
base: master
Are you sure you want to change the base?
Conversation
… "TypeError: Cannot read property 'on' of undefined"
}) | ||
stream.on('error', (err) => { | ||
try { | ||
let delimiter = '' |
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.
If stream.on
is failing, it seems like a fundamental problem with the code that should cause a crash instead of being wrapped in a 500 error code and sent back to client? I.e. it is an unexpected error that should not really ever occur unless there is a problem with our code.
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.
that's true
it('responds 500 and error message if networkNode signals error', (done) => { | ||
networkNode.requestResendRange = () => intoStream.object(Promise.reject(new Error('error'))) | ||
it('responds 500 and error message if networkNode signals error', async () => { | ||
networkNode.requestResendRange = jest.fn(() => intoStream.object(Promise.reject(new Error('expected error')))) |
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.
What's the reasoning behind wrapping the function with jest.fn
if it not being asserted (e.g. num of calls)?
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.
just consistency really, jest.fn
also signals (to me at least) that this is a mock function.
Fixes distracting and unnecessary
TypeError
issues reported in tests.Also converts the
supertest
tests to useasync
/await
instead of Jest'sdone
callback.Shouldn't conflict with #238