-
Notifications
You must be signed in to change notification settings - Fork 353
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
Change Object mode for package testing & remove older frameworks & rids #7510
Conversation
Anipik
commented
Jun 10, 2021
- use nuget content model for package object similar to package validation.
- remove out of support frameworks.
- remove rids metadata as we no longer need.
- modify tests
src/Microsoft.DotNet.PackageTesting/GetCompatiblePackageTargetFrameworks.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.PackageTesting/GetCompatiblePackageTargetFrameworks.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.PackageTesting/GetCompatiblePackageTargetFrameworks.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.PackageTesting/GetCompatiblePackageTargetFrameworks.cs
Outdated
Show resolved
Hide resolved
…ll supported frameworks and add min dotnet version for packages contatining netstandard config
Cool. You are missing the props file which defines the current set of tfms:
|
shouldnt this be set in the runtime repo ? so that when the tfms go out of support we can just change the property in the runtime repo. |
Projects that live in arcade are meant to be consumed by multiple repositories. Moving the current set of supported TFMs into a props file a) makes it easier for multiple repositories to use the package and b) allows consumers to mutate that list. Imagine a repository like dotnet/runtime that versions to the next version of .NETCoreApp and wants to do that independently of the current set of exposed supported TFMs in this package. In example dotnet/runtime versions from .NET 6 to .NET 7 and can add this code block temporarily to avoid a change in the package itself:
At a later point these changes will then flow back into this package to keep the defined TFMs consistent with the .NET support policy. |
@@ -3,4 +3,8 @@ | |||
<DotNetPackageTestingAssembly Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\tools\net472\Microsoft.DotNet.PackageTesting.dll</DotNetPackageTestingAssembly> | |||
<DotNetPackageTestingAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\netcoreapp3.1\Microsoft.DotNet.PackageTesting.dll</DotNetPackageTestingAssembly> | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<SupportedTestFramework Include="netcoreapp3.1;net5.0;net6.0;net461;net462;net471;net472;net48;netstandard2.0;netstandard2.1" /> |
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.
SupportedTargetFramework is currently set by the sdk and it contains all the frameworks so i defined it as SupportedTestFramework
src/Microsoft.DotNet.PackageTesting/build/Microsoft.DotNet.PackageTesting.props
Outdated
Show resolved
Hide resolved
@@ -3,4 +3,8 @@ | |||
<DotNetPackageTestingAssembly Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\tools\net472\Microsoft.DotNet.PackageTesting.dll</DotNetPackageTestingAssembly> | |||
<DotNetPackageTestingAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\tools\netcoreapp3.1\Microsoft.DotNet.PackageTesting.dll</DotNetPackageTestingAssembly> | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<SupportedTestFramework Include="netcoreapp3.1;net5.0;net6.0;net461;net462;net471;net472;net48;netstandard2.0;netstandard2.1" /> |
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.
I believe you are missing test coverage in the test assembly as when I tried to add net6.0 last time, the NuGet Parse mechanism blew up. Can you please add a test that passes these items in?
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 tests contains
and tested with runtime repo as well.
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.
Thanks a lot 👍