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

use TestDisableParallelization property to disable parallel test execution #62133

Closed

Conversation

adamsitnik
Copy link
Member

It's defined in:

<RunSettingsFileContent Condition="'$(TestDisableParallelization)' == 'true'">$(RunSettingsFileContent.Replace('$$MAXCPUCOUNT$$', '1'))</RunSettingsFileContent>
<RunSettingsFileContent Condition="'$(TestDisableParallelization)' != 'true'">$(RunSettingsFileContent.Replace('$$MAXCPUCOUNT$$', '0'))</RunSettingsFileContent>
<!-- Arm64 is currently not a known TargetPlatform value in VSTEST: https://github.com/microsoft/vstest/issues/2566 -->
<RunSettingsFileContent Condition="'$(TargetArchitecture)' != 'arm64'">$(RunSettingsFileContent.Replace('$$TARGETPLATFORM$$', '<TargetPlatform>$(TargetArchitecture)</TargetPlatform>'))</RunSettingsFileContent>
<RunSettingsFileContent Condition="'$(TargetArchitecture)' == 'arm64'">$(RunSettingsFileContent.Replace('$$TARGETPLATFORM$$', ''))</RunSettingsFileContent>
<RunSettingsFileContent>$(RunSettingsFileContent.Replace('$$COVERAGE_INCLUDE$$', '$(CoverageIncludeFilter)')
.Replace('$$COVERAGE_EXCLUDEBYFILE$$', '$(CoverageExcludeByFileFilter)')
.Replace('$$COVERAGE_INCLUDEDIRECTORY$$', '$(CoverageIncludeDirectoryFilter)')
.Replace('$$COVERAGE_ENABLED$$', '$([MSBuild]::ValueOrDefault('$(Coverage)', 'false'))')
.Replace('$$DISABLEPARALLELIZATION$$', '$([MSBuild]::ValueOrDefault('$(TestDisableParallelization)', 'false'))')
.Replace('$$DISABLEAPPDOMAIN$$', '$([MSBuild]::ValueOrDefault('$(TestDisableAppDomain)', 'false'))')
.Replace('$$TESTCASEFILTER$$', '$(_testFilter)')
.Replace('$$DOTNETHOSTPATH$$', '$(NetCoreAppCurrentTestHostPath)$([System.IO.Path]::GetFileName('$(DotNetTool)'))'))</RunSettingsFileContent>

and I've tested that it works as expected.

@ghost
Copy link

ghost commented Nov 29, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

It's defined in:

<RunSettingsFileContent Condition="'$(TestDisableParallelization)' == 'true'">$(RunSettingsFileContent.Replace('$$MAXCPUCOUNT$$', '1'))</RunSettingsFileContent>
<RunSettingsFileContent Condition="'$(TestDisableParallelization)' != 'true'">$(RunSettingsFileContent.Replace('$$MAXCPUCOUNT$$', '0'))</RunSettingsFileContent>
<!-- Arm64 is currently not a known TargetPlatform value in VSTEST: https://github.com/microsoft/vstest/issues/2566 -->
<RunSettingsFileContent Condition="'$(TargetArchitecture)' != 'arm64'">$(RunSettingsFileContent.Replace('$$TARGETPLATFORM$$', '<TargetPlatform>$(TargetArchitecture)</TargetPlatform>'))</RunSettingsFileContent>
<RunSettingsFileContent Condition="'$(TargetArchitecture)' == 'arm64'">$(RunSettingsFileContent.Replace('$$TARGETPLATFORM$$', ''))</RunSettingsFileContent>
<RunSettingsFileContent>$(RunSettingsFileContent.Replace('$$COVERAGE_INCLUDE$$', '$(CoverageIncludeFilter)')
.Replace('$$COVERAGE_EXCLUDEBYFILE$$', '$(CoverageExcludeByFileFilter)')
.Replace('$$COVERAGE_INCLUDEDIRECTORY$$', '$(CoverageIncludeDirectoryFilter)')
.Replace('$$COVERAGE_ENABLED$$', '$([MSBuild]::ValueOrDefault('$(Coverage)', 'false'))')
.Replace('$$DISABLEPARALLELIZATION$$', '$([MSBuild]::ValueOrDefault('$(TestDisableParallelization)', 'false'))')
.Replace('$$DISABLEAPPDOMAIN$$', '$([MSBuild]::ValueOrDefault('$(TestDisableAppDomain)', 'false'))')
.Replace('$$TESTCASEFILTER$$', '$(_testFilter)')
.Replace('$$DOTNETHOSTPATH$$', '$(NetCoreAppCurrentTestHostPath)$([System.IO.Path]::GetFileName('$(DotNetTool)'))'))</RunSettingsFileContent>

and I've tested that it works as expected.

Author: adamsitnik
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@jkotas
Copy link
Member

jkotas commented Nov 29, 2021

I've tested that it works as expected.

Does it work as expected when running tests from command line or in the CI? The .runsettings file seems to be VS specific.

@adamsitnik
Copy link
Member Author

Does it work as expected when running tests from command line or in the CI?

I was running it using the dotnet.cmd script:

.\dotnet.cmd build /t:Test .\src\libraries\System.IO.FileSystem.DriveInfo\tests\System.IO.FileSystem.DriveInfo.Tests.csproj  /p:Configuration=Release

Which I assume is used by the CI? I am going to run it using a global SDK and get back to you.

@adamsitnik
Copy link
Member Author

adamsitnik commented Nov 29, 2021

It works both when using dotnet.cmd and a dotnet.exe from $PATH

@jkotas
Copy link
Member

jkotas commented Nov 29, 2021

Ah ok, it is passed to XUnit on command line here:

<RunScriptCommand Condition="'$(TestDisableParallelization)' == 'true'">$(RunScriptCommand) -maxthreads 1</RunScriptCommand>

@jkotas
Copy link
Member

jkotas commented Nov 29, 2021

But it is not obvious to me that the extra argument will be passed all the way through for mobile test targets (iOS/Android/browser) that encapsulate the test in an app.

I think the existing solution where the test spec is fully encapsulated in the test .dll is better.

@stephentoub
Copy link
Member

I think the existing solution where the test spec is fully encapsulated in the test .dll is better.

+1. The less msbuild magic that might subtly be missed in various configurations, the better.

@adamsitnik
Copy link
Member Author

@jkotas @stephentoub Thank you for your feedback. I am going to close the PR and just move src/libraries/System.Private.Xml/tests/Writers/XmlWriterApi/DisableParallelization.cs to some common folder and re-use it for System.IO.FileSystem.DriveInfo tests (#62134)

@adamsitnik adamsitnik closed this Nov 30, 2021
@adamsitnik adamsitnik deleted the TestDisableParallelization branch November 30, 2021 09:29
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants