Upgrade to .NET 10#109
Conversation
- Bump TargetFramework from net7.0 to net10.0 across library and tests - Upgrade Microsoft.Extensions.* packages to 10.0.6 - Upgrade test framework packages (xunit, Microsoft.NET.Test.Sdk, coverlet, etc.) - Update CI/CD workflows to dotnet-version 10.0.x - Replace deprecated PackageLicenseUrl with PackageLicenseExpression (AGPL-3.0-only) - Fix CS8602 null dereference warning in Operator<T>.GetQueryString - Inject stub configuration in test fixtures so tests run without real credentials
WalkthroughThis pull request upgrades the .NET SDK version from 7.0.x to 10.0.x across CI/CD workflows and project target frameworks, replaces environment variable-based test configuration with in-memory configuration dictionaries, fixes a null-safety issue in query operator string conversion, and refreshes multiple NuGet dependencies to compatible versions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/dotnet-cd.yml (1)
24-31:⚠️ Potential issue | 🟠 MajorCD "real API" run is partially stubbed after this PR.
The
testjob setsTEST_REAL_API=trueand provides realSpaceTrackSdkOptions__*secrets so the suite hits the live Space-Track API.ClientTestFixturerespects this viaAddEnvironmentVariables(), butServiceCollectionExtensionsTests(seesrc/SpaceTrackSdk.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cslines 15–22) hardcodes stub configuration and does not callAddEnvironmentVariables(), so itsResolvedClientActuallyWorkstest always runs againsthttp://stub.localregardless ofTEST_REAL_API. Either fix the test class to honour environment variables (preferred) or explicitly document that this job no longer exercises that assertion against the real API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/dotnet-cd.yml around lines 24 - 31, ServiceCollectionExtensionsTests currently hardcodes a stub config (causing ResolvedClientActuallyWorks to hit http://stub.local) instead of honoring environment variables; update the test to build its IConfiguration using ConfigurationBuilder().AddEnvironmentVariables() (or reuse ClientTestFixture's configuration) and remove or make the hardcoded SpaceTrackSdkOptions conditional so that when TEST_REAL_API / SpaceTrackSdkOptions__* are present the resolved client points at the real API; specifically modify ServiceCollectionExtensionsTests and the ResolvedClientActuallyWorks setup to read configuration from AddEnvironmentVariables() (or accept an injected IConfiguration) rather than the existing hardcoded values.
🧹 Nitpick comments (1)
src/SpaceTrackSdk/SpaceTrackSdk.csproj (1)
4-4: Confirm .NET 10 support expectations for library consumers.Bumping a published NuGet library's
TargetFrameworkdirectly fromnet7.0tonet10.0(with no multi-targeting) drops support for every intermediate TFM, so consumers onnet8.0/net9.0(still in support) will no longer be able to reference new versions of this package. If that's intentional, fine — otherwise consider<TargetFrameworks>net8.0;net10.0</TargetFrameworks>(ornetstandard2.0+net10.0) to keep the consumer surface broad.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SpaceTrackSdk/SpaceTrackSdk.csproj` at line 4, The project file currently sets a single TargetFramework of net10.0 which will drop support for intermediate TFMs; update the csproj to multi-target so consumers on net8.0/net9.0 can still use the package—replace the single <TargetFramework>net10.0</TargetFramework> with a multi-targeting element (e.g. <TargetFrameworks>net8.0;net10.0</TargetFrameworks>) or another appropriate combination (or include netstandard2.0 plus net10.0) to restore the broader consumer surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/SpaceTrackSdk.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs`:
- Around line 15-23: The test removed AddEnvironmentVariables so
ServiceCollectionExtensionsTests (and specifically ResolvedClientActuallyWorks)
no longer picks up real CI secrets and now performs real HTTP calls to the
hard-coded http://stub.local; restore the previous behavior by calling
IConfigurationBuilder.AddEnvironmentVariables() before or along with the
in-memory collection and then branch on the TEST_REAL_API env var like
ClientTestFixture does: when TEST_REAL_API is not true, register HttpHandlerStub
in the DI setup and call ConfigurePrimaryHttpMessageHandler to wire the stubbed
handler into the HttpClient used by the resolved client; alternatively ensure
ResolvedClientActuallyWorks explicitly registers HttpHandlerStub +
ConfigurePrimaryHttpMessageHandler so no real network call is made.
---
Outside diff comments:
In @.github/workflows/dotnet-cd.yml:
- Around line 24-31: ServiceCollectionExtensionsTests currently hardcodes a stub
config (causing ResolvedClientActuallyWorks to hit http://stub.local) instead of
honoring environment variables; update the test to build its IConfiguration
using ConfigurationBuilder().AddEnvironmentVariables() (or reuse
ClientTestFixture's configuration) and remove or make the hardcoded
SpaceTrackSdkOptions conditional so that when TEST_REAL_API /
SpaceTrackSdkOptions__* are present the resolved client points at the real API;
specifically modify ServiceCollectionExtensionsTests and the
ResolvedClientActuallyWorks setup to read configuration from
AddEnvironmentVariables() (or accept an injected IConfiguration) rather than the
existing hardcoded values.
---
Nitpick comments:
In `@src/SpaceTrackSdk/SpaceTrackSdk.csproj`:
- Line 4: The project file currently sets a single TargetFramework of net10.0
which will drop support for intermediate TFMs; update the csproj to multi-target
so consumers on net8.0/net9.0 can still use the package—replace the single
<TargetFramework>net10.0</TargetFramework> with a multi-targeting element (e.g.
<TargetFrameworks>net8.0;net10.0</TargetFrameworks>) or another appropriate
combination (or include netstandard2.0 plus net10.0) to restore the broader
consumer surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b9915aa-099c-4ddd-87df-8be33c42eee2
📒 Files selected for processing (7)
.github/workflows/dotnet-cd.yml.github/workflows/dotnet-ci.ymlsrc/SpaceTrackSdk.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cssrc/SpaceTrackSdk.Tests/Fixtures/ClientTestFixture.cssrc/SpaceTrackSdk.Tests/SpaceTrackSdk.Tests.csprojsrc/SpaceTrackSdk/Public/Queries/Operators/Operator.cssrc/SpaceTrackSdk/SpaceTrackSdk.csproj
| IConfigurationBuilder configBuilder = new ConfigurationBuilder(); | ||
| configBuilder.AddEnvironmentVariables(); | ||
| configBuilder.AddInMemoryCollection(new Dictionary<string, string?> | ||
| { | ||
| ["SpaceTrackSdkOptions:Username"] = "stub", | ||
| ["SpaceTrackSdkOptions:Password"] = "stub", | ||
| ["SpaceTrackSdkOptions:ApiUrl"] = "http://stub.local", | ||
| ["SpaceTrackSdkOptions:AuthEndpoint"] = "/ajaxauth/login", | ||
| }); | ||
| IConfigurationRoot config = configBuilder.Build(); |
There was a problem hiding this comment.
Regression: this test class no longer honours the real-API credentials in CD.
Previously the setup called AddEnvironmentVariables(), so in .github/workflows/dotnet-cd.yml (where TEST_REAL_API=true and SpaceTrackSdkOptions__Username/Password/ApiUrl/AuthEndpoint are set as real secrets) ResolvedClientActuallyWorks would hit the real Space-Track API. After this change the configuration is hard-coded to http://stub.local with no HttpHandlerStub registered on the DI container built here (unlike ClientTestFixture, this class doesn't swap in the stub handler). On the CD pipeline service.Announcements.Get() will therefore attempt a real HTTP call to http://stub.local and fail, and in CI the same call will happen without any stub in place.
If these tests were previously relying on the env-var-driven real API path in CD, mirror the fixture's behaviour (branch on TEST_REAL_API, add env-vars first, register HttpHandlerStub + ConfigurePrimaryHttpMessageHandler when stubbed). Otherwise, at minimum ResolvedClientActuallyWorks needs a stub HTTP handler wired up so it doesn't perform a real network call against http://stub.local.
♻️ Sketch
- IConfigurationBuilder configBuilder = new ConfigurationBuilder();
- configBuilder.AddInMemoryCollection(new Dictionary<string, string?>
- {
- ["SpaceTrackSdkOptions:Username"] = "stub",
- ["SpaceTrackSdkOptions:Password"] = "stub",
- ["SpaceTrackSdkOptions:ApiUrl"] = "http://stub.local",
- ["SpaceTrackSdkOptions:AuthEndpoint"] = "/ajaxauth/login",
- });
- IConfigurationRoot config = configBuilder.Build();
-
- ServiceCollection services = new();
- services.AddSpaceTrackServices(config);
+ bool useRealApi = Environment.GetEnvironmentVariable("TEST_REAL_API") is not null;
+
+ IConfigurationBuilder configBuilder = new ConfigurationBuilder();
+ configBuilder.AddEnvironmentVariables();
+ if (!useRealApi)
+ {
+ configBuilder.AddInMemoryCollection(new Dictionary<string, string?>
+ {
+ ["SpaceTrackSdkOptions:Username"] = "stub",
+ ["SpaceTrackSdkOptions:Password"] = "stub",
+ ["SpaceTrackSdkOptions:ApiUrl"] = "http://stub.local",
+ ["SpaceTrackSdkOptions:AuthEndpoint"] = "/ajaxauth/login",
+ });
+ }
+ IConfigurationRoot config = configBuilder.Build();
+
+ ServiceCollection services = new();
+ services.AddSpaceTrackServices(config);
+ if (!useRealApi)
+ {
+ // swap in HttpHandlerStub, mirroring ClientTestFixture
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/SpaceTrackSdk.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs`
around lines 15 - 23, The test removed AddEnvironmentVariables so
ServiceCollectionExtensionsTests (and specifically ResolvedClientActuallyWorks)
no longer picks up real CI secrets and now performs real HTTP calls to the
hard-coded http://stub.local; restore the previous behavior by calling
IConfigurationBuilder.AddEnvironmentVariables() before or along with the
in-memory collection and then branch on the TEST_REAL_API env var like
ClientTestFixture does: when TEST_REAL_API is not true, register HttpHandlerStub
in the DI setup and call ConfigurePrimaryHttpMessageHandler to wire the stubbed
handler into the HttpClient used by the resolved client; alternatively ensure
ResolvedClientActuallyWorks explicitly registers HttpHandlerStub +
ConfigurePrimaryHttpMessageHandler so no real network call is made.
1 participant
Summary
Framework upgrade from
net7.0(out of support) tonet10.0(current), aligning package versions with the new runtime.Changes
TargetFrameworkbumped tonet10.0in library and testsMicrosoft.Extensions.*packages aligned at10.0.6dotnet-version: 10.0.xPackageLicenseUrlreplaced withPackageLicenseExpression(AGPL-3.0-only)Operator<T>.GetQueryString(Value.ToString()→Value?.ToString())Test plan
-warnaserrorTEST_REAL_APIenv var is set (unchanged code path)Summary by CodeRabbit
Release Notes
Chores
Bug Fixes