-
Notifications
You must be signed in to change notification settings - Fork 153
Aspire-13.1 #1056
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
Aspire-13.1 #1056
Conversation
… methods - Updated multiple test files to replace calls to GetEnvironmentVariableValuesAsync() with GetEnvironmentVariablesAsync(). - Replaced calls to GetArgumentValuesAsync() with GetArgumentListAsync() in various test cases. - Introduced new utility methods in the CommunityToolkit.Aspire.Testing namespace for better argument and environment variable handling.
Updated infrastructure and test code to use Azure Redis Enterprise (Microsoft.Cache/redisEnterprise) instead of the legacy Azure Redis Cache. Refactored connection string handling and key retrieval to use RedisEnterpriseCluster and RedisEnterpriseDatabase resources. Updated Bicep templates and test assertions accordingly. Removed obsolete test for non-SSL port, as Redis Enterprise always requires TLS. This modernizes the codebase to align with current Azure recommendations.
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 pull request updates the Aspire Community Toolkit to use .NET Aspire 13.1.0, introducing new testing utilities and migrating from Azure Redis Cache to Azure Managed Redis Enterprise.
Changes:
- Upgraded Aspire SDK from 13.0.0 to 13.1.0 across all project files
- Introduced utility methods
GetEnvironmentVariablesAsync()andGetArgumentListAsync()in theCommunityToolkit.Aspire.Testingnamespace to simplify test code - Migrated Azure Dapr Redis integration from Azure Redis Cache to Azure Managed Redis Enterprise
Reviewed changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CommunityToolkit.Aspire.Testing/EnvironmentVariableEvaluator.cs | New utility class for evaluating environment variables in tests |
| tests/CommunityToolkit.Aspire.Testing/ArgumentEvaluator.cs | New utility class for evaluating command-line arguments in tests |
| tests/CommunityToolkit.Aspire.Testing/CommunityToolkit.Aspire.Testing.csproj | Updated Aspire SDK version to 13.1.0 |
| Multiple test files | Replaced direct calls with new utility methods for cleaner test code |
| tests/CommunityToolkit.Aspire.Hosting.Keycloak.Extensions.Tests/*.cs | Updated to manually process environment callback annotations instead of using utility methods |
| src/CommunityToolkit.Aspire.Hosting.Azure.Dapr.Redis/AzureRedisCacheDaprHostingExtensions.cs | Migrated from Azure Redis Cache to Azure Managed Redis Enterprise with updated resource types and connection handling |
| tests/CommunityToolkit.Aspire.Hosting.Azure.Dapr.Redis.Tests/ResourceCreationTests.cs | Updated tests to reflect Redis Enterprise migration and removed obsolete TLS test |
| Directory.Build.props | Updated Aspire version to 13.1.0 and other dependency versions |
| Directory.Packages.props | Fixed Keycloak package version format and updated OllamaSharp version |
| nuget.config | Added dotnet9 package source |
| All example AppHost .csproj files | Updated Aspire.AppHost.Sdk version to 13.1.0 |
| examples/dapr/CommunityToolkit.Aspire.Hosting.Azure.Dapr.AppHost/Program.cs | Updated to use AddAzureManagedRedis instead of AddAzureRedis |
| .github/agents/hosting-integration-creator.agent.md | Formatting improvements for list items |
Comments suppressed due to low confidence (1)
tests/CommunityToolkit.Aspire.Hosting.Azure.Dapr.Tests/AzureDaprPublishingHelperTests.cs:53
- This assignment to redisState is useless, since its value is never read.
| public static async ValueTask<Dictionary<string, string>> GetEnvironmentVariablesAsync( | ||
| this IResource resource, | ||
| DistributedApplicationOperation applicationOperation = DistributedApplicationOperation.Run, | ||
| IServiceProvider? serviceProvider = null) |
Copilot
AI
Jan 12, 2026
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.
The public method GetEnvironmentVariablesAsync is missing XML documentation comments. According to the project's coding guidelines, all public members require XML doc comments.
|
|
||
| public static class ArgumentEvaluator | ||
| { | ||
| public static async ValueTask<List<string>> GetArgumentListAsync(this IResource resource, IServiceProvider? serviceProvider = null) |
Copilot
AI
Jan 12, 2026
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.
The public method GetArgumentListAsync is missing XML documentation comments. According to the project's coding guidelines, all public members require XML doc comments.
| var env = context.EnvironmentVariables; | ||
|
|
||
| Assert.Equal("postgres", env["KC_DB"]); | ||
| Assert.True(env.ContainsKey("KC_DB_URL")); |
Copilot
AI
Jan 12, 2026
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.
Inefficient use of 'ContainsKey' and indexer.
| redisBuilder.ConfigureInfrastructure(infrastructure => | ||
| { | ||
| var redisResource = infrastructure.GetProvisionableResources().OfType<AzureRedisResource>().SingleOrDefault(); | ||
| var redisDatabase = infrastructure.GetProvisionableResources().OfType<RedisEnterpriseDatabase>().SingleOrDefault(); |
Copilot
AI
Jan 12, 2026
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.
This assignment to redisDatabase is useless, since its value is never read.
| using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish); | ||
|
|
||
| var redisState = builder.AddAzureRedis("redisState").RunAsContainer(); | ||
| var redisState = builder.AddAzureManagedRedis("redisState").RunAsContainer(); |
Copilot
AI
Jan 12, 2026
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.
This assignment to redisState is useless, since its value is never read.
This issue also appears in the following locations of the same file:
- line 53
This is from discussion with @FullStackChef as the test could be indicating something underlying but we can't figure out what so making the packages 'not stable' as a warning to users
Minimum allowed line rate is |
Closes #<ISSUE_NUMBER>
PR Checklist
Other information