[tests] Fix flaky SecureTransportTest.Tls12 by ignoring network timeouts in CI#25302
[tests] Fix flaky SecureTransportTest.Tls12 by ignoring network timeouts in CI#25302rolfbjarne wants to merge 2 commits intomainfrom
Conversation
…uts in CI Wrap the Tls12 test body in a try/catch that calls TestRuntime.IgnoreInCIIfBadNetwork to mark the test as ignored (rather than failed) when transient network issues occur in CI. Also extend IgnoreInCIIfTimedOut to handle SocketException timeouts, not just WebException timeouts, so that the SocketException thrown by TcpClient is properly recognized. Fixes #25241 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce flakiness in the SecureTransport external-network test suite by treating transient CI network timeouts as ignorable test noise instead of hard failures. It updates the SecureTransport TLS 1.2 test and the shared test runtime helper that classifies transient network errors across the repository.
Changes:
- Wrapped
SecureTransportTest.Tls12in atry/catchso CI can ignore recognized transient network failures instead of failing the test. - Extended
TestRuntime.IgnoreInCIIfTimedOut (Exception ex)to recognizeSocketExceptiontimeouts in addition to the existingWebExceptiontimeout handling. - Kept the existing TLS 1.2 handshake/request assertions intact while routing failures through the shared CI network-flake helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/monotouch-test/Security/SecureTransportTest.cs |
Adds CI-specific bad-network handling around the external TLS 1.2 test. |
tests/common/TestRuntime.cs |
Extends shared timeout classification so socket connect timeouts can be ignored in CI. |
| var result = ssl.Handshake (); | ||
| while (result == SslStatus.WouldBlock || result == (SslStatus) (-108)) { | ||
| // we need to ask again - but if we're too fast we'll get -108 (errSecAllocate) | ||
| Thread.Sleep (100); | ||
| // during the above call SessionState is Handshake | ||
| Assert.That (ssl.SessionState, Is.EqualTo (SslSessionState.Handshake), "Handshake/in progress"); |
| result = ssl.Read (data, out processed); | ||
| Assert.That (result, Is.EqualTo (SslStatus.Success), "Read"); | ||
|
|
||
| string s = Encoding.UTF8.GetString (data, 0, (int) processed); | ||
| // The result apparently depends on where you are: I get a 302, the bots get a 200. | ||
| // Also sometimes it fails with 502 Bad Gateway on the bots | ||
| Assert.That (s, Does.StartWith ("HTTP/1.0 302 Found").Or.StartWith ("HTTP/1.0 200 OK").Or.StartWith ("HTTP/1.0 502 Bad Gateway"), "response"); | ||
| while (result == SslStatus.WouldBlock) | ||
| result = ssl.Read (data, out processed); |
| while (result == SslStatus.WouldBlock || result == (SslStatus) (-108)) { | ||
| // we need to ask again - but if we're too fast we'll get -108 (errSecAllocate) |
- Add 30-second deadlines to both the handshake and read retry loops to prevent indefinite hangs in CI. - Replace magic number -108 with the existing errSecAllocate constant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #9a8e2ad] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #9a8e2ad] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #9a8e2ad] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #9a8e2ad] Test results 🔥Test results❌ Tests failed on VSTS: test results 2 tests crashed, 1 tests failed, 160 tests passed. Failures❌ monotouch tests (iOS)1 tests failed, 12 tests passed.Failed tests
Html Report (VSDrops) Download ❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests🔥 Failed catastrophically on VSTS: test results - mac_monterey (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Wrap the Tls12 test body in a try/catch that calls TestRuntime.IgnoreInCIIfBadNetwork to mark the test as ignored (rather than failed) when transient network issues occur in CI.
Also extend IgnoreInCIIfTimedOut to handle SocketException timeouts, not just WebException timeouts, so that the SocketException thrown by TcpClient is properly recognized.
Fixes #25241