-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add flush() method #115
Add flush() method #115
Conversation
Hey @loucadufault, could you give this PR a quick review to ensure that it fits with your use case (pay extra attention to the unit tests) ? |
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.
looks to cover the use case proposed in the issue well, thanks!
.add(async () => { | ||
await this.device.write(data) | ||
}) |
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.
small nit: you could directly return the promise here
.add(async () => { | |
await this.device.write(data) | |
}) | |
.add(() => this.device.write(data)) |
expect(mockWriteEnd).toBeCalledTimes(0) // Should not have been called yet | ||
|
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.
you could use jest fake timers instead, and advance the timers here by 10ms to make the test a bit more guaranteed
Add a
flush()
method, as discussed in #106