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 lazy-loading /sync tests for faster room joins #442

Merged
merged 7 commits into from
Sep 2, 2022

Conversation

squahtx
Copy link
Contributor

@squahtx squahtx commented Aug 10, 2022

Test that lazy-loading /syncs do not block when receiving events from
remote senders. Also test that lazy-loading /syncs return the
appropriate memberships for event senders.


Contains some changes from #440 and #441.

Sean Quah added 3 commits August 10, 2022 17:26
Test that lazy-loading `/sync`s do not block when receiving events from
remote senders. Also test that lazy-loading `/sync`s return the
appropriate memberships for event senders.
@squahtx squahtx requested review from a team as code owners August 10, 2022 16:44
@squahtx
Copy link
Contributor Author

squahtx commented Aug 10, 2022

The three new tests have a lot of duplicate code. Suggestions for tidying them up are welcome.

if err == nil {
t.Errorf("gappy /sync returned the first event unexpectedly")
}

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should also assert that the limited flag is set? I don't think it matters much for this test, but in general I think it might avoid potential pitfalls to explicitly assert that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a check for the limited flag.

@squahtx
Copy link
Contributor Author

squahtx commented Aug 17, 2022

Do not merge this yet - the objections from #440 need to be resolved.

@squahtx squahtx requested review from erikjohnston and kegsay and removed request for kegsay and erikjohnston August 17, 2022 14:43
@squahtx
Copy link
Contributor Author

squahtx commented Aug 17, 2022

I've removed the objectionable parts of #440.

@kegsay Can you check the changes to internal/client/client.go?

squahtx added a commit that referenced this pull request Sep 2, 2022
@squahtx squahtx merged commit 163e00a into main Sep 2, 2022
@squahtx squahtx deleted the squah/faster_room_joins_unblock_lazy_loading_sync_2 branch September 2, 2022 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants