ably-js
ably-js copied to clipboard
Add tests which use the promisified version of all public async methods
Someone recently opened #943 which is a bug with the promisified version of RestPresence.get. This would have been caught by our tests but we don't currently have any tests which check that promisification is working here.
I'm guessing that this issue (and this fix) also would've been caught by these.
@JCB-K Realistically, probably not. The only way we would have caught it through testing is if we started adding tests to ensure that optional arguments work the same whether they're omitted vs when undefined is passed in explicitly (the promisification should work fine at the moment if you just don't pass in anything for the second arg). I won't go in to too much detail here but we're planning on changing the way we handle callbacks/promises in the near future so issues like #983 won't be possible anymore. FWIW we have already added promise tests for every async API method in the library except for the methods on RealtimeChannel.
To add some context to GitHub label events above, I just renamed the old tests label to failing-test as I'm currently conforming labels. That was a bad decision - I should have created a new label, not renamed the existing one. Sorry for the noise but I just wanted to explain as I've modified the previous event's context by doing this. 🤦