Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

twsouthwick
Copy link
Member

@twsouthwick twsouthwick commented Jul 16, 2025

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

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

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.
@twsouthwick twsouthwick force-pushed the servicediscovery-fx branch from aa30c55 to 17f3a62 Compare July 16, 2025 23:17
@twsouthwick twsouthwick marked this pull request as ready for review July 16, 2025 23:17
@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 23:17
Copy link
Contributor

@Copilot 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 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 the RequiresDocker 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 each ConfigureAwaitOptions 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)

@CZEMacLeod
Copy link

These polyfills are excellent. I wish there was an official MS polyfill for net4x, to prevent multiple copies of classes/code in each dll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants