Fix memory leak in waitDevice method#75
Conversation
|
Hello @chrvadala, Please let us know when you plan to have a look at this PR by @Raffone17. Thank you! |
|
Thanks for this PR guys, can you integrate it with a test? |
|
Hello @chrvadala, |
chrvadala
left a comment
There was a problem hiding this comment.
Thanks for adding a test. I added few comments inline.
test/Adapter.spec.js
Outdated
| const discoveryInterval = 100 | ||
|
|
||
| const spyClearInterval = jest.spyOn(global, 'clearInterval') | ||
| const spyClearTimeout = jest.spyOn(global, 'clearInterval') |
There was a problem hiding this comment.
I think that there's a mistake on this line.
test/Adapter.spec.js
Outdated
| }) | ||
|
|
||
| test('clear intervals and timeouts after fail', async () => { | ||
| jest.useFakeTimers('legacy') |
There was a problem hiding this comment.
Is there a specific reason for using the legacy implementation?
test/Adapter.spec.js
Outdated
|
|
||
| const waitDevicePromise = adapter.waitDevice('44:44:44:44:44:44', timeout, discoveryInterval) | ||
|
|
||
| jest.advanceTimersByTime(500) |
There was a problem hiding this comment.
you can specify here the timeout variable that you declared
| jest.advanceTimersByTime(500) | |
| jest.advanceTimersByTime(timeout) |
|
@chrvadala I made the requested changes. |
|
Released with v1.12.0 |
In the waitDevice method, if the device is not found the 'operation timed out' error is thrown but the interval is not cleared. I added a try and finally statements for avoid this problem.