Skip to content
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 more tests for room reactions #88

Open
umair-ably opened this issue Oct 18, 2024 · 0 comments
Open

Add more tests for room reactions #88

umair-ably opened this issue Oct 18, 2024 · 0 comments

Comments

@umair-ably
Copy link
Collaborator

umair-ably commented Oct 18, 2024

Initial PR with the work for implementing room level reactions is not exhaustive with regards to unit tests for the spec (however, all of the spec has been implemented).

Adding this issue as a reminder to add more tests

┆Issue is synchronized with this Jira Task by Unito

lawrence-forooghian added a commit that referenced this issue Dec 16, 2024
Similar issue to the one which was fixed in 7fcab5c.

Haven’t added tests for the handling of the incoming message, since
there were existing tests to update; will leave this for whoever writes
these tests in #88.

The approach that I’ve taken here of using a DTO is consistent with the
approach that we use for presence data. I should have done this in
7fcab5c too; will do it separately.

TODO add tests for DTO

Resolves #198.
lawrence-forooghian added a commit that referenced this issue Dec 16, 2024
Similar issue to the one which was fixed in 7fcab5c.

Haven’t added tests for the handling of the incoming message, since
there were no existing tests to update and I didn’t feel like writing
them; will leave this for whoever writes these tests in #88.

The approach that I’ve taken here of using a DTO is consistent with the
approach that we use for presence data. I should have done this in
7fcab5c too; will do it separately.

Resolves #198.
lawrence-forooghian added a commit that referenced this issue Dec 16, 2024
Similar issue to the one which was fixed in 7fcab5c.

Haven’t added tests for the handling of the incoming message, since
there were no existing tests to update and I didn’t feel like writing
them; will leave this for whoever writes these tests in #88.

The approach that I’ve taken here of using a DTO is consistent with the
approach that we use for presence data. I should have done this in
7fcab5c too; will do it separately.

TODO where has the behaviour of absent / present keys come from?

Resolves #198.
lawrence-forooghian added a commit that referenced this issue Dec 17, 2024
Similar issue to the one which was fixed in 7fcab5c.

Haven’t added tests for the handling of the incoming message, since
there were no existing tests to update and I didn’t feel like writing
them; will leave this for whoever writes these tests in #88.

The approach that I’ve taken here of using a DTO is consistent with the
approach that we use for presence data. I should have done this in
7fcab5c too; will do it separately.

The spec doesn’t describe how to behave if the user doesn’t pass headers
or metadata; I’ve taken the behaviour of passing an empty object from
the JS Chat SDK at 6d1b04a. Have created spec issue [1] for specifying
this.

Resolves #198.

[1] ably/specification#256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant