Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Use dotnet MSBuild for tests #19324

Merged
merged 3 commits into from
Aug 8, 2018
Merged

Use dotnet MSBuild for tests #19324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9b37dc
Revert "Revert "Build managed tests with dotnet (#19254)""
AaronRobinsonMSFT Aug 6, 2018
0418137
Reenable MSBuild support through dotnet instead of global MSBuild
AaronRobinsonMSFT Aug 7, 2018
537dd5d
Merge remote-tracking branch 'upstream/master' into useDotNetMSBuildF…
AaronRobinsonMSFT Aug 8, 2018
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
10 changes: 6 additions & 4 deletions build-test.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ if [!processedArgs!]==[] (
:ArgsDone

@REM Special handling for -priority=N argument.
if %__Priority% GTR 0 (set "__PriorityArg=-priority=%__Priority%")
if %__Priority% GTR 0 (
set "__PriorityArg=-priority=%__Priority%"
set "__PriorityMsbuildArg=/p:CLRTestPriorityToBuild=%__Priority%"
Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The issue was that for MSBuild the correct property is CLRTestPriorityToBuild, not Priority. The priority flag that was previously passed is mapped in the config.json file and converted in the depths of run.exe 😖 unclear why this mapping logic exists.

)

if defined __BuildAgainstPackagesArg (
if not defined __RuntimeID (
Expand All @@ -120,7 +123,7 @@ if defined __BuildAgainstPackagesArg (

set __RunArgs=-BuildOS=%__BuildOS% -BuildType=%__BuildType% -BuildArch=%__BuildArch%
REM As we move from buildtools to arcade, __RunArgs should be replaced with __msbuildArgs
set __msbuildArgs=/p:__BuildOS=%__BuildOS% /p:__BuildType=%__BuildType% /p:__BuildArch=%__BuildArch%
set __msbuildArgs=/p:__BuildOS=%__BuildOS% /p:__BuildType=%__BuildType% /p:__BuildArch=%__BuildArch% /nologo /verbosity:minimal /clp:Summary

echo %__MsgPrefix%Commencing CoreCLR test build

Expand Down Expand Up @@ -317,10 +320,9 @@ for /l %%G in (1, 1, %__BuildLoopCount%) do (
set __MsbuildLog=/flp:Verbosity=normal;LogFile="%__BuildLog%";Append=!__AppendToLog!
set __MsbuildWrn=/flp1:WarningsOnly;LogFile="%__BuildWrn%";Append=!__AppendToLog!
set __MsbuildErr=/flp2:ErrorsOnly;LogFile="%__BuildErr%";Append=!__AppendToLog!
set __Logging=-MsBuildLog=!__MsbuildLog! -MsBuildWrn=!__MsbuildWrn! -MsBuildErr=!__MsbuildErr!

set TestBuildSlice=%%G
call "%__ProjectDir%\run.cmd" build -Project=%__ProjectDir%\tests\build.proj !__Logging! %__RunArgs% %__BuildAgainstPackagesArg% %__PriorityArg% %__UnprocessedBuildArgs%
call %__DotnetHost% msbuild %__ProjectDir%\tests\build.proj !__MsbuildLog! !__MsbuildWrn! !__MsbuildErr! %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% !__PriorityMsbuildArg! %__PassThroughArg% %__UnprocessedBuildArgs%

if errorlevel 1 (
echo %__MsgPrefix%Error: build failed. Refer to the build log files for details:
Expand Down
4 changes: 2 additions & 2 deletions tests/build.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@
</Target>

<Target Name="RestorePackage">
<Exec Condition="'$(RunningOnCore)' == 'false'" Command="$(DotnetRestoreCommand) $(RestoreProj) $(PackageVersionArg)" StandardOutputImportance="Low" />
<Exec Condition="'$(RunningOnCore)' == 'true'" Command="$(DotnetRestoreCommand) -r $(__DistroRid) $(RestoreProj) $(PackageVersionArg)" StandardOutputImportance="Low" />
<Exec Condition="'$(__DistroRid)' == ''" Command="$(DotnetRestoreCommand) $(RestoreProj) $(PackageVersionArg)" StandardOutputImportance="Low" />
<Exec Condition="'$(__DistroRid)' != ''" Command="$(DotnetRestoreCommand) -r $(__DistroRid) $(RestoreProj) $(PackageVersionArg)" StandardOutputImportance="Low" />
Copy link

Choose a reason for hiding this comment

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

Dependency restore is one of the pain points in test build. Could we explicitly output more detailed info on restore process? It would be sufficient to hook above commands log output at normal verbosity to logs and have a StandardOutputImportance console output set to High with minimal verbosity for console output set in restore command.

Copy link
Member Author

Choose a reason for hiding this comment

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

What exactly is the pain point? Does it fail? Does it not properly fully restore? I am more than willing to add logging, but I would like to understand pain points because there may be a more general fix.

Copy link

Choose a reason for hiding this comment

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

The problem is that there are multiple already attempts at fixing dependencies in particular for performance tests where majority of them failed from different reasons. See #16647 #9281 #16126. Discussion related to the problem was in some comments of the https://github.com/dotnet/corefx/issues/31536#issuecomment-409635103 issue.

In general build-test configuration causes download of roughly 2.5 GB of assemblies out of which only about 500 - 600 are necessary. There is a possibility of reducing that amount by careful management of dependencies. This will lead to deterministic build which should be made against built product and chosen CoreFX or netstandard assemblies. Detailed information on dependency graph resolution would make any feature test dependencies updates easier.

Part of the above goal can be achieved by introducing new features into NuGet which would allow to override resolution of inbox assemblies with latest used version.

Solution which will not impact standard builds could be based on verbosity parameter passed to build-test.(cmd|sh) which would be used in the above code to set logging verbosity for restore and visibility of command line messages in console output by changing StandardOutputImportance

Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

That is indeed a valuable goal. In this case, however providing a new knob to configure logging for the update isn't the purpose of this PR. It is for a long standing work item related to consolidating usage of dotnet's MSBuild version. I think adding that support is very valid and should be done.

</Target>

<!-- Override RestorePackages from dir.traversal.targets and do a batch restore -->
Expand Down
1 change: 1 addition & 0 deletions tests/dir.sdkbuild.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<RuntimeFrameworkVersion>$(MicrosoftNETCoreAppPackageVersion)</RuntimeFrameworkVersion>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<Platform>AnyCPU</Platform>

<!-- Force the CLI to allow us to target higher netcoreapp than it may know about -->
<NETCoreAppMaximumVersion>99.0</NETCoreAppMaximumVersion>
Expand Down
4 changes: 2 additions & 2 deletions tests/src/dir.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="..\dir.props" />

<Import Project="..\dir.common.props" />
<Import Project="..\dir.common.props" Condition="'$(UsingMicrosoftNETSdk)' != 'true'" />

<!-- Setup Default symbol and optimization for Configuration -->
<PropertyGroup Condition="'$(Configuration)' == 'Debug'">
Expand Down Expand Up @@ -37,7 +37,7 @@
<BaseOutputPath Condition="'$(__TestRootDir)' != ''">$(__TestRootDir)</BaseOutputPath>
<BaseOutputPathWithConfig>$(BaseOutputPath)\$(OSPlatformConfig)\</BaseOutputPathWithConfig>
<BinDir>$(BaseOutputPathWithConfig)</BinDir>
<BaseIntermediateOutputPath>$(ProjectDir)\..\bin\tests\obj\$(OSPlatformConfig)\Managed\</BaseIntermediateOutputPath>
<BaseIntermediateOutputPath>$(ProjectDir)..\bin\tests\obj\$(OSPlatformConfig)\Managed\</BaseIntermediateOutputPath>
<BaseIntermediateOutputPath Condition="'$(__ManagedTestIntermediatesDir)' != ''">$(__ManagedTestIntermediatesDir)\</BaseIntermediateOutputPath>
<__NativeTestIntermediatesDir Condition="'$(__NativeTestIntermediatesDir)' == ''">$([System.IO.Path]::GetFullPath($(BaseOutputPathWithConfig)..\obj\$(BuildOS).$(Platform).$(Configuration)\Native\))</__NativeTestIntermediatesDir>
<BuildProjectRelativeDir>$(MSBuildProjectName)\</BuildProjectRelativeDir>
Expand Down