Fix Grpc.Net.ClientFactory version range check - Fixes #7565#7566
Open
Ghost93 wants to merge 6 commits into
Open
Fix Grpc.Net.ClientFactory version range check - Fixes #7565#7566Ghost93 wants to merge 6 commits into
Ghost93 wants to merge 6 commits into
Conversation
Author
|
@dotnet-policy-service agree company="Microsoft" |
Ghost93
force-pushed
the
fix/grpc-net-clientfactory-version-range
branch
from
a2c32db to
7895395
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds resilience build-transitive validation for Grpc.Net.ClientFactory versions and tests to ensure version parsing no longer fails on non-System.Version NuGet version formats.
Changes:
- Parse comparable versions from
PackageReference/PackageVersion/ transitiveReferencePathusing a regex to avoidMSB4184when versions are ranges or include prerelease labels. - Emit warnings based on the parsed comparable version instead of the raw NuGet version string.
- Add integration-style tests that run
dotnet msbuildagainst a temporary project to verify warning/no-warning behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/BuildTransitive/GrpcNetClientFactoryVersionTargetTests.cs | New tests that execute the MSBuild target and assert warnings are/aren’t emitted for various version sources and formats. |
| src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets | Updates the target to extract a comparable numeric version via regex before calling MSBuild::VersionLessThan. |
| using FluentAssertions; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Extensions.Http.Resilience.Test.BuildTransitive; |
Author
There was a problem hiding this comment.
Thanks. I kept this as Microsoft.Extensions.Http.Resilience.Test.BuildTransitive because this test project explicitly sets:
<RootNamespace>Microsoft.Extensions.Http.Resilience.Test</RootNamespace>
and the existing tests consistently use Microsoft.Extensions.Http.Resilience.Test.* namespaces. So this new namespace follows the prevailing project convention rather than the folder name.
Author
|
Update on the remaining The same issue is being tracked/fixed separately in #7561: This PR’s WarningsCheck issue from the test helper has been addressed in |
Ghost93
force-pushed
the
fix/grpc-net-clientfactory-version-range
branch
from
e8fa206 to
892d086
Compare
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1464903&view=codecoverage-tab |
Ghost93
force-pushed
the
fix/grpc-net-clientfactory-version-range
branch
from
892d086 to
911bf59
Compare
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1465046&view=codecoverage-tab |
Ghost93
force-pushed
the
fix/grpc-net-clientfactory-version-range
branch
from
911bf59 to
0c487a8
Compare
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1465182&view=codecoverage-tab |
Normalize NuGet version range metadata before calling VersionLessThan in the Http.Resilience buildTransitive target. Add regression coverage for bracket-pinned versions across all target inputs. Fixes dotnet#7565 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid letting temporary directory cleanup failures mask the original test assertion or process failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Kill the process tree where supported, wait for termination, and observe output reader tasks before reporting a timeout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prefer ProcessStartInfo.ArgumentList on modern target frameworks while keeping the quoted Arguments fallback for net462. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explain the comparable version regex inputs so future changes preserve the MSBuild VersionLessThan guard behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid empty catch blocks while preserving best-effort cleanup and output task observation behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ghost93
force-pushed
the
fix/grpc-net-clientfactory-version-range
branch
from
0c487a8 to
4c7e573
Compare
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1465941&view=codecoverage-tab |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7565
Summary
Grpc.Net.ClientFactoryNuGet version metadata before callingVersionLessThanin theMicrosoft.Extensions.Http.ResiliencebuildTransitive target.[2.80.0]without failing the build withMSB4184.PackageReference.VersionPackageReference.VersionOverridePackageVersion.VersionReferencePath.NuGetPackageVersionValidation
./restore.sh./.dotnet/dotnet test test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Microsoft.Extensions.Http.Resilience.Tests.csproj --filter GrpcNetClientFactoryVersionTargetTests --no-restore -v:minimal./build.sh --vs Polly,Resilience./build.sh --build --testMicrosoft Reviewers: Open in CodeFlow