Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13769Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13769" |
There was a problem hiding this comment.
Pull request overview
This PR fixes flaky tests in WithHttpCommandTests by addressing three core issues:
- Simplified UpdateState logic: Replaced a complex background task approach with direct access to
context.ResourceSnapshot.State, eliminating race conditions - Disabled resilience retries in tests: Prevents long-running retry loops that caused slow test execution (especially for 500 status code tests)
- Replaced DCP dependency with custom resources: Uses
CustomResourcewith pre-allocated endpoints andFakeHttpMessageHandlerinstead of relying on DCP to build/launch projects, improving test reliability and speed
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Aspire.Hosting/ResourceBuilderExtensions.cs |
Simplified UpdateState logic to directly read resource state from context instead of using a background task watching resource events |
tests/Aspire.Hosting.Tests/WithHttpCommandTests.cs |
Refactored all tests to use CustomResource with FakeHttpMessageHandler instead of real projects, added helper methods for resource/endpoint creation, disabled HTTP resilience retries, and removed quarantine attributes |
Co-authored-by: JamesNK <303201+JamesNK@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
I noticed that if I run the same .NET project multiple times, in parallel, from different tests, I get unpredictable errors from the build. I suspect this was what was going on here. |
|
I changed all the |
Yeah no idea, was too long ago. |
|
Keep an eye on enabling/disabling HTTP commands for the upcoming release. If it isn't what you expect, this PR is the problem! |
Should fix #9670
Should fix #9725
Should fix #9800
Should fix #9818
Should fix #9789
Should fix #9772
Should fix #8101
Should fix #9811
Various issues:
UpdateStatewas odd. There was a background task looking at the resource state and then using that to update the command state the next time the resource was updated (maybe it would update immediately, may it would wait for the next update before the state was it. It was a race). The problem is if the resource was never updated again then the command state would never change. Caused one flaky test. The fix is to use the resource status from the context which is much simpler. @DamianEdwards is there a reason whyUpdateStatewas written this way?