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.

Sign up for GitHub

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

Jump to bottom

cannon: Create queue before responding to the websocket #4404

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

cannon: Create queue before responding to the websocket #4404

wants to merge 6 commits into from
+92 −86

Conversation

akshaymankar
Copy link
Member

@akshaymankar akshaymankar commented Jan 8, 2025 •
edited
Loading

This way tests can run more reliably, i.e. when they see that the websocket is functional, they queue already exists in RabbitMQ.

Also in this PR:

  • Remove channel re-establishment code in Cannon as it is not used anyway and it was easier to ensure that the queue is created before we respond to the websocket.
  • Add Deflake type to run a test multiple times
  • Ensure that integration tests run the websocket assertions only after the websocket is functional.
  • Small changes to websockets handling in integration tests so errors look nicer

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d No changelog.
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 8, 2025
@akshaymankar akshaymankar force-pushed the deflake-testConsumeTempEventsWithoutOwnClient branch from 30dcbd8 to 8ce531b Compare January 9, 2025 13:49
@akshaymankar akshaymankar changed the title integration: Deflake test consume temp events without own client cannon: Create queue before responding to the websocket Jan 9, 2025
@akshaymankar akshaymankar marked this pull request as ready for review January 9, 2025 14:52
@akshaymankar
Copy link
Member Author

Ran that test 100 times twice:
https://concourse.ops.zinfra.io/teams/main/pipelines/wire-server-pr/jobs/integration-tests-kube-ci/builds/4066
https://concourse.ops.zinfra.io/teams/main/pipelines/wire-server-pr/jobs/integration-tests-kube-ci/builds/4066.1

It failed 7 out 100 times before the fix: https://concourse.ops.zinfra.io/teams/main/pipelines/wire-server-pr/jobs/integration-tests-kube-ci/builds/4059.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@battermann battermann battermann approved these changes

Assignees
No one assigned
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@akshaymankar @battermann @zebot