fix(Client): Do not throw error on bad message#43
Conversation
alex-ketch
left a comment
alex-ketch
left a comment
There was a problem hiding this comment.
I'm all for fewer exceptions :)
| import { delay } from '../test/delay' | ||
|
|
||
| jest.setTimeout(30 * 1000) | ||
| jest.setTimeout(2 * 60 * 1000) |
There was a problem hiding this comment.
I think this is fine for now (especially given the reasoning commented on line 51), but I am a little hesitant about our need to use delay and adjust setTimeout. The tests in Encoda have become quite flaky in reliability and I'd be cautious that it doesn't happen here too.
It's something that I've been meaning to look into after current work iteration.
There was a problem hiding this comment.
Fair point, I'll see if I can find a way to avoid those as part of this PR, no point _delay_ing that 😁
There was a problem hiding this comment.
I've removed the need to use delay and adjust setTimeout by using a promise on the next log event. @alex-ketch could you review those changes - I'll refactor the other tests accordingly. c4c6031
Codecov Report
@@ Coverage Diff @@
## master #43 +/- ##
==========================================
+ Coverage 86.67% 87.17% +0.49%
==========================================
Files 26 27 +1
Lines 938 951 +13
Branches 178 178
==========================================
+ Hits 813 829 +16
+ Misses 121 118 -3
Partials 4 4
Continue to review full report at Codecov.
|
alex-ketch
left a comment
alex-ketch
left a comment
There was a problem hiding this comment.
Looks good, thanks for making the changes @nokome
|
🎉 This PR is included in version 0.14.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
5 participants
Closes #41. A remaining, related, issue is that a bad message will mean that a call to a client message never resolves. Approaches to that (for another PR) could include:
rejectall outstanding client requests