[CI Visibility] Fix coverage rewrites with shared framework dependencies - #9102
Open
tonyredondo wants to merge 5 commits into
Open
[CI Visibility] Fix coverage rewrites with shared framework dependencies#9102tonyredondo wants to merge 5 commits into
tonyredondo wants to merge 5 commits into
Conversation
BenchmarksBenchmark execution time: 2026-08-24 12:22:08 Comparing candidate commit fe1c0b0 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 62 known flaky benchmarks, 64 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9102) and master. ✅ No regressions detected |
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ab68d9e57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+487
to
+490
| .Where(candidate => candidate.Parsed && | ||
| candidate.Version.Major == requestedVersion.Major && | ||
| candidate.Version.Minor == requestedVersion.Minor && | ||
| candidate.Version >= requestedVersion) |
There was a problem hiding this comment.
When a test application relies on a permissive runtime policy such as rollForward: Major or DOTNET_ROLL_FORWARD=Major, the host can successfully run an 8.0 runtimeconfig using an installed 9.0 shared framework. This filter nevertheless requires the installed framework to have exactly the requested major and minor, so the collector cannot find dependencies that the running application is actually loading and the coverage rewrite still fails. Framework selection needs to account for the effective roll-forward policy instead of hard-coding same-major/same-minor compatibility.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
This is a valid broader edge case, but I am keeping it outside the scope of this fix. The resolver here follows the Coverlet approach referenced by issue 9023 and targets same-major/minor patch resolution. Correct major roll-forward support must account for the effective host policy from runtimeconfig settings, per-framework overrides, environment variables, command-line options, and prerelease behavior. Simply probing a higher major could select a framework the host would not use. I am leaving this thread unresolved rather than treating that partial behavior as fixed.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8178eddf8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe1c0b0bef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tonyredondo
marked this pull request as ready for review
August 24, 2026 13:19
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe1c0b0bef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
1 participant
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.
Summary of changes
Reason for change
The coverage collector could not resolve assemblies that were available only through an installed shared framework, such as
Microsoft.Extensions.Logging.AbstractionsfromMicrosoft.AspNetCore.App. If Mono.Cecil encountered that dependency while writing metadata, it had already truncated the assembly under test and left a damaged DLL on disk.Implementation details
framework,frameworks, andincludedFrameworksentries from sibling*.runtimeconfig.jsonfilesSystem.Private.CoreLiband select the highest compatible patch version using full semantic-version orderingFile.Replacebackups and rollbackTest coverage
CoverageResolverTestson net10.0 and net8.0CoverageRewriteTestson net10.0Datadog.Trace.Coverage.collectorproject buildMicrosoft.AspNetCore.Appand an optionalLogLevelparameterOther details
Windows and Linux CI provide the remaining platform coverage for replacement and locking semantics.
Fixes #9023