Skip to content

Conversation

@jiceathome
Copy link
Contributor

Fixes: #4861

@jiceathome jiceathome changed the title Fix4861 test: ping has extra delay at the end Dec 25, 2025
@oncilla oncilla requested a review from Copilot December 29, 2025 12:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where the ping function had an extra delay at the end of its execution. The change modifies the loop structure to avoid waiting for a final tick after all ping attempts have been sent.

Key changes:

  • Restructured the ping sending loop to break before the final sleep/tick wait
  • Changed from a traditional for loop to a manual counter decrement approach

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

defer log.HandlePanic()
defer wg.Done()
for i := uint16(0); i < p.attempts; i++ {

Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

If p.attempts is 0, the loop will run indefinitely since i will underflow when decremented. Add a check to handle the case where p.attempts is 0, or ensure p.attempts is always greater than 0.

Suggested change
if p.attempts == 0 {
// No packets to send; avoid underflow in the decrementing loop.
return
}

Copilot uses AI. Check for mistakes.
defer wg.Done()
for i := uint16(0); i < p.attempts; i++ {

i := p.attempts
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The variable i should retain the uint16 type annotation for clarity and to match the original type, e.g., i := uint16(p.attempts) or ensure type consistency with the original loop counter.

Copilot uses AI. Check for mistakes.
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.

test: ping has unnecessary delay

1 participant