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.

Sign up for GitHub

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

Jump to bottom

Make VSTest runner work and enable html log #33915

Merged
merged 1 commit into from
Mar 26, 2020
Merged

Make VSTest runner work and enable html log #33915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@
<XUnitVersion>2.4.1</XUnitVersion>
<TraceEventVersion>2.0.5</TraceEventVersion>
<NewtonsoftJsonVersion>12.0.3</NewtonsoftJsonVersion>
<XUnitXmlTestLoggerVersion>2.1.26</XUnitXmlTestLoggerVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed? Just left-over?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this was introduced only for vstest in a past PR but actually we don't need it.

<MoqVersion>4.12.0</MoqVersion>
<!-- Docs -->
<MicrosoftPrivateIntellisenseVersion>3.1.0-preview-20200129.1</MicrosoftPrivateIntellisenseVersion>
Expand Down
12 changes: 4 additions & 8 deletions eng/testing/xunit/vstest.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,15 @@
<PropertyGroup>
<!-- Microsoft.Net.Test.Sdk brings a lot of satellite assemblies in. -->
<SatelliteResourceLanguages>en</SatelliteResourceLanguages>
<TrxTestResultsName>testResults.trx</TrxTestResultsName>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if the helix sdk supports .trx files to show the test results in AzDo, submitting to kusto and failing the build? If not we should request a feature for when we want to move to use this as our default test runner. cc: @alexperovich

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helix supports trx files for uploading to azure devops. Kusto is only supported for xunit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a lot of people use Kusto for metrics and what not as the AzDo APIs aren't that great to get test results and search through them. Would it be possible to add Kusto from a trx file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The support for processing xunit results into kusto is on the server side for helix and extending that for other formats would be a very large an potentially disruptive change. The azure devops uploading code is in arcade and very easy to change so it supports many formats. My understanding is that we want to consolidate everything to azure devops and make that experience better, rather than continue investing in our custom reporting. @Chrisboh may have more context.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I knew that trx is supported but I didn't know about Kusto only working with the xunit xml format.

<HtmlTestResultsName>testResults.html</HtmlTestResultsName>
<TestResultsName>$(HtmlTestResultsName)</TestResultsName>
<TestRunnerConfigPath>$(MSBuildThisFileDirectory)vstest.xunit.json</TestRunnerConfigPath>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="xunit.runner.visualstudio" Version="$(XUnitVersion)" />
<PackageReference Include="XunitXml.TestLogger" Version="$(XUnitXmlTestLoggerVersion)" />

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkVersion)" />
<PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
</ItemGroup>

<ItemGroup>
<None Include="$(TestRunnerConfigPath)"
CopyToOutputDirectory="PreserveNewest"
Visible="false" />
</ItemGroup>
</Project>
10 changes: 7 additions & 3 deletions eng/testing/xunit/vstest.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
<Project>
<PropertyGroup>
<RunCommand>"$(RunScriptHost)"</RunCommand>
<RunCommand>"$(DotNetTool)"</RunCommand>
<RunCommand Condition="'$(ContinuousIntegrationBuild)' == 'true'">dotnet</RunCommand>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand, could you explain what's the difference in between DotNetTool and dotnet and why does it has to be different in CI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DotNetTool is a property set by Arcade, mean it's available during the test invocation on your machine. On a different machine you don't want to hardcode the path into the RunTemplate.cmd/sh. There is more work required here anyway to make this work on CI.

My goal in a subsequent PR is to just use dotnet here instead and then for local local development, modify PATH to point to the RunScriptHost if not on CI. As said, I don't want to do this now as this just enables VSTest optionally and not on CI.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another blocker for enabling VSTest in CI is #1980.

<RunArguments>test $(TargetFileName)</RunArguments>

<!-- CLI options -->
<RunArguments>$(RunArguments) --logger "xunit;LogFilePath=$(TestResultsName)"</RunArguments>
<RunArguments>$(RunArguments) --nologo</RunArguments>
<RunArguments>$(RunArguments) --logger "trx;LogFileName=$(TrxTestResultsName)"</RunArguments>
<RunArguments>$(RunArguments) --logger "html;LogFileName=$(HtmlTestResultsName)"</RunArguments>
<RunArguments>$(RunArguments) --framework $(TargetFramework)</RunArguments>
<RunArguments>$(RunArguments) --platform $(ArchGroup)</RunArguments>
<RunArguments Condition="'$(TestDisableParallelization)' != 'true'">$(RunArguments) --parallel</RunArguments>
Expand All @@ -27,6 +30,7 @@
<!-- RunConfiguration settings. -->
<RunSettingsOptions Condition="'$(TestDisableParallelization)' == 'true'">$(RunSettingsOptions) RunConfiguration.DisableParallelization=true</RunSettingsOptions>
<RunSettingsOptions Condition="'$(TestDisableAppDomain)' == 'true'">$(RunSettingsOptions) RunConfiguration.DisableAppDomain=true</RunSettingsOptions>
<RunArguments Condition="'$(RunSettingsOptions)' != ''">$(RunArguments) --(RunSettingsOptions)</RunArguments>
<RunSettingsOptions>$(RunSettingsOptions) RunConfiguration.DotNetHostPath="$(RunScriptHost)"</RunSettingsOptions>
<RunArguments>$(RunArguments) --$(RunSettingsOptions)</RunArguments>
</PropertyGroup>
</Project>
5 changes: 5 additions & 0 deletions eng/testing/xunit/vstest.xunit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"diagnosticMessages": false,
"longRunningTestSeconds": 120,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this support is now built-in in VSTest?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but I still need to play around with it. There's a blame mode which creates a sequence.xml which tells you which tests have run and which haven't but I don't know if that mode slows down the test invocation. cc @nohwnd

"shadowCopy": false
}
0 eng/testing/xunit/xunit.runner.json → eng/testing/xunit/xunit.console.json
View file
Open in desktop
File renamed without changes.
2 changes: 2 additions & 0 deletions eng/testing/xunit/xunit.console.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<Project>
<PropertyGroup>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
<TestResultsName>testResults.xml</TestResultsName>
<TestRunnerConfigPath>$(MSBuildThisFileDirectory)xunit.console.json</TestRunnerConfigPath>
</PropertyGroup>

<ItemGroup>
Expand Down
5 changes: 0 additions & 5 deletions eng/testing/xunit/xunit.console.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,6 @@
<!-- ResolveAssemblyReferences is the target that populates ReferenceCopyLocalPaths which is what is copied to output directory. -->
<Target Name="CopyRunnerToOutputDirectory" BeforeTargets="ResolveAssemblyReferences">
<ItemGroup>
<!-- Add the runner configuration file -->
<None Include="$(TestRunnerConfigPath)"
CopyToOutputDirectory="PreserveNewest"
Visible="false" />

<!-- Copy test runner to output directory -->
<None Include="$([System.IO.Path]::GetDirectoryName('$(XunitConsole472Path)'))\*"
Exclude="$([System.IO.Path]::GetDirectoryName('$(XunitConsole472Path)'))\xunit.console.*exe.config;$([System.IO.Path]::GetDirectoryName('$(XunitConsole472Path)'))\xunit.console.x86.exe"
Expand Down
2 changes: 0 additions & 2 deletions eng/testing/xunit/xunit.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<Project>
<PropertyGroup>
<TestRunner Condition="'$(TestRunner)' == ''">xunit.console</TestRunner>
<TestRunnerConfigPath>$(MSBuildThisFileDirectory)xunit.runner.json</TestRunnerConfigPath>
<TestResultsName>testResults.xml</TestResultsName>
</PropertyGroup>

<ItemGroup>
Expand Down
7 changes: 7 additions & 0 deletions eng/testing/xunit/xunit.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@
<_withoutCategories Condition="'$(TargetOSCategory)' != ''">$(_withoutCategories);$(TargetOSCategory)</_withoutCategories>
</PropertyGroup>

<ItemGroup>
<None Include="$(TestRunnerConfigPath)"
Link="xunit.runner.json"
CopyToOutputDirectory="PreserveNewest"
Visible="false" />
</ItemGroup>

<Target Name="ValidateTargetOSCategory"
BeforeTargets="GenerateRunScript">
<Error Condition="'$(TargetOSCategory)' == ''"
Expand Down
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"rollForward": "major"
},
"tools": {
"dotnet": "5.0.100-preview.3.20168.11"
"dotnet": "5.0.100-preview.3.20170.6"
},
"native-tools": {
"cmake": "3.14.2",
Expand Down