Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

sliding sync: Add test to verify we send unsubscribe_rooms for every room switch #9784

Merged
merged 12 commits into from
Dec 22, 2022

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Dec 16, 2022

This adds a test to verify that we send unsubscribe_rooms for every room switch.
CI is going to fail for this test, as of now, as it needs matrix-org/matrix-js-sdk#2991

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

I didn't know you could do this in Cypress! Might need a check from @t3chguy with his Cypress-fu, but this LGTM.

@kegsay kegsay added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Dec 16, 2022
@t3chguy
Copy link
Member

t3chguy commented Dec 16, 2022

Looks sane to me, but in future if you name your branches the same then the CI will run them against each other, presumably the fail right now is due to the js-sdk changeset not being considered. Also might be worth probably checking the content of the room_subscriptions - to ensure the wrong room isn't being unsubbed?

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

looks like the new test is failing

@richvdh
Copy link
Member

richvdh commented Dec 19, 2022

oh I see, that's expected. Still, we can't merge this until it is unblocked by matrix-org/matrix-js-sdk#2991.

@S7evinK
Copy link
Contributor Author

S7evinK commented Dec 20, 2022

oh I see, that's expected. Still, we can't merge this until it is unblocked by matrix-org/matrix-js-sdk#2991.

Thanks for approving this one, now lets wait for CI. :)

@richvdh richvdh enabled auto-merge (squash) December 21, 2022 10:56
@robintown robintown removed their request for review December 21, 2022 15:06
@richvdh richvdh merged commit d073b3b into matrix-org:develop Dec 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants