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

Add unit tests for UDPServer #97311

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add unit tests for UDPServer #97311

wants to merge 1 commit into from
+285 −0

Conversation

pafuent
Copy link
Contributor

@pafuent pafuent commented Sep 22, 2024

This PR aims to help "fix" #43440

@pafuent pafuent requested a review from a team as a code owner September 22, 2024 03:11
@pafuent pafuent force-pushed the adding_udp_server_unit_tests branch 3 times, most recently from 7196c4b to bd02dc9 Compare September 22, 2024 03:38
@AThousandShips AThousandShips added this to the 4.x milestone Sep 22, 2024
@pafuent pafuent mentioned this pull request Sep 23, 2024
Open
99 tasks
AThousandShips
AThousandShips reviewed Sep 24, 2024
View reviewed changes
tests/core/io/test_udp_server.h Outdated Show resolved Hide resolved
tests/core/io/test_udp_server.h Outdated Show resolved Hide resolved
tests/core/io/test_udp_server.h Outdated Show resolved Hide resolved
@pafuent pafuent force-pushed the adding_udp_server_unit_tests branch 2 times, most recently from 4eb8243 to e00f29e Compare September 25, 2024 12:35
Calinou
Calinou requested changes Oct 1, 2024
View reviewed changes
tests/core/io/test_udp_server.h Outdated Show resolved Hide resolved
tests/core/io/test_udp_server.h Outdated Show resolved Hide resolved
@pafuent pafuent force-pushed the adding_udp_server_unit_tests branch 2 times, most recently from aa5b83d to 969fc6c Compare October 3, 2024 03:12
@pafuent
Copy link
Contributor Author

pafuent commented Oct 10, 2024

@Calinou I addressed all your comments, Could you please review again this PR?

@pafuent pafuent force-pushed the adding_udp_server_unit_tests branch from 969fc6c to da9ee93 Compare October 17, 2024 13:31
@pafuent
Copy link
Contributor Author

pafuent commented Oct 17, 2024

Friendly remainder

@Faless
Copy link
Collaborator

Faless commented Oct 18, 2024

Friendly remainder

As mentioned in #97262 (comment) :

The problem here is that you keep blocking for a long time and only poll once, while you should only block for brief periods of time and regularly poll (i.e. multiple times), or you risk of having the same error on other OS configurations.

@pafuent pafuent force-pushed the adding_udp_server_unit_tests branch from da9ee93 to 6f30f3a Compare October 18, 2024 19:14
@pafuent
Copy link
Contributor Author

pafuent commented Nov 12, 2024

@Calinou I addressed all your comments, Could you please review if I addressed them correctly?

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

@AThousandShips AThousandShips AThousandShips left review comments

@Faless Faless Faless left review comments

@Calinou Calinou Calinou requested changes

@RadiantUwU RadiantUwU RadiantUwU approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement topic:network topic:tests
Projects
None yet
Milestone
4.x
Development

Successfully merging this pull request may close these issues.
5 participants
@pafuent @Faless @Calinou @RadiantUwU @AThousandShips