Stabilize CachedResponseBodyTests.Copy_SingleSegment

[irfanajaffer]

Stabilize CachedResponseBodyTests.Copy_SingleSegment

Description

This PR addresses flakiness in CachedResponseBodyTests.Copy_SingleSegment.

The test could intermittently fail with System.OperationCanceledException during CI execution. The failure was determined to originate from the test implementation rather than the CachedResponseBody production code.

The test uses a shared CancellationTokenSource with a 5-second timeout for both the producer and consumer operations. Under normal execution the copy and read operations complete successfully. However, under heavy CI load or thread-pool contention, the timeout can expire while the operations are still making forward progress. When this occurs, either PipeReader.ReadAsync(...) or CachedResponseBody.CopyToAsync(...) observes cancellation and throws OperationCanceledException, causing the test to fail despite the underlying functionality behaving correctly.

To make the test more resilient, timeout-driven cancellation has been removed from the copy and receive operations and replaced with an explicit hang guard using Task.WaitAsync(...). This preserves protection against hangs while allowing valid in-flight work to complete regardless of temporary scheduling delays.

The updated implementation explicitly:

• Removes cancellation-based failure paths from the producer and consumer operations.
• Preserves timeout protection through a dedicated hang guard.
• Allows genuine hangs to surface as TimeoutException instead of injecting cancellation into otherwise healthy operations.
• Eliminates the race condition between task completion and timeout expiration.

With these changes in place, the test no longer depends on a timeout-based cancellation mechanism and is resilient to transient CI scheduling delays.

---

Validation / Investigation

As part of the investigation:

1. The failure was analyzed and determined to be caused by timeout-triggered cancellation rather than a defect in CachedResponseBody.
2. The test flow was reviewed and the shared CancellationTokenSource was identified as the source of the intermittent OperationCanceledException.
3. The timeout logic was replaced with a dedicated hang guard using Task.WaitAsync(...).
4. Additional stress runs were performed to verify that the test remains stable under varying scheduling conditions.

While the issue is timing-dependent and primarily manifests under CI contention, the updated implementation removes the cancellation race entirely by allowing the producer and consumer to complete naturally while enforcing an external completion deadline.

---

Changes

1. Removed timeout-based CancellationTokenSource usage from producer and consumer operations.
2. Updated the test to execute copy and receive operations using CancellationToken.None.
3. Replaced cancellation-driven timeout behavior with Task.WaitAsync(...) hang-guard protection.
4. Added debugger-aware timeout handling to prevent interference during local debugging sessions.
5. Ensured genuine hangs continue to fail deterministically while eliminating false failures caused by cancellation timing races.

Fixes #61670.

[dotnet-policy-service]

Thanks for your PR, @irfanajaffer. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Pull request overview

This PR stabilizes CachedResponseBodyTests.Copy_SingleSegment by removing timeout-driven cancellation from the producer/consumer paths (which could surface as intermittent OperationCanceledException under CI contention) and replacing it with an external hang-guard via Task.WaitAsync(...).

Changes:

• Replace the shared 5s CancellationTokenSource pattern with a debugger-aware HangGuardTimeout and Task.WhenAll(...).WaitAsync(...).
• Run copy/receive operations with CancellationToken.None to avoid injecting cancellation into healthy in-flight work.
• Simplify CopyDataAsync to no longer accept a cancellation token, and adjust PipeReader.ReadAsync usage accordingly.

Show a summary per file

File	Description
src/Middleware/ResponseCaching/test/CachedResponseBodyTests.cs	Reworks the test timeout strategy to avoid cancellation-based flakiness and uses a hang guard for deterministic failures.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 2

[kotlarmilos]

Tests passed in the CI, @BrennanConroy PTAL

[PureWeen]

🤖 GitHub Copilot review

Moving the timeout outside the producer and consumer addresses the issue-backed problem because the test no longer injects cancellation into the pipe operations.

The remaining question is whether this test should continue using a fixed five-second observer or use the repository’s standard DefaultTimeout() policy. A focused local repro deliberately delayed the producer by six seconds. The current PR timed out at five seconds, while replacing WaitAsync(HangGuardTimeout) with DefaultTimeout() passed the identical test 3/3 and the full test class 20/20 in Release.

That experiment proves the timeout-policy difference. It does not reproduce the original CI scheduling failure or establish that completing after five seconds is part of the required test contract.

Would you be open to using DefaultTimeout() for all three waits? It would align this test with the repository timeout policy and diagnostics and provide additional headroom in Release builds. If the five-second bound is intentional for this test, the current PR still fixes the cancellation-injection problem reported in #61670.

This review was generated by GitHub Copilot using local red/green validation. Please reply with how accurate this was and any context it missed. That feedback will be used to improve the reviewer.

[irfanajaffer]

@PureWeen ,

Thanks for the review. The primary goal of this PR was to eliminate timeout-driven cancellation in the pipe operations. I'm also fine with using DefaultTimeout() for consistency with the repository test infrastructure if there's no requirement to enforce the current 5-second limit.

[kotlarmilos]

Please note that this component we are planning to obsolete: #62728, so fixing a flaky test there is not interesting.