Skip to content

fix: treat missing expected cookie as transient network failure in TestNSurlSessionHandlerCookieContainerSetCookie#25420

Merged
rolfbjarne merged 3 commits into
mainfrom
copilot/fix-flaky-test-nsurlsessionhandler
May 13, 2026
Merged

fix: treat missing expected cookie as transient network failure in TestNSurlSessionHandlerCookieContainerSetCookie#25420
rolfbjarne merged 3 commits into
mainfrom
copilot/fix-flaky-test-nsurlsessionhandler

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 13, 2026

  • Fix TestNSurlSessionHandlerCookieContainerSetCookie to treat missing expected cookie as transient network failure (ignore in CI)
  • Address race condition: move CookieContainer inspection after !completed guard to avoid concurrent access
  • Address exception masking: separate !completed and !hasExpectedCookie CI-ignore guards so real exceptions still fail the test

…stNSurlSessionHandlerCookieContainerSetCookie

Agent-Logs-Url: https://github.com/dotnet/macios/sessions/08f85733-39b4-40c6-bcf5-d46ae2c50f48

Co-authored-by: rolfbjarne <249268+rolfbjarne@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix flaky test NSurlSessionHandlerCookieContainerSetCookies on macOS fix: treat missing expected cookie as transient network failure in TestNSurlSessionHandlerCookieContainerSetCookie May 13, 2026
Copilot AI requested a review from rolfbjarne May 13, 2026 09:56
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne rolfbjarne marked this pull request as ready for review May 13, 2026 10:18
Copilot AI review requested due to automatic review settings May 13, 2026 10:18
Copy link
Copy Markdown
Contributor

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 adjusts a flaky network-dependent test (TestNSurlSessionHandlerCookieContainerSetCookie) to treat “request completed but expected cookie missing” as a transient CI failure, reducing spurious macOS CI failures in the System.Net.Http test suite.

Changes:

  • Precomputes whether the expected cookie was stored in the CookieContainer after the async request.
  • Expands the CI ignore guard to include missing-cookie cases in addition to timeouts.
  • Reuses the computed boolean in the final assertion.

Comment thread tests/monotouch-test/System.Net.Http/MessageHandlers.cs Outdated
Comment thread tests/monotouch-test/System.Net.Http/MessageHandlers.cs Outdated
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne
Copy link
Copy Markdown
Member

@copilot address the comments

…HandlerCookieContainerSetCookie

Agent-Logs-Url: https://github.com/dotnet/macios/sessions/269d09e0-2844-4dfa-9c95-470afec2eead

Co-authored-by: rolfbjarne <249268+rolfbjarne@users.noreply.github.com>
auto-merge was automatically disabled May 13, 2026 10:47

Head branch was pushed to by a user without write access

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 13, 2026

@copilot address the comments

Addressed both review comments in 883e89e:

  1. Race condition: CookieContainer is now only accessed after the !completed CI-ignore guard (and Assert.IsTrue(completed)), so we only read it when the async task has actually finished.
  2. Exception masking: Split into two separate guards — !completed first, then ex is asserted null, then !hasExpectedCookie — so a real exception still fails the test rather than being silently ignored.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Copilot AI requested a review from rolfbjarne May 13, 2026 10:48
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [PR Build #883e89e] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 883e89e18404044b4a2eaed5220a13a8b58703d2 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [PR Build #883e89e] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 883e89e18404044b4a2eaed5220a13a8b58703d2 [PR build]

@rolfbjarne rolfbjarne enabled auto-merge (squash) May 13, 2026 11:17
@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 883e89e18404044b4a2eaed5220a13a8b58703d2 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [PR Build #883e89e] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 883e89e18404044b4a2eaed5220a13a8b58703d2 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🚀 [CI Build #883e89e] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 175 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. (⚠️ Html Report Publish failed ⚠️) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. (⚠️ Html Report Publish failed ⚠️) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 6 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 16 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 18 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 16 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

Linux build succeeded

Pipeline on Agent
Hash: 883e89e18404044b4a2eaed5220a13a8b58703d2 [PR build]

@rolfbjarne rolfbjarne merged commit 2daba49 into main May 13, 2026
45 checks passed
@rolfbjarne rolfbjarne deleted the copilot/fix-flaky-test-nsurlsessionhandler branch May 13, 2026 13:46
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.

[CI Postmortem] Flaky: TestNSurlSessionHandlerCookieContainerSetCookies on macOS

5 participants