Skip to content

Add test for udp socket reachability#6676

Merged
gregcusack merged 3 commits intoanza-xyz:masterfrom
gregcusack:add-test-udp-reachability
Jun 23, 2025
Merged

Add test for udp socket reachability#6676
gregcusack merged 3 commits intoanza-xyz:masterfrom
gregcusack:add-test-udp-reachability

Conversation

@gregcusack
Copy link
Copy Markdown

Problem

Follow up to: #6611
I forgot to add a test to check that the new udp reachability function properly works with multiple IPs

Summary of Changes

add test to make sure the updated udp reachability function works properly

Comment thread net-utils/src/lib.rs Outdated
let mut udp_sockets = Vec::new();
for _ in 0..MAX_PORT_VERIFY_THREADS * 2 {
let (_p1, (sock_a, _tl_a)) =
bind_common_in_range_with_config(ip_a, (3000, 4000), config).unwrap();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would be nice if these used the ports from reserved range too. you can call localhost_port_range_for_tests multiple times if needed it will yield more unique ranges. Otherwise this test might flake due to silly port conflicts.

alexpyattaev
alexpyattaev previously approved these changes Jun 21, 2025
Copy link
Copy Markdown

@alexpyattaev alexpyattaev left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@alexpyattaev alexpyattaev dismissed their stale review June 21, 2025 18:33

github showed CI green...

Comment thread net-utils/src/lib.rs Outdated
for _ in 0..MAX_PORT_VERIFY_THREADS {
let (_p1, (sock_a, _tl_a)) = bind_common_in_range_with_config(
ip_a,
sockets::localhost_port_range_for_tests(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You really do not need to run this so many times, each range has 20 IPs, but calling these in a loop exhausts the quota for the unittest. You can use fewer iterations here (I do not think we even need the for loop on line 1000) or allocate one range for both binds within the loop (so you are using less ranges).

@gregcusack gregcusack force-pushed the add-test-udp-reachability branch from ba0f73d to 63afe7d Compare June 23, 2025 15:41
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.4%. Comparing base (c4f5c58) to head (63afe7d).
Report is 14 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6676   +/-   ##
=======================================
  Coverage    83.4%    83.4%           
=======================================
  Files         850      850           
  Lines      377876   377916   +40     
=======================================
+ Hits       315320   315388   +68     
+ Misses      62556    62528   -28     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gregcusack gregcusack requested a review from alexpyattaev June 23, 2025 17:45
Copy link
Copy Markdown

@alexpyattaev alexpyattaev left a comment

Choose a reason for hiding this comment

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

Thank you!

@gregcusack gregcusack merged commit 55b5c08 into anza-xyz:master Jun 23, 2025
28 checks passed
@gregcusack gregcusack deleted the add-test-udp-reachability branch June 23, 2025 20:34
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