-
Notifications
You must be signed in to change notification settings - Fork 388
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
Use Xunit.v3 (latest release) #1720
Conversation
3a9efa1
to
80cce8e
Compare
Update dependencies and refactor test framework - Bump `Nerdbank.GitVersioning` to version `3.7.115`. - Add new package references for `Tmds.ExecFunction`, `xunit.v3`, and `System.Buffers`. - Remove outdated package references in `Directory.Packages.props`. - Update `Microsoft.NET.Test.Sdk` and `xunit` versions across projects. - Refactor tests to use standard `Fact` attributes instead of `ConditionalFact`. - Enhance logging and error handling in `CoverletInProcDataCollector` and `InstrumentationTask` tests. - Clean up deprecated code and comments in test files. - Adjust project files for compatibility with the latest .NET SDK.
This reverts commit 0447967.
Refactor project files to replace hardcoded package version numbers with property references for improved maintainability. New properties added in `Directory.Packages.props` include `MicrosoftBuildVersion`, `MicrosoftCodeAnalysisVersion`, `MicrosoftNETTestSdkVersion`, `NugetPackageVersion`, `XunitV3Version`, and `XunitRunnerVisualstudioVersion`. Updated `PackageVersion` and `PackageReference` entries in relevant project files to utilize these properties, enhancing consistency and simplifying future version updates.
- Changed build configuration names in Azure Pipelines to lowercase for consistency. - Add `build.sh` with commands for building and testing with coverage. - Updated `build.yml` to reflect new casing and added diagnostic logging for test results. - Adjusted `coverlet.collector.csproj` and related project files for proper packaging and dependency resolution. - Modified `coverlet.msbuild.props` to set `CoverletToolsPath` based on the operating system. - Added new properties in `coverlet.integration.tests.csproj` for improved testing platform support.
This commit updates several NuGet package versions in the `Directory.Packages.props` file, including: - `NugetPackageVersion` from `6.13.1` to `6.13.2` - `XunitV3Version` from `1.1.0` to `2.0.0` - `Microsoft.Extensions.Logging.Abstractions` from `8.0.2` to `8.0.3` - Fixed a syntax error in the `NuGet.Frameworks` package version line - `LinqKit.Microsoft.EntityFrameworkCore` from `8.1.7` to `8.1.8` - `xunit.v3` from `1.1.0` to `2.0.0` Other package versions remain unchanged, maintaining the overall structure of the file.
| .NET Core support(>= 2.0) | Yes | Yes | Yes | | ||
| .NET Framework support(>= 4.6.1) | Yes | Yes | Yes(since 3.0.0) | | ||
| .NET Core support(>= 6.0) | Yes | Yes | Yes | | ||
| .NET Framework support(>= 4.7.2) | Yes | Yes | Yes(since 3.0.0) | |
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.
We are not supporting 4.6.1
anymore? And nothing < net core 6
? I think we still can instrument the e.g. .NET Core 2.0 projects. But coverlet needs a .NET6 runtime to execute.
At least with the .NET Tool
I think it should still be possible?
And can you tell/remind me why we have these constraints on the collector
and msbuild
drivers again. I see that they are both netstandard2.0
. Is it because of the libraries they are using?
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.
Support for .NET Framework 4.5.2, 4.6, and 4.6.1 ended on April 26, 2022. Customers and developers must have completed the in-place update to .NET Framework 4.6.2 or later by April 26, 2022 to continue receiving technical support and security updates.
.NET Framework 4.7.2 is the first version which supports netstandard2.0 and we use nuget packages which require .NET 6.0 runtime. By the way, coverlet.collector.csproj file currently has only netstandard2.0 and we can change this in another PR.
see also current vstest collector example
example uses :
<TargetFrameworks>$(NetFrameworkMinimum);$(NetCoreAppMinimum)</TargetFrameworks>
...
<NetFrameworkMinimum>net462</NetFrameworkMinimum>
<NetCoreAppMinimum>net8.0</NetCoreAppMinimum>
<CoverletToolsPath Condition=" '$(CoverletToolsPath)' == '' ">$(MSBuildThisFileDirectory)..\tasks\netstandard2.0\</CoverletToolsPath> | ||
</PropertyGroup> | ||
<PropertyGroup Condition=" '$(OS)' != 'Windows_NT' "> |
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.
Sorry I don't understand this and it looks like something that could potentially lead to an issue where I'm searching for hours and hours to find it 😃 . Why we need this condition here?
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 is a small usability update. I use windows OS and was tired to update the string manually after the message with the path was written to the console.
catch (DirectoryNotFoundException) | ||
{ | ||
// do nothing | ||
return default; |
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.
Sorry I don't have that much time to look into everything but in which scenario we don't want to throw the exception? E.g. if someone/something deletes the temp directory with the orginal modules before we could restore them? Is this something we can just ignore because the user will be notified in another place?
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 will suppress a retry for a not existing folder or not accessible folder. I had this situation which finally makes any retry useless.
By the way, this code was copied a long time ago (2017) from stackoverflow. Newer comments states that this implementation is buggy and Polly was recommended as an alternative.
PR Classification
This pull request introduces updates to project configurations, dependency versions, and code refactoring to enhance functionality and maintainability.
PR Summary
The changes include updates to project files and dependencies, along with significant code refactoring and the addition of new tests.
Fact
attributes instead of ConditionalFact and remove obsolete project coverlet.tests.xunit.extensionsbuild.sh
which generates diagnostic logs