-
Notifications
You must be signed in to change notification settings - Fork 659
Build ServiceDiscovery library and tests against .NET Framework #10470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
adb78e1
to
aa30c55
Compare
This enables scenarios to use the library on .NET Framework. This is mostly done by using C# 14 new extension syntax so very little code changes were made but rather the APIs that didn't exist are added into FrameworkExtensions classes to light them up on .NET Core builds.
aa30c55
to
17f3a62
Compare
There was a problem hiding this 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 enables the ServiceDiscovery library and its tests to build and run on .NET Framework by multi-targeting the projects and providing polyfills for newer APIs via C# 14 extension syntax.
- Multi-target key projects and tests against net462/net472 alongside existing frameworks.
- Added a shared
FxPolyfills
folder with extension methods to surface missing .NET Core APIs on .NET Framework. - Updated test and service discovery projects to conditionally include additional references and imports.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/Shared/TemplatesTesting/Aspire.Shared.TemplatesTesting.targets | Added IncludeTestUtilities property and removed direct test utilities reference |
tests/Microsoft.Extensions.ServiceDiscovery.Tests/…Tests.cs | Suppressed a type conflict warning around the fake resolver factory |
tests/**/*.csproj | Switched to multi-targeting net472 on Windows and added framework-specific package imports |
tests/Aspire.TestUtilities/Aspire.TestUtilities.csproj | Multi-target net472 and import polyfills targets for framework support |
src/Shared/FxPolyfills/*.cs | Introduced extension polyfills for various System and threading APIs on .NET Framework |
src/Shared/FxPolyfills/FxPolyfills.targets | Configured inclusion/exclusion of polyfill sources based on targeting |
src/Microsoft.Extensions.ServiceDiscovery*.csproj | Multi-target net462, conditional AOT flag, reference Bcl packages, and import polyfills |
src/Microsoft.Extensions.ServiceDiscovery/ServiceDiscoveryHttpClientBuilderExtensions.cs | Wrapped gRPC-disable filter in #if NET for proper cross-target behavior |
eng/Versions.props & Directory.Packages.props | Added and updated version properties for newly referenced packages |
Comments suppressed due to low confidence (3)
tests/Shared/TemplatesTesting/Aspire.Shared.TemplatesTesting.targets:18
- The ProjectReference to
Aspire.TestUtilities
was removed, which may break resolution of theRequiresDocker
attribute and other test utilities. Please re-add or replace this reference so that the shared test utilities are included in the build.
</ItemGroup>
src/Shared/FxPolyfills/Task.cs:20
- Add unit tests for
FxPolyfillTask.ConfigureAwait
covering eachConfigureAwaitOptions
path to ensure the polyfill behaves identically to its .NET Core counterpart.
public async Task ConfigureAwait(ConfigureAwaitOptions options)
src/Shared/FxPolyfills/Task.TimeProvider.cs:12
- Consider adding tests for
FxPolyfillTask.WaitAsync
to validate timeout and cancellation behavior on .NET Framework scenarios.
public Task WaitAsync(CancellationToken token)
These polyfills are excellent. I wish there was an official MS polyfill for net4x, to prevent multiple copies of classes/code in each dll. |
Description
This enables scenarios to use the library on .NET Framework. This is mostly done by using C# 14 new extension syntax so very little code changes were made but rather the APIs that didn't exist are added into FrameworkExtensions classes to light them up on .NET Core builds.
NOTE: This updates the abstractions and main ServiceDiscovery library, but does not enable the DNS nor YARP. YARP isn't supported on framework, so that isn't necessary. The DNS has a larger change due to more APIs it's using that don't exist, so once this PR gets in I can push that up if this pattern works.
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template