Skip to content
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

.NET 8: Make Microsoft.NET.Test.Sdk an implicit reference #25797

Open
richlander opened this issue Jun 2, 2022 · 37 comments
Open

.NET 8: Make Microsoft.NET.Test.Sdk an implicit reference #25797

richlander opened this issue Jun 2, 2022 · 37 comments
Assignees
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@richlander
Copy link
Member

The Microsoft.NET.Test.Sdk PackageReference is a pain w/o obvious value. Presumably, everyone wants to have the matching PackageReference for their .NET SDK and/or VS version. That's not the default experience due to the PackageReference.

As a symptom of that, this package is one that dependabot has to update most. Example: dotnet/iot#1556.

It would be great if we could (somehow) make packages we update monthly as implicit PackageReferences.

There is no design here (and I haven't thought about it). This is just a UX pain.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Jun 2, 2022
@baronfel
Copy link
Member

baronfel commented Jun 2, 2022

Definitely in favor of this - a user adding <IsTestProject>true<IsTestProject> to a project should be enough information to add this PackageReference if it doesn't exist already. We would need testing to ensure that we only ever add references to Package Versions that actually exist, though.

@richlander
Copy link
Member Author

Here's the relevant part of a test project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />

Ideally, we could just switch the first line to do what you said:

<Project Sdk="Microsoft.NET.Test.Sdk">

Right?

@WeihanLi
Copy link
Contributor

WeihanLi commented Jun 9, 2022

Like it, the sdk name maybe should be Microsoft.NET.Sdk.Test? Just likes Microsoft.NET.Sdk.Web
Guess this would fix this issue #3790

@nohwnd
Copy link
Member

nohwnd commented Jun 9, 2022

richlander: Presumably, everyone wants to have the matching PackageReference for their .NET SDK and/or VS version. That's not the default experience due to the PackageReference.

baronfel: add this PackageReference if it doesn't exist already.

Will this allow us to upgrade the package version as needed? We want to ship .net framework testhost in the TestHost package that is a dependency of Microsoft.NET.Test.Sdk, so MSTest and other adapters can start start depending on Microsoft.NET.Test.Sdk directly to get the correct compatible version for them. And if dependebot upgrades them to latest, they might need latest Microsoft.NET.Test.Sdk as well.

So if the proposed change forces a recent version appropriate for the current sdk, or newer, then it is beneficial for us, because we won't have to fight with very outdated version of Microsoft.NET.Test.Sdk.

<IsTestProject>true<IsTestProject>

This should not need to be defined, but from the linked issue I see that this is sometimes needed. This is probably unrelated issue that we (test platform) should solve. Correctly user should not be required to define this. Unless they define <IsTestProject>false</IsTestProject> to force skipping the project.

@nohwnd
Copy link
Member

nohwnd commented Jun 9, 2022

Is your plan to make this change in our test-templates for all projects, or am I misunderstanding, and this is only dotnet/* repositories specific proposal?

@richlander
Copy link
Member Author

The proposal is to define a pattern that would work for everyone. We'd make that change in templates and existing projects would need to implement the pattern to get the new behavior. I don't see a good or desired way to retroactively force it on old projects.

@richlander richlander changed the title Make Microsoft.NET.Test.Sdk an implicit reference .NET 8: Make Microsoft.NET.Test.Sdk an implicit reference Jun 25, 2022
@Nirmal4G
Copy link
Contributor

Nirmal4G commented Oct 4, 2022

Out of all features proposed for .NET 8, this would be the most useful and underrated feature ever. It would merely take an hour or so to move, rework the .NET Test SDK into dotnet/sdk repo. This could've been done in .NET 5. I don't understand why the team didn't and dragged this on till v8!?

@paulomorgado
Copy link

I just did dotnet new mstest and got this csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <RootNamespace>dotnet_issues_25797</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.10" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.10" />
    <PackageReference Include="coverlet.collector" Version="3.1.2" />
  </ItemGroup>

</Project>

Changed it to:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <RootNamespace>dotnet_issues_25797</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MSBuildVersion)" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.10" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.10" />
    <PackageReference Include="coverlet.collector" Version="3.1.2" />
  </ItemGroup>

</Project>

Now, depending on the version of MSBuild on the SDK being used, I get a different version of Microsoft.NET.Test.Sdk.

@paulomorgado
Copy link

I just did dotnet new mstest and got this csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <RootNamespace>dotnet_issues_25797</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.10" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.10" />
    <PackageReference Include="coverlet.collector" Version="3.1.2" />
  </ItemGroup>

</Project>

Changed it to:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <RootNamespace>dotnet_issues_25797</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MSBuildVersion)" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.10" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.10" />
    <PackageReference Include="coverlet.collector" Version="3.1.2" />
  </ItemGroup>

</Project>

Now, depending on the SDK, I get a different version of

@richlander
Copy link
Member Author

@baronfel -- Is this a scenario we can consider? It seems like multiple folks would appreciate making this easier.

@Nirmal4G
Copy link
Contributor

Better make this as an SDK instead of package reference.

@richlander
Copy link
Member Author

richlander commented Jul 19, 2023

I tried to set Microsoft.NET.Test.Sdk via Central Package Management. I was not able to define just it.

If I set ManagePackageVersionsCentrally to true, then I get this error.

/Users/rich/apps/tests/tests.csproj : error NU1008: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: xunit;xunit.runner.visualstudio;coverlet.collector. [/Users/rich/apps/tests/tests.sln]

If I don't have ManagePackageVersionsCentrally at all but do the following:

In Directory.Packages.props:

<Project>
  <ItemGroup>
    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.6.1" />
  </ItemGroup>
</Project>

In tests.csproj

<PackageReference Include="Microsoft.NET.Test.Sdk" />

Then SDK falls back to a specific min version.

$ dotnet build               
MSBuild version 17.7.0+5785ed5c2 for .NET
  Determining projects to restore...
/Users/rich/apps/tests/tests.csproj : warning NU1604: Project dependency Microsoft.NET.Test.Sdk does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results. [/Users/rich/apps/tests/tests.sln]
$ cat bin/Debug/net8.0/tests.deps.json | grep Microsoft.NET.Test.Sdk
          "Microsoft.NET.Test.Sdk": "15.0.0",
      "Microsoft.NET.Test.Sdk/15.0.0": {
    "Microsoft.NET.Test.Sdk/15.0.0": {

It would be really nice if Central Package Versioning could work for this package. Is there a "progressively adopt CPM pattern"? I didn't see one documented.

@nohwnd
Copy link
Member

nohwnd commented Jul 25, 2023

Presumably, everyone wants to have the matching PackageReference for their .NET SDK and/or VS version.

I am not sure about this, I don't think this would work well with our current extension model, and I am not sure how other teams (like msbuild) handle this.

Let's pretend we already did this change in net7, and Microsoft.NET.Test.SDK became a built in SDK.

We then have a projects where DependaBot pushes MSTest (or Xunit) and other dependencies to the latest.

MSTest relies on Microsoft.TestPlatform.ObjectModel which is our extensibility model for TestPlatform. ObjectModel is shipped with testhost, so with the SDK approach it would always have the version that is built in the SDK.

So if there is net7 with 17.6 ObjectModel, and then net8 with 17.7 ObjectModel, then MSTest either:

  • has to stick with 17.6, and never use new features of 17.7 (<- this is what xunit and nunit does)
  • release package specific to 17.6, and 17.7 through some version tricks like 3.xxx -> 17.6, 4.xxx -> 17.7
  • release just 1 version, and users must limit Dependabot to not upgrade them above a certain version when they use net7. They also won't get any new fixes.

Currently this is not a concern for us because Test.SDK is free to update to latest version, so the extensions can load because they are compatible on binary level. And testhost then stays backwards compatible with vstest.console on protocol level, where they can handshake the correct version to use and the set of capabilities they both support.

Theoretically we could state our dependencies in the <Project SDK= so we get the latest ObjectModel that we, or any of the extensions needs via nuget. But then our code in testhost has to assume that it could be getting a newer version of ObjectModel, which really limits how we can evolve ObjectModel.

@richlander
Copy link
Member Author

I'm missing something. Today, the version of this reference is (nearly) totally arbitrary. The only certainty you have is a lower bound and nothing more. I am merely proposing more certainty within the same scope that the reference can be today. I do not understand how this affects your code since the versioning scheme I'm suggesting already exists as one possibility. In fact, if you strictly merge Dependabot PRs, you'd get something very similar to what I'm suggesting.

@nohwnd
Copy link
Member

nohwnd commented Jul 26, 2023

Maybe it is me who is missing something, because I don't know how being an SDK behaves exactly. 🙂

Looking for example at Razor.SDK here: "C:\Program Files\dotnet\sdk\7.0.306\Sdks\Microsoft.NET.Sdk.Razor\tools\Newtonsoft.Json.dll"

Contents of our Microsoft.NET.Test.SDK nuget package would be placed in that folder (along with targets etc, but lets not complicate this):

C:\Program Files\dotnet\sdk\7.0.306\Sdks\Microsoft.NET.Sdk.Test\tools\Microsoft.TestPlatform.ObjectModel.dll"

This particular dll is our extension model and in 7.0.306 it would have version 17.6.0. The testhost.dll (that would be in the same folder) is also 17.6.0 and is compatible with the ObjectModel 17.6.0.

User installs MSTest Nuget package in version 3.2.0, which also requires a minimum version of ObjectModel that is 17.6.0. In MSTest (and xunit and nunit) the ObjectModel is used as a reference, but is not shipped.

User can happily run tests.


Later MSTest releases 3.3.0 and that depends on 17.7.0, Dependabot upgrades MSTest in this project, but it won't update Microsoft.NET.Sdk.Test because that is not a nuget package. Tests try to load ObjectModel 17.7.0, but only 17.6.0 is available and we cannot run tests.

User tries to fix this problem and installs ObjectModel 17.7.0 explicitly, testhost loads it, but there is some incompatibility, and tests still don't run.

If we were not a built-in SDK, user would update both MSTest and Microsoft.NET.Test.SDK nugets and they would both work.


With XUnit or NUnit you won't see this problem often, because they are sticking with a very old version of ObjectModel, and we keep backward compatibility.

@richlander
Copy link
Member Author

Sorry. We were talking past one another. I am not pushing on the specific implementation detail. An SDK or a package controlled via Central Package Management is fine.

Tests try to load ObjectModel 17.7.0, but only 17.6.0 is available and we cannot run tests.

Isn't this already a problem?

@nohwnd
Copy link
Member

nohwnd commented Jul 27, 2023

Sorry. We were talking past one another. I am not pushing on the specific implementation detail.

I would like test.sdk to be built in, I am just trying to see if it would work.

Isn't this already a problem?

Not often, because you can freely update Microsoft.NET.Test.Sdk nuget, so if DependaBot pushes you to the latest MSTest version it also pushes you to the latest Microsoft.NET.Test.Sdk.

If you don't use dependabot you are probably sticking with version of MSTest and Microsoft.NET.Test.Sdk that works for you. And you are not silently upgrading half of this (Microsoft.NET.Test.Sdk) by changing dotnet sdk version.

@richlander
Copy link
Member Author

DependaBot pushes you to the latest MSTest version it also pushes you to the latest Microsoft.NET.Test.Sdk.

That's a good point. I wasn't thinking of that dynamic. Your point (which I now understand) is that these test packages are often updated together such that they are likely to compatible (if not a perfect match). That's an interesting dynamic.

Perhaps there is another way. Imagine a carefully curated set of Directory.Packages.props (with unique names, like Directory.TestPackages.props) that pulled together all the relevant packages (Microsoft.NET.Test.Sdk, MSTest, nunut, xunit, ...) and kept their packages versions updated to latest. Customers could then subscribe to those through some mechanism and (in the common case) keep those at repo root. The mechanism could be via restore, Dependabot, or GitHub Actions.

Point being that perhaps using the SDK as the vehicle isn't the best idea, but making these files an external thing that works well for any packages and work just as well for the community.

The problem is that Central Package Versioning doesn't enable this scenario currently. We'd have to fix that. However, coming up with a general plan we like is a first step.

@baronfel
Copy link
Member

This idea of different groups of dependencies that change at different rates is a pretty common thing in the Paket world. Paket allows for grouping dependencies by name, and referring to packages within those groups from projects. Then, it becomes a matter of updating just that group when you want to

  • check for outdated packages: paket outdated [--group <groupname>]
  • update that group (or a package inside that group): paket update <package name> [--group <groupname>] [--version <version constraint>]

it's a really nice workflow that I wish NuGet/CPM would support in a first-class manner.

@WeihanLi
Copy link
Contributor

The dotnet-outdated tool would be helpful for updating the outdated package
https://github.com/dotnet-outdated/dotnet-outdated

@richlander
Copy link
Member Author

Yes, but it isn't integrated into the toolchain. I'm looking for something automatic.

If dotnet restore told you to run that tool, that'd be cool.

@WeihanLi
Copy link
Contributor

If dotnet restore told you to run that tool, that'd be cool.

Maybe better with a switch/option since we may not need the outdated info when publishing or building a docker image

@richlander
Copy link
Member Author

I need to go back to the team and see what we can do. At best, we'll do something for a .NET 8 SDK feature band update like 8.0.200. More likely, we'll need to consider this as part of .NET 9.

@baronfel
Copy link
Member

@nohwnd circling back to this one, is the new, unified package you all created the way forward here?

@Evangelink
Copy link
Member

@baronfel Are you referring to the platform behind MSTest runner? Or to the update that was done in MSBuild to allow /t:Test? Or maybe even something else?!

@baronfel
Copy link
Member

I was thking about the platform - the announcement post suggested that merely adding the package reference would accomplish a lot of the goals of this issue. If that's not the case we can keep this one open - I'm just doing some triage at the moment.

@Evangelink
Copy link
Member

This is one of the goal we want to accomplish for the new platform but it's not yet available publicly. I'll setup some call with you because there are many details I'd like to discuss before commiting on anything.

@richlander
Copy link
Member Author

Will the new mstest also be a NuGet package? If so, that doesn't seem like it would solve much since I assume it would then need to update on the same cadence.

@Evangelink
Copy link
Member

The idea is to provide 2 SDK packages, one that gets inserted into SDK and follows SDK lifecycle and one that ships on the side (let's say something like OutOfBand) that allows people to benefit from new features early or even to get sooner a bug fix (we have had a few instances where we provided fixes nearly next day but users were complaining because it tooks months for the fix to be shipped in SDK.

@Grastveit
Copy link

Grastveit commented Feb 16, 2024

Is it possible to opt-in for SDK-updates with little overhead by creating a custom tools package?

We have many test-projects in separate repositories. I guess that makes Directory.Packages.props-file unsuitable as central package management...

Can a package be used instead? Its .csproj should add specific versions of Microsoft.NET.Test.Sdk, xunit, covertlet+setup, and more, in such a way that test-detection work for consumers.

@Varorbc
Copy link

Varorbc commented Apr 12, 2024

I saw the solution in the blog in the morning, but the name MSTest.Sdk is strange. I prefer his name is Microsoft.NET.Sdk.Test. I also created the issue

@Evangelink
Copy link
Member

I saw the solution in the blog in the morning, but the name MSTest.Sdk is strange. I prefer his name is Microsoft.NET.Sdk.Test. I also created the issue

Microsoft.Net.TestSdk is linked to VSTest and not MSTest. It contains the "testhost" and some dependencies required to host the execution of the test.

@richlander @baronfel Given that we are trying to push on our new testing platform (the platform behind MSTest runner and hopefully soon available for xUnit and NUnit), I would like to avoid the Microsoft.NET.Test.Sdk.

@paulomorgado
Copy link

Microsoft.NET.Test.Sdk

Microsoft.NET.MSTest.Sdk?

@Nirmal4G
Copy link
Contributor

@Evangelink

I thought Microsoft was good at naming. You guys disappoint me. You have loads of names at your disposal. Come on! Pick a good one that is simple and is a one word change to the standard SDK Name.

Some Examples:

  • Microsoft.Test.Sdk
  • Microsoft.Testing.Sdk
  • Microsoft.NET.Test.Sdk
  • Microsoft.NET.Sdk.Test
  • Microsoft.NET.Sdk.Testing
  • Microsoft.NET.Testing.Sdk

Pick one from above that seems intuitive to users who are already familiar with existing packages. Please do note that Microsoft.NET.Test.Sdk package can only be used as a package reference. So, why not convert to an SDK, ship in-band with the .NET SDK and deprecate the package reference functionality after v18?

@Evangelink
Copy link
Member

Let's continue the discussion of naming of MSTest in MSTest ticket microsoft/testfx#2693 so we don't pollute this thread that has a totally different agenda.

@nohwnd
Copy link
Member

nohwnd commented Apr 12, 2024

To me an sdk is not a good fit for shipping vstest. Lot of its functionality lies in test frameworks that are not owned or maintained by us.

There are 2 options.

  1. Recommend to framework authors (e.g. mstest, xunit, nunit) to depend on microsoft.net.test.sdk (or similar) nuget, and make sure they use our targets (we cannot make them transitive) to generate program.cs. And remove microsoft.net.test.sdk from project templates.

MSTest nuget package already does this.

The reference in the project looks like this: <PackageReference Include="MSTest" Version="3.3.1" />, and it installs Microsoft.NET.Test.SDK as dependency.

This (to me) is preferable because:

  • extensions need to keep binary compatibility with Object Model and Testhost. So it is beneficial to them to control the version of testhost they are deployed with.
  • we don't limit who can do this, and don't give preferential treatment to a small subset of most used test frameworks.
  • Users can stay with the same version of their testing framework across SDK updates, making it easier to ensure that their code still works without changing tests.
  1. Add this SDK. This requires us to make some test frameworks first class. We need to ensure that we are deploying them in their typical setup. And we need to keep them up to date, but not too new.

Updating the the sdk would also mean that users could see build errors because their mstest/nunit/xunit was updated automatically with sdk update, making it difficult to ensure that their code works on this new sdk without making code changes to their tests.

Having this SDK will solve updating of 1 nuget package, and will ensure that the version of testhost and dotnet sdk aligns, but there we communicate over IPC so keeping compatibility is not as difficult as in 1) above.

@Nirmal4G
Copy link
Contributor

Why not? We're only using SDK as a delivery mechanism for opinionated set of defaults and allow extending when needed. Similar to the .NET SDK that ships with... well... the .NET SDK!

You can still deliver the frameworks via Framework and Package references individually and ask the customer to add them per their requirements.

Essentially it's same as .NET SDK but with 10s packages instead of 100s of packages!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

9 participants