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

Use ExUnit's assert_receive_timeout as the default receive timeout #2655

Merged

Conversation

s3cur3
Copy link
Contributor

@s3cur3 s3cur3 commented Jun 1, 2023

Per a brief discussion on Mastodon, this replaces the hardcoded default timeout of 100 milliseconds on message-receiving test functions (assert_patch, assert_redirect, assert_push_event, and assert_reply) with a lookup of ExUnit's assert_receive_timeout.

This makes the behavior of Phoenix.LiveViewTest more consistent with Phoenix.ChannelTest, where assert_receive, assert_push, assert_reply, and assert_broadcast all delegate to the ExUnit config for their default timeout.

…the behavior of Phoenix.ChannelTest

Per a [brief discussion on Mastodon](https://merveilles.town/@peregrine/110469721567235316), this replaces the hardcoded default timeout of 100 milliseconds on message-receiving test functions (`assert_patch`, `assert_redirect`, `assert_push_event`, and `assert_reply`) with a lookup of ExUnit's `assert_receive_timeout`.

This makes the behavior of `Phoenix.LiveViewTest` more consistent with `Phoenix.ChannelTest`, where `assert_receive`, `assert_push`, `assert_reply`, and `assert_broadcast` all delegate to the ExUnit config for their default timeout.
@s3cur3 s3cur3 marked this pull request as ready for review June 1, 2023 16:48
lib/phoenix_live_view/test/live_view_test.ex Outdated Show resolved Hide resolved
lib/phoenix_live_view/test/live_view_test.ex Outdated Show resolved Hide resolved
lib/phoenix_live_view/test/live_view_test.ex Outdated Show resolved Hide resolved
lib/phoenix_live_view/test/live_view_test.ex Show resolved Hide resolved
@jeregrine jeregrine self-assigned this Jun 1, 2023
Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

LGTM! Go ahead and merge it @jeregrine when you are happy too :)

@jeregrine
Copy link
Member

Just some docs fixes

@jeregrine
Copy link
Member

<3 thank you so much @s3cur3

@s3cur3
Copy link
Contributor Author

s3cur3 commented Jun 1, 2023

Thank you! 😄 💜

@jeregrine jeregrine merged commit 1812c0e into phoenixframework:main Jun 1, 2023
@Nezteb
Copy link
Contributor

Nezteb commented Jun 1, 2023

💚💙💜💛❤️

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.

4 participants