feat: add contains_handshake and assert_contains_handshake helpers#3533
Merged
larseggert merged 2 commits intomozilla:mainfrom Apr 21, 2026
Merged
Conversation
mvanhorn
requested review from
KershawChang,
larseggert,
martinthomson and
mxinden
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3533 +/- ##
==========================================
- Coverage 94.53% 94.43% -0.11%
==========================================
Files 127 131 +4
Lines 39627 39957 +330
Branches 39627 39957 +330
==========================================
+ Hits 37462 37733 +271
- Misses 1328 1377 +49
- Partials 837 847 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
larseggert
reviewed
Apr 9, 2026
Collaborator
larseggert
left a comment
larseggert
left a comment
There was a problem hiding this comment.
Were you planning to actually use these new functions for something?
Contributor
Author
|
Yes - they're intended for tests where packet sizing causes a small Initial to be coalesced before the Handshake. The existing |
Collaborator
|
I'd suggest to combine those PRs (into this PR), so we can see whether those additions have the right shape overall. |
Add helpers that scan through coalesced long-header packets in a datagram to find a Handshake packet. This reduces test fragility when packet sizing causes a small Initial to be coalesced before the Handshake. Follows the same scanning pattern as assert_no_1rtt. Fixes mozilla#3447
…ndshake The two assertions after the server's second handshake burst were commented out because the datagram can contain a small coalesced Initial in front of the Handshake packet, which tripped the first-packet-only check in assert_handshake. Use the new assert_contains_handshake helper so the check is deterministic regardless of whether an Initial is coalesced at the front of the datagram.
mvanhorn
force-pushed
the
osc/3447-assert-contains-handshake
branch
from
fe90214 to
3325659
Compare
Contributor
Author
|
Pushed a consumer in That test had two The rest of the Ran Happy to grow the migration if you have other specific tests in mind. |
Contributor
Benchmark resultsNo significant performance differences relative to 7c0b85f. All resultstransfer/1-conn/1-100mb-resp (aka. Download)/mtu-1504: No change in performance detected. time: [204.15 ms 204.65 ms 205.24 ms]
thrpt: [487.24 MiB/s 488.65 MiB/s 489.83 MiB/s]
change:
time: [-0.6986% -0.3024% +0.0942] (p = 0.13 > 0.05)
thrpt: [-0.0941% +0.3033% +0.7035]
No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) high mild
3 (3.00%) high severetransfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1504: No change in performance detected. time: [285.47 ms 287.20 ms 288.91 ms]
thrpt: [34.613 Kelem/s 34.819 Kelem/s 35.030 Kelem/s]
change:
time: [-1.0760% -0.1804% +0.6795] (p = 0.68 > 0.05)
thrpt: [-0.6749% +0.1807% +1.0877]
No change in performance detected.transfer/1-conn/1-1b-resp (aka. HPS)/mtu-1504: No change in performance detected. time: [38.818 ms 38.947 ms 39.092 ms]
thrpt: [25.581 B/s 25.676 B/s 25.761 B/s]
change:
time: [-0.4803% +0.0617% +0.6116] (p = 0.83 > 0.05)
thrpt: [-0.6079% -0.0616% +0.4826]
No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
3 (3.00%) high mild
4 (4.00%) high severetransfer/1-conn/1-100mb-req (aka. Upload)/mtu-1504: Change within noise threshold. time: [204.47 ms 204.89 ms 205.33 ms]
thrpt: [487.01 MiB/s 488.08 MiB/s 489.06 MiB/s]
change:
time: [-0.8461% -0.5221% -0.2056] (p = 0.00 < 0.05)
thrpt: [+0.2060% +0.5249% +0.8533]
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severestreams/walltime/1-streams/each-1000-bytes: No change in performance detected. time: [596.01 µs 597.73 µs 599.81 µs]
change: [-0.4315% +0.2495% +0.8579] (p = 0.46 > 0.05)
No change in performance detected.
Found 8 outliers among 100 measurements (8.00%)
2 (2.00%) high mild
6 (6.00%) high severestreams/walltime/1000-streams/each-1-bytes: Change within noise threshold. time: [12.499 ms 12.541 ms 12.606 ms]
change: [+0.1341% +0.5267% +1.1002] (p = 0.01 < 0.05)
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severestreams/walltime/1000-streams/each-1000-bytes: Change within noise threshold. time: [45.550 ms 45.600 ms 45.650 ms]
change: [-0.6778% -0.3491% -0.1029] (p = 0.01 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-false/varying-seeds: Change within noise threshold. time: [78.503 ms 78.562 ms 78.621 ms]
change: [-0.9828% -0.7616% -0.5959] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [79.758 ms 79.807 ms 79.858 ms]
change: [+0.0394% +0.1571% +0.2667] (p = 0.01 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severetransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [78.197 ms 78.291 ms 78.402 ms]
change: [-0.8935% -0.7078% -0.5328] (p = 0.00 < 0.05)
Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
4 (4.00%) high mild
2 (2.00%) high severetransfer/walltime/pacing-true/same-seed: Change within noise threshold. time: [79.914 ms 79.972 ms 80.037 ms]
change: [-0.6600% -0.5234% -0.4054] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severeDownload data for |
Contributor
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Contributor
Client/server transfer resultsPerformance differences relative to b50af73. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
larseggert
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
contains_handshake()andassert_contains_handshake()to the test fixture assertions module. These scan through coalesced long-header packets in a datagram to check if any is a Handshake packet.Why this matters
Tests that assert on Handshake packets are fragile when packet sizing causes a small Initial to be coalesced before the Handshake. The existing
assert_handshake()only checks the first packet in a datagram.assert_contains_handshake()scans through all coalesced packets.Changes
test-fixture/src/assertions.rs: Addedcontains_handshake()andassert_contains_handshake()following the same coalesced-packet scanning pattern used byassert_no_1rtt().Testing
assert_no_1rttfor scanning coalesced packetscargo checklocally (NSS system dependency not available); GitHub CI will verifyFixes #3447
This contribution was developed with AI assistance (Claude Code).