coverlet-coverage / coverlet

Cross platform code coverage for .NET
MIT License
2.93k stars 385 forks source link

[BUG] 6.0.1 requires dotnetsdk 8 but doesn't say that #1625

Closed drdamour closed 3 months ago

drdamour commented 4 months ago

Describe the bug Had a build tied to 6.* for a version using sdk 6 and it failed with

Data collector 'XPlat code coverage' message: [coverlet]System.TypeLoadException: Could not load type 'Microsoft.Extensions.DependencyInjection.ServiceCollection' from assembly 'Microsoft.Extensions.DependencyInjection.Abstractions, Version=2.2.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.GetDefaultServiceCollection(TestPlatformEqtTrace eqtTrace, TestPlatformLogger logger, String testModule)
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.OnSessionStart(Object sender, SessionStartEventArgs sessionStartEventArgs) in /_/src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs:line 135.

To Reproduce build with dotnet6

Expected behavior Expect coverlet to run and report successfully

Actual behavior fails to generate report

Configuration (please complete the following information): Please provide more information on your .NET configuration:

Additional context the changes in this PR https://github.com/coverlet-coverage/coverlet/pull/1601 seems to force having dotnet 8 installed so that thinks like System.Text.Json will load the 8.0 version from the shared framework.

:exclamation: Please also read Known Issues

doesn't appear to be any

overall, seems like mistake to break backwards compatability between 6.0.0 and 6.0.1 like this...doesn't seem to match semver...but maybe that was your intent? understand 6 is EOL at end of year so we should be moving to 8 sdk soon...

Bertk commented 4 months ago

Thank you for reporting.

I used the coverlet HelloWord example and the test was executed successfully.

image

The .NET SDK 8.0 was installed.

SeWieland commented 4 months ago

Same problem here.. All our CI pipelines are failing because we are adding coverlet.collector dynamically to our test solutions via dotnet add package.

@Bertk the hello world example still uses PackageReference Include="coverlet.collector" Version="6.0.0" > => Version 6.0.0

dotnet add package will install version 6.0.1 (a patch update that must not contain breaking changes according to SemVer) and this package does not include any dependency to system.text.json at all.

andreycha commented 4 months ago

Have same issue. We're using .NET 6 SDK and have coverlet.collector referenced as 6.0.*. Having such a breaking change in a patch version is quite a surprise. Changes described in https://github.com/coverlet-coverage/coverlet/pull/1626 require at least new minor version.

Bertk commented 4 months ago

Sorry, I missed the to update the version.

@MarcoRossignoli @daveMueller We should update the version to 6.1.0.

SeWieland commented 4 months ago

Another idea: "SuppressDependenciesWhenPacking" is set on the collector - effectively hiding all dependencies. Would it be possible to pack these two required package references as dependencies instead or something similar? The breaking behaviour could thus probably be reverted for .NET < 8

andreycha commented 4 months ago

@Bertk would it be also possible to release 6.0.2 with reverted changes?

Bertk commented 4 months ago

@SeWieland Thank you for the hint but coverlet.collector package has no dependencies. The dilemma is the support for framework net6.0, net7.0 and net8.0 and this works with the latest packages for

    <PackageReference Include="System.Reflection.Metadata" VersionOverride="8.0.0" />
    <PackageReference Include="System.Collections.Immutable" VersionOverride="8.0.0" />
image
MarcoRossignoli commented 4 months ago

@MarcoRossignoli @daveMueller We should update the version to 6.1.0.

Why? we bump the patch we will bump the minor if we ship a new feature and a major if we do breaking change.

Bertk commented 4 months ago

@MarcoRossignoli coverlet 6.0.1 is a bugfix version but requires a .NET 8.0 SDK.

semver 2.0 has this paragraph

What should I do if I update my own dependencies without changing the public API?

That would be considered compatible since it does not affect the public API. Software that explicitly depends on the same dependencies as your package should have their own dependency specifications and the author will notice any conflicts. Determining whether the change is a patch level or minor level modification depends on whether you updated your dependencies in order to fix a bug or introduce new functionality. We would usually expect additional code for the latter instance, in which case it’s obviously a minor level increment.

MarcoRossignoli commented 4 months ago

but requires a .NET 8.0 SDK.

Why? we ship for

image

so we're good for .NET Framework and every project with tfm >= 6.0

SeWieland commented 4 months ago

@SeWieland Thank you for the hint but coverlet.collector package has no dependencies. The dilemma is the support for framework net6.0, net7.0 and net8.0 and this works with the latest packages for

    <PackageReference Include="System.Reflection.Metadata" VersionOverride="8.0.0" />
    <PackageReference Include="System.Collections.Immutable" VersionOverride="8.0.0" />
image

IMHO if there is support for .NET 6.0 it now has dependencies. If I have to pin other packages to a specific version for coverlet to be compatible, that package is a depdendency... Therefore, right now my workaround is to pin coverlet to v6.0.0 in all .NET 6 projects.

drdamour commented 4 months ago

but requires a .NET 8.0 SDK.

Why? we ship for image

so we're good for .NET Framework and every project with tfm >= 6.0

Doesnt matter what u target or if u declare a dependency or not...u now require system.text.json 8 in 6.0.1…when u used to NOT..just cause u do not claim a dependendency doesnt mean u dont have one.

in fact by not claiming a dependency u seem to have..u’ve made yourself backwards incompatible.

So let me be clearer..i think its a mistake to require json 8 and think you should revert that change back to (i think it was 3.1) unless u require some functionality in json 8. Thats wouldnbe the better solution IMO.

If u MUST use json 8 you should bump a major version since users bow need it to implicitly be present or declare that dependency.

i too must pin to 6.0.0 when using sdk 6

MarcoRossignoli commented 4 months ago

u now require system.text.json 8 in 6.0.1

I saw now what you mean https://github.com/coverlet-coverage/coverlet/pull/1601/files#diff-1d12ae6a5243ca16cfc9b56abc8423c15a917dd1adcdc3d7dd3482ccff2208da this was wrong... tfm is 6.0 https://github.com/coverlet-coverage/coverlet/pull/1601/files#diff-1d12ae6a5243ca16cfc9b56abc8423c15a917dd1adcdc3d7dd3482ccff2208daL5

@Bertk @daveMueller we cannot declare >= 6.0 and insert non 6 lib...this needs to be fixed.

tvbishan commented 4 months ago

I have the same issue. Works well with the coverlet.collector Version="6.0.0". But when upgraded to the coverlet.collector Version="6.0.1", it failed.

Data collector 'XPlat code coverage' message: [coverlet]System.IO.FileLoadException: Could not load file or assembly 'System.Text.Json, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. Could not find or load a specific file. (0x80131621)
File name: 'System.Text.Json, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'
cremor commented 4 months ago

I suggest to release 6.0.2 with that fix ASAP.

kev24uk commented 4 months ago

Just experienced the same issue in one of our builds where it is unable to find System.Text.Json, Version=8.0.0.0 - downgrading to 6.0.0 fixed it for now.

akutuev commented 4 months ago

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

MarcoRossignoli commented 4 months ago

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

It's fixed here https://github.com/coverlet-coverage/coverlet/pull/1628 cc: @daveMueller

akutuev commented 4 months ago

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

It's fixed here #1628 cc: @daveMueller

Ah I see, thanks for heads up! Hope NuGet repo will be updated soon

drdamour commented 4 months ago

hope we see a release soon

JeroenDeHaan1980 commented 4 months ago

When do you expect to have a new version released with the issue solved?

daveMueller commented 3 months ago

Hi guys, we finally merged everything we broke with the last release. We would really appreciate if someone could give it a try. Here is how to consume the nightly: https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md

tvbishan commented 3 months ago

@daveMueller

It is working now. Tested with image: mcr.microsoft.com/dotnet/sdk:6.0 and coverlet.collector --version 6.0.2-preview.8.g892d86e16b

daveMueller commented 3 months ago

We now have a new official release 6.0.2 that can be consumed from nuget.org.