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

[ECO-4858] TO3g compliance #1941

Merged
merged 7 commits into from
Jul 3, 2024
Merged

[ECO-4858] TO3g compliance #1941

merged 7 commits into from
Jul 3, 2024

Conversation

maratal
Copy link
Collaborator

@maratal maratal commented Jun 29, 2024

Closes #1936, #1938 and #1940

These three bugs were cancelling each other, so it's impossible to address them in different PRs since to fix all the tests failures you need to fix other two bugs. Please see commit messages and review per commit basis.

See also ably/specification#194

Some additional info for commits messages.

d60269a "Proceeding with detach on CONNECTED":

What happens here is that in addition to re-attach we need re-detach as well to continue the process after connection becomes CONNECTED. But in oppose to attach there were no internal method for doing so, that's why I've splitted _detach into internalDetach: which doesn't do state check before proceeding with detaching. Also because detach message shouldn't be queued I've removed shouldQueueEvents check from the detachAfterChecks.

305d8db "Sending queued messages should be after reattach":

If you send message before sending ATTACH, realtime won't respond and tests will timeout. You can check it with test__065__Channel__publish__Message_connectionId_should_match_the_current_Connection_id_for_all_published_messages test.

16b3806 "Only queue publish and presence messages":

The check else if (msg.ackRequired) you should read as "if message can be queued", since ACK required with the same condition as queuing - message is either MESSAGE or PRESENCE. Also I've added log line if message sending was ignored.

@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 29, 2024 23:16 Inactive
@maratal maratal changed the base branch from main to fix/1938-invalid-RTL4i June 29, 2024 23:16
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 29, 2024 23:19 Inactive
@maratal maratal force-pushed the fix/1940-TO3g-violation branch from 21744f1 to 16b3806 Compare June 30, 2024 02:41
@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 30, 2024 02:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 30, 2024 02:46 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 30, 2024 11:52 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 30, 2024 11:55 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 30, 2024 13:09 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 30, 2024 13:13 Inactive
@maratal maratal force-pushed the fix/1940-TO3g-violation branch from 5634ed8 to 586c255 Compare June 30, 2024 21:24
@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 30, 2024 21:25 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 30, 2024 21:30 Inactive
@maratal maratal changed the base branch from fix/1938-invalid-RTL4i to main June 30, 2024 23:18
@maratal maratal closed this Jun 30, 2024
@maratal
Copy link
Collaborator Author

maratal commented Jun 30, 2024

Closed by mistake

@maratal maratal reopened this Jun 30, 2024
@github-actions github-actions bot temporarily deployed to staging/pull/1941/features June 30, 2024 23:22 Inactive
@maratal maratal requested review from sacOO7 and umair-ably June 30, 2024 23:23
@maratal maratal marked this pull request as ready for review June 30, 2024 23:23
@github-actions github-actions bot temporarily deployed to staging/pull/1941/jazzydoc June 30, 2024 23:26 Inactive
@maratal maratal changed the title [ECO-4858] TO3g violation fix [ECO-4858] TO3g compliance Jun 30, 2024
This was linked to issues Jul 1, 2024
Copy link
Contributor

@umair-ably umair-ably left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maratal maratal merged commit d1458ad into main Jul 3, 2024
7 checks passed
@maratal maratal deleted the fix/1940-TO3g-violation branch July 3, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Violation of TO3g Invalid RTL4i usage shouldQueueEvents should not check options.queueMessages
2 participants