Lower default SSL peek test rounds and remove CI workarounds#3155
Merged
Lower default SSL peek test rounds and remove CI workarounds#3155
Conversation
5477317 to
ba79cf1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3155 +/- ##
==========================================
- Coverage 77.95% 77.93% -0.02%
==========================================
Files 689 689
Lines 122537 122537
Branches 17071 17070 -1
==========================================
- Hits 95526 95503 -23
- Misses 26112 26136 +24
+ Partials 899 898 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dougch
reviewed
Apr 9, 2026
Contributor
dougch
left a comment
There was a problem hiding this comment.
nit; the comment on https://github.com/aws/aws-lc/blob/main/CMakeLists.txt#L1342 needs an update.
dougch
previously approved these changes
Apr 10, 2026
09f6a14 to
2ea8a40
Compare
dougch
approved these changes
Apr 10, 2026
samuel40791765
approved these changes
Apr 10, 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
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.
Issues:
Resolves #3141
Description of changes:
The SSL runner's
DEFAULT_PEEK_ROUNDSof 50 has been causing i/o timeouts on non-Linux platforms for a while now. We've been working around this by settingAWS_LC_SSL_TEST_RUNNER_PEEK_ROUNDS=5in CI for Windows, OpenBSD, FreeBSD, and NetBSD — but that leaves anyone building locally on those platforms hitting the same failures (#3141).This lowers the default from 50 to 5 and removes all the per-job CI overrides. The
sanity-test-runjob (Linux,ubuntu-latest) now runs withAWS_LC_SSL_TEST_RUNNER_PEEK_ROUNDS=100to provide stress-test coverage of the peek buffer drain/refill cycle on a platform that can comfortably handle it.Call-outs:
The peek test verifies the
SSL_peek → SSL_peek → SSL_readstate transition per TLS record. Each round exercises the same code path — the internal buffer fill/drain cycle throughssl->s3->pending_app_data. 5 rounds is sufficient for correctness; higher round counts only repeat the same transition and don't exercise additional code paths. The env var override remains available for targeted stress testing.The
AWS_LC_SSL_TEST_RUNNER_PEEK_ROUNDSenv var support inrunner.goand theAWS_LC_SSL_RUNNER_IDLE_TIMEOUTCMake plumbing are intentionally preserved as general-purpose knobs for slow environments or stress testing.Testing:
CI should pass without the per-platform peek rounds overrides. The FreeBSD, OpenBSD, NetBSD, and Windows jobs are the ones to watch. The
sanity-test-runjob validates the stress-test configuration at 100 rounds on Linux.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.