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

feat: add a new test that produces stored notifs #598

Merged
merged 9 commits into from
Feb 6, 2024

Conversation

pjenvey
Copy link
Member

@pjenvey pjenvey commented Feb 2, 2024

(a quick, initial implementation)

this uses the more basic websocket.WebSocket as WebSocketApp.run_forever
doesn't seem to handle the workflow needed (where the websocket is kept
disconnected for a large portion of the test)

Issue: SYNC-3916

(a quick, initial implementation)

this uses the more basic websocket.WebSocket as WebSocketApp.run_forever
doesn't seem to handle the workflow needed (where the websocket is kept
disconnected for a large portion of the test)

Issue: SYNC-3916
@pjenvey
Copy link
Member Author

pjenvey commented Feb 2, 2024

What's changed between this and locustfile.py:

4694798...feat/stored-notifs-SYNC-3916

@pjenvey pjenvey force-pushed the feat/stored-notifs-SYNC-3916 branch from 57616b7 to e45901e Compare February 3, 2024 01:03
jrconlin
jrconlin previously approved these changes Feb 5, 2024
tests/load/locustfiles/stored.py Outdated Show resolved Hide resolved

Message: TypeAlias = HelloMessage | NotificationMessage | RegisterMessage | UnregisterMessage
Record: TypeAlias = HelloRecord | NotificationRecord | RegisterRecord

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to include the enableTrace and setdefaulttimeout that were defined in locustfile.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope because it's global it should only be done in one place, especially since we invoke locust on both locustfile/stored.py. Those lines should probably move out of locustfile.py into a shared module eventually.

Trinaa
Trinaa previously approved these changes Feb 5, 2024
Copy link
Contributor

@Trinaa Trinaa left a comment

Choose a reason for hiding this comment

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

Are you wanting to update the load.py Shape to include a percentage of this type of user? Or maybe this is a follow-up?

Also I'm curious if the on_ws lifecycle methods are working?

@jrconlin
Copy link
Member

jrconlin commented Feb 5, 2024

Aaand looks like black became more pedantic about the comment strings.

@pjenvey pjenvey dismissed stale reviews from Trinaa and jrconlin via cdc880b February 5, 2024 23:07
@pjenvey
Copy link
Member Author

pjenvey commented Feb 5, 2024

Are you wanting to update the load.py Shape to include a percentage of this type of user? Or maybe this is a follow-up?

Let's make this a followup, we'll probably initially start with solely using StoredAutopushUser by itself.

Also I'm curious if the on_ws lifecycle methods are working?

Only on_ws_message is used here -- the others should be removed from this module later when we clean this up further.

@pjenvey pjenvey requested review from Trinaa and jrconlin February 6, 2024 00:22
@pjenvey pjenvey merged commit a6bd3cd into master Feb 6, 2024
1 check passed
@pjenvey pjenvey deleted the feat/stored-notifs-SYNC-3916 branch February 6, 2024 18:43
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