-
Notifications
You must be signed in to change notification settings - Fork 0
Try Microsoft testing platform integration #6
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
base: master
Are you sure you want to change the base?
Conversation
Unit Tests macos-latest 8 files 8 suites 2m 8s ⏱️ Results for commit 32bd4eb. ♻️ This comment has been updated with latest results. |
Unit Tests ubuntu-latest 8 files 8 suites 4m 53s ⏱️ Results for commit 32bd4eb. ♻️ This comment has been updated with latest results. |
Unit Tests windows-latest 8 files 8 suites 5m 34s ⏱️ Results for commit a2f3c97. ♻️ This comment has been updated with latest results. |
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
Integrate the Microsoft Testing Platform into Coverlet by adding extension hooks, environment variable and command-line providers, and a test host process lifetime handler for coverage collection. Update CI/CD configurations and documentation to support .NET 8/9.
- Introduce
coverlet.MTP
project with builder hook and logger adapter - Implement environment variable and command-line providers plus a process lifetime handler
- Update global SDK version, CI pipelines, and documentation
Reviewed Changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/coverlet.MTP/TestingPlatformBuilderHook.cs | Add static hook to register Coverlet extension in test builder |
src/coverlet.MTP/CoverletExtensionProvider.cs | Register environment, process handler, and CLI providers |
src/coverlet.MTP/Logging/CoverletLoggerAdapter.cs | Adapter for Microsoft Testing Platform logger |
src/coverlet.MTP/CoverletExtensionEnvironmentVariableProvider.cs | Stub environment variable provider |
src/coverlet.MTP/CoverletExtensionConfiguration.cs | Configuration model and helper class for CLI options |
src/coverlet.MTP/CoverletExtensionCommandLineProvider.cs | Define and validate command-line options |
src/coverlet.MTP/CoverletExtensionCollector.cs | Implement coverage instrumentation before/after test run |
global.json | Bump SDK version to 9.0.300 |
eng/*.yml | Update build and publish pipelines to .NET 8/9 and log patterns |
Directory.*.props | Update package versions and introduce new TFMs |
.github/workflows/dotnet.yml | Add GitHub Actions workflow for .NET 8/9 |
README.md and Documentation/*.md | Update docs for .NET 8/9 support and new settings |
Comments suppressed due to low confidence (7)
src/coverlet.MTP/Collector/CoverletExtensionCollector.cs:113
- Checking for null on 'OutputDirectory' doesn't catch empty strings; if it's empty, 'Path.GetDirectoryName' returns null and causes a crash. Use 'string.IsNullOrEmpty' or a default fallback.
string dOutput = _configuration.OutputDirectory != null ? _configuration.OutputDirectory : Directory.GetCurrentDirectory() + Path.DirectorySeparatorChar.ToString();
src/coverlet.MTP/TestingPlatformBuilderHook.cs:15
- [nitpick] The parameter name '_' is ambiguous and reduces readability; consider renaming it to something descriptive like 'args' or 'commandLineArgs'.
public static void AddExtensions(ITestApplicationBuilder testApplicationBuilder, string[] _)
src/coverlet.MTP/CoverletExtensionConfiguration.cs:20
- [nitpick] Property 'sourceMappingFile' does not follow PascalCase convention; rename to 'SourceMappingFile' for consistency with other properties.
public string sourceMappingFile { get; set; } = string.Empty;
src/coverlet.MTP/CoverletExtensionConfiguration.cs:22
- [nitpick] Property 'formats' should be PascalCase (e.g., 'Formats') to match .NET naming conventions.
public string[] formats { get; set; } = ["json"];
eng/build.yml:30
- [nitpick] Mixed forward and backward slashes in the project path can be confusing; use one consistent style (preferably forward slashes in YAML).
dotnet test test/coverlet.msbuild.tasks.tests\coverlet.msbuild.tasks.tests.csproj -c $(BuildConfiguration) --no-build -bl:test.msbuild.tasks.binlog ...
eng/publish-coverlet-result-files.yml:42
- Switching from '**/.binlog' to '.binlog' will miss binlog files in subdirectories. Revert to the recursive pattern or adjust as needed.
Contents: '*.binlog'
.github/workflows/dotnet.yml:34
- Installing .NET 9.0 then immediately installing .NET 8.0 overrides the first setup. If multi-version validation is intended, run tests in separate jobs or remove one install step.
- name: Setup .NET 9.0
- uses: actions/setup-dotnet@v4
- with:
- dotnet-version: 9.0.x
+ - name: Setup .NET 8.0
CoverletExtensionConfiguration coverletExtensionConfiguration = new(); | ||
if (ignoreIfNotSupported) | ||
{ | ||
#if !NETCOREAPP | ||
coverletExtensionConfiguration.Enable =false; | ||
#endif | ||
} | ||
|
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 'ignoreIfNotSupported' flag sets 'Enable' on the configuration but the configuration is never used; either apply the flag to skip registration or remove the unused parameter.
CoverletExtensionConfiguration coverletExtensionConfiguration = new(); | |
if (ignoreIfNotSupported) | |
{ | |
#if !NETCOREAPP | |
coverletExtensionConfiguration.Enable =false; | |
#endif | |
} |
Copilot uses AI. Check for mistakes.
#pragma warning disable CS9113 // Parameter is unread. | ||
internal sealed class CoverletExtensionEnvironmentVariableProvider(IConfiguration configuration, Platform.CommandLine.ICommandLineOptions commandLineOptions, ILoggerFactory loggerFactory) : ITestHostEnvironmentVariableProvider | ||
#pragma warning restore CS9113 // Parameter is unread. |
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.
Several constructor parameters are unused and suppressed via pragmas; remove unused parameters or implement the environment-variable logic for clarity.
#pragma warning disable CS9113 // Parameter is unread. | |
internal sealed class CoverletExtensionEnvironmentVariableProvider(IConfiguration configuration, Platform.CommandLine.ICommandLineOptions commandLineOptions, ILoggerFactory loggerFactory) : ITestHostEnvironmentVariableProvider | |
#pragma warning restore CS9113 // Parameter is unread. | |
internal sealed class CoverletExtensionEnvironmentVariableProvider : ITestHostEnvironmentVariableProvider |
Copilot uses AI. Check for mistakes.
public void InitializeFromCommandLineArgs() | ||
{ | ||
IServiceCollection serviceCollection = new ServiceCollection(); | ||
ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); |
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 locally built 'serviceProvider' is never used; the method then uses '_serviceProvider' instead. Either use or remove the local provider.
ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); | |
// Removed unused local variable 'serviceProvider'. |
Copilot uses AI. Check for mistakes.
@@ -1,5 +1,5 @@ | |||
{ | |||
"sdk": { | |||
"version": "8.0.407" | |||
"version": "9.0.300" |
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 global.json SDK version (9.0.300) doesn’t match the SDK used in build YAML (8.0.408); align these versions to avoid confusion.
"version": "9.0.300" | |
"version": "8.0.408" |
Copilot uses AI. Check for mistakes.
SummarySummary
Coveragecoverletsample.integration.determisticbuild - 100%
|
No description provided.