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 option timeout to WebSocketTestSession::receive #2329

Closed
wants to merge 1 commit into from

Conversation

mbergen
Copy link

@mbergen mbergen commented Nov 7, 2023

Summary

Allows to pass a timeout to various receive methods of WebSocketTestSession which is passed into Queue::get.
This allows to write tests that raise an exception (which can be expected as well) instead of blocking forever in case the tested app does not send a message on the websocket.

It is a potential fix for #2195, but allows finer control about which call to receive should have a timeout, and which should block instead of setting a generic timeout option to the Session.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@Kludex Kludex added the hold Don't merge it label Nov 7, 2023
@Kludex
Copy link
Sponsor Member

Kludex commented Nov 11, 2023

Thanks for the PR, but this was already rejected on: #1140.

@Kludex Kludex closed this Nov 11, 2023
@mbergen
Copy link
Author

mbergen commented Nov 12, 2023

Thanks for the PR, but this was already rejected on: #1140.

Sorry, that PR didn't show up in my search (only the referenced issue/discussion).
Nontheless, the contributors of that PR as well as I don't think the reasoning for closing that PR doesn't really address the issue we experience, having an option like timeout would improve test development a lot.
Could you elaborate why you are against it? Do you want to support every parameter of the underlying receive before considering it?

@nextmat
Copy link

nextmat commented Dec 5, 2023

I agree. This would be very helpful, I don't really understand the resistance to allowing it as an option?

When refactoring a failing test with details about the failure is generally preferable to the test suite hanging indefinitely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Don't merge it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants