Fix arcade property for Microsoft.VisualStudio.Razor.IntegrationTests.csproj #10661
Conversation
…Test Uses `UseVSTestRunner` instead of `TestRunneName`. Fixes dotnet#10660
|
Our integration tests are weird, and are 100% definitely written using xunit and so we need to use the xunit runner. Is the current set up causing issues somewhere? Or is this just an oddity you noticed? Because if so, I suspect the real answer is that you're right, it is odd, but it also works :) |
Not at all! I am the owner of MSTest and was doing some lookup with @ViktorHofer of the repos onboarded to arcade and using MSTest and you were one of them. I was happily surprised so wanted to see how you were using MSTest and realized you don't and you probably only mixed up MSTest and VSTest :D |
Yeah something is strange! You are relying on VSTest runner but you also add |
| @@ -14,7 +14,7 @@ | |||
| using Microsoft.Internal.VisualStudio.Shell.Interop; | |||
| using Microsoft.VisualStudio.Settings; | |||
| using Microsoft.VisualStudio.Shell; | |||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | |||
There was a problem hiding this comment.
You were using 2 assertions from MSTest but all the rest of the project is using xUnit so I have changed these 2 assertions to use xUnit.
| @@ -39,19 +39,8 @@ | |||
| <PackageReference Include="Microsoft.VisualStudio.Language.Intellisense" /> | |||
| <PackageReference Include="NuGet.VisualStudio" /> | |||
| <PackageReference Include="Microsoft.Internal.VisualStudio.Shell.Framework" /> | |||
| <PackageReference Include="xunit" /> | |||
There was a problem hiding this comment.
This is a metapackage and all its transitive deps are already brought up by arcade so that's now useless.
| <PackageReference Include="xunit.runner.console" Version="$(XUnitRunnerConsoleVersion)" IsImplicitlyDefined="true" PrivateAssets="all" Publish="true" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="$(XUnitRunnerVisualStudioVersion)" IsImplicitlyDefined="true" PrivateAssets="all" Publish="true" /> |
There was a problem hiding this comment.
Already brought by arcade
| the Arcade XUnit.targets file, but finding that file is a pain, so instead we can just manually | ||
| import the package ourselves, but as an implicit definition to stop Nuget complaining. | ||
| TL;DR: MSBuild was designed to cause pain. | ||
| --> | ||
| <PackageReference Include="xunit.runner.console" Version="$(XUnitRunnerConsoleVersion)" IsImplicitlyDefined="true" PrivateAssets="all" Publish="true" /> |
There was a problem hiding this comment.
This package is strange. Either you want to run tests from VSTest/dotnet test (which seems to be the case given comment on line 8) or through xunit runner (in which case it's brought by arcade if you don't specify UseVSTestRunner).
There was a problem hiding this comment.
Agreed. The Arcade Sdk already brings this in.
|
Thanks for the updates, at least it compiles now :) I'm going to kick off an integration test run and if that passes, I'm happy to merge. In the past this area has proven very fragile though, so please forgive my caution. |
I know what it's like to have fragile testing infra so no worries at all! I honestly prefer that you take some time and don't call me back for an issue after that xD |
|
92 tests ran, and passed, so that's good enough for me. Thanks! |
Summary of the changes
Uses
UseVSTestRunnerinstead ofTestRunneName.Fixes #10660