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

Remove debug leftovers from a test. #314

Merged
merged 5 commits into from
Jul 6, 2022
Merged

Remove debug leftovers from a test. #314

merged 5 commits into from
Jul 6, 2022

Conversation

Gnuxie
Copy link
Contributor

@Gnuxie Gnuxie commented Jul 1, 2022

This is really terrible and has meant whenever anyone has run yarn test:integration they have only been running this test.
💀💀💀

Edit: So it turned out a couple of tests (ban list test & the throttling test) were silently broken because of the issue this PR originally fixes, so these tests have also been fixed as a part of this PR

@Gnuxie Gnuxie requested a review from jesopo July 1, 2022 10:09
Copy link
Contributor

@jesopo jesopo left a comment

Choose a reason for hiding this comment

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

oof ow oopsie 💀

This is really terrible and has meant whenever anyone has run `yarn test:integration` they have only been running this test.
💀💀💀
https://www.youtube.com/watch?v=jmX-tzSOFE0
Gnuxie added 4 commits July 6, 2022 11:53
Seriously, I don't think there is much to gain by making people guess
a reasnoble time for a test to complete in all the time, especially
with how much Synapse changes in response time and all of the machines
involved in running these tests.
no i am not going to track down why yet.
matrix-org/synapse#13018

Rate limiting in Synapse used to reset the burst count and remove
the backoff when you were spamming continuously, now it doesn't.
Ideally we'd rewrite the rate limiting logic to back off for longer
than suggested so we could get burst again, but for now
lets just unblock CI by reducing the number of events we send in these
tests.
@Gnuxie Gnuxie requested a review from jesopo July 6, 2022 11:39
Copy link
Contributor

@Yoric Yoric left a comment

Choose a reason for hiding this comment

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

Let's hope this helps!

@Gnuxie Gnuxie merged commit b850e45 into main Jul 6, 2022
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.

3 participants