Skip to content

Fix the AckSendDelay test #5125

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

Merged
merged 5 commits into from
Jun 4, 2025
Merged

Conversation

guhetier
Copy link
Contributor

Description

The AckSendDelay test was flaky and of doubtful utility:

  • it asserts there was only a single ACK received after a client send a buffer, which isn't fully true: other frames can be sent for reasons outside of the test control (for instance, NEW_CONNECTION_ID).

  • the ack delay feature tested is more about whether an ACK is eventually sent if no other ack-eliciting frame is received, and less about whether an ack piggy-pack on the next frame sent by the server (which is an MsQuic implementation detail)

The test is re-written to validate an ACK is received within a reasonable amount of time after a send.

Testing

N/A

Documentation

N/A

@guhetier guhetier self-assigned this May 27, 2025
@guhetier guhetier added the Area: Testing Related to test coverage label May 27, 2025
Copy link

codecov bot commented May 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.33%. Comparing base (bb17196) to head (e2710af).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5125      +/-   ##
==========================================
- Coverage   85.93%   85.33%   -0.61%     
==========================================
  Files          59       59              
  Lines       18048    18048              
==========================================
- Hits        15510    15401     -109     
- Misses       2538     2647     +109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guhetier guhetier marked this pull request as ready for review May 30, 2025 00:56
@guhetier guhetier requested a review from a team as a code owner May 30, 2025 00:56
@guhetier guhetier merged commit 72593a3 into main Jun 4, 2025
358 of 365 checks passed
@guhetier guhetier deleted the user/guhetier/QuicTestAckSendDelay_fix branch June 4, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Testing Related to test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants