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.

Sign up for GitHub

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

Jump to bottom

Update all tests to use the Promise API #1213

Closed

owenpearson opened this issue Apr 26, 2023 · 2 comments

Assignees

Labels

testing

Member

owenpearson commented Apr 26, 2023

[Self explanatory]

owenpearson added the testing label

owenpearson mentioned this issue

Make Promise API default #1199

Closed

sync-by-unito bot commented Apr 26, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3554

sync-by-unito bot assigned owenpearson

owenpearson mentioned this issue

fix(RestPresence): fix broken promisification #1224

Merged

ably deleted a comment from sync-by-unito bot

sync-by-unito bot unassigned owenpearson

lawrence-forooghian self-assigned this

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

f4cd108

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

f2fe78c

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

d64e31a

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

8a0092e

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

656414a

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

5fbeace

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

9ef066d

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

668bf3e

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

7b9d844

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

4c78188

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

df12b8a

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what is Ably.Realtime vs Ably.Realtime.Promise and have I
converted properly? I think these might only be direclty used in the
init and api tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO remove the SkippingError

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

ddd0e3b

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO introduce a promise-based variant of generateRandomKey

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

e149db8

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Try a different way of testing using async API

517c496

I won't make any changes to `once` or `whenState` because we want to
keep them

TODO remove specific Promise tests

TODO what about uncaught promises

TODO a better name for ptcb

TODO introduce a promise-based variant of generateRandomKey

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Update Realtime tests to use SDK’s Promise-based API

676df20

I’ve chosen to do this with the lightest touch possible — that is,
maintaining the tests’ callback-based approach and simply bridging the
SDK’s Promise-based API back to callbacks. I did this for the sake of
reviewability and not accidentally changing the behaviour of the tests
in some subtle way that I’d then have to put time into understanding.

It would be good to, at some point, update the structure of the tests to
use `async` / `await`, to improve readability and to make them reflect
how the users actually interact with the Promise-based API in the real
world. I’ve split out into the separate task #1348.

Note that I haven’t made any changes to the calls to EventEmitter’s
`once` or `whenState` methods — we’re going to keep the callback-based
versions of those methods, since there is no Promise equivalent of being
able to turn them off using `off`.

TODO remove specific Promise tests

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Update Realtime tests to use SDK’s Promise-based API

d7ba3d5

I’ve chosen to do this with the lightest touch possible — that is,
maintaining the tests’ callback-based approach and simply bridging the
SDK’s Promise-based API back to callbacks. I did this for the sake of
reviewability and not accidentally changing the behaviour of the tests
in some subtle way that I’d then have to put time into understanding.

It would be good to, at some point, update the structure of the tests to
use `async` / `await`, to improve readability and to make them reflect
how the users actually interact with the Promise-based API in the real
world. I’ve split out into the separate task #1348.

Note that I haven’t made any changes to the calls to EventEmitter’s
`once` or `whenState` methods — we’re going to keep the callback-based
versions of those methods, since there is no Promise equivalent of being
able to turn them off using `off`.

Resolves #1213.

lawrence-forooghian mentioned this issue

Update the tests to use the Promise-based API #1349

Merged

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

f744893

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Update Realtime tests to use SDK’s Promise-based API

a5d2785

I’ve chosen to do this with the lightest touch possible — that is,
maintaining the tests’ callback-based approach and simply bridging the
SDK’s Promise-based API back to callbacks. I did this for the sake of
reviewability and not accidentally changing the behaviour of the tests
in some subtle way that I’d then have to put time into understanding.

It would be good to, at some point, update the structure of the tests to
use `async` / `await`, to improve readability and to make them reflect
how the users actually interact with the Promise-based API in the real
world. I’ve split out into the separate task #1348.

Note that I haven’t made any changes to the calls to EventEmitter’s
`once` or `whenState` methods — we’re going to keep the callback-based
versions of those methods, since there is no Promise equivalent of being
able to turn them off using `off`.

Resolves #1213.

lawrence-forooghian added a commit that referenced this issue


          Remove parts of test relating to setOptions calling its callback imme…

a1c3592

…diately

As part of #1213, I’ll be updating this test to use the Promise-based
API, to which the concept of "calls the callback before returning" can’t
be detected, since the JavaScript engine always calls Promise callbacks
asynchronously.

lawrence-forooghian added a commit that referenced this issue


          Update Realtime tests to use SDK’s Promise-based API

43a2d1d

I’ve chosen to do this with the lightest touch possible — that is,
maintaining the tests’ callback-based approach and simply bridging the
SDK’s Promise-based API back to callbacks. I did this for the sake of
reviewability and not accidentally changing the behaviour of the tests
in some subtle way that I’d then have to put time into understanding.

It would be good to, at some point, update the structure of the tests to
use `async` / `await`, to improve readability and to make them reflect
how the users actually interact with the Promise-based API in the real
world. I’ve split out into the separate task #1348.

Note that I haven’t made any changes to the calls to EventEmitter’s
`once` or `whenState` methods — we’re going to keep the callback-based
versions of those methods, since there is no Promise equivalent of being
able to turn them off using `off`.

Resolves #1213.

Collaborator

lawrence-forooghian commented Jul 3, 2023

Resolved by #1349.

lawrence-forooghian closed this as completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment