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

Build managed tests with dotnet #19230

Merged
merged 6 commits into from
Aug 1, 2018
Merged

Conversation

chsienki
Copy link

@chsienki chsienki commented Aug 1, 2018

Similar to the way we now build the xunit wrappers with the CLI dotnet, this change moves the managed projects to use the CLI as the build invoker as well.

On the build servers we only have the 1.1 sdk installed, which leads to failures in the tests when building them using desktop msbuild. While we could install a newer sdk on the build machines, and most developers already have a suitable > 2.0 sdk, this reduces our external dependencies and ensures the builds won't fail due to external environment issues.

@@ -10,6 +10,7 @@
<RuntimeFrameworkVersion>$(MicrosoftNETCoreAppPackageVersion)</RuntimeFrameworkVersion>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<Platform>AnyCPU</Platform>
Copy link
Member

Choose a reason for hiding this comment

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

Is this property still used in SDK projects? I always thought of this as a very VS specific property.

Copy link
Author

@chsienki chsienki Aug 1, 2018

Choose a reason for hiding this comment

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

Yes, it's still used. By default in SDK projects it seems to be set to x64/x86 etc, rather than AnyCPU. For the cross arm builds, we build the test exes on windows and copy them to arm, so compiling not AnyCPU breaks that.

build-test.cmd Outdated
@@ -321,7 +321,7 @@ for /l %%G in (1, 1, %__BuildLoopCount%) do (
set __msbuildErr=/flp2:ErrorsOnly;LogFile="%__BuildErr%";Append=!__AppendToLog!

set TestBuildSlice=%%G
call "%__ProjectDir%\run.cmd" build -Project=%__ProjectDir%\tests\build.proj -MsBuildLog=!__msbuildLog! -MsBuildWrn=!__msbuildWrn! -MsBuildErr=!__msbuildErr! %__RunArgs% %__BuildAgainstPackagesArg% %__PriorityArg% %__PassThroughArg% %__unprocessedBuildArgs%
call %__DotnetHost% msbuild %__ProjectDir%\tests\build.proj !__msbuildLog! !__msbuildWrn! !__msbuildErr! %__msbuildArgs% %TargetsWindowsMsbuildArg% %__BuildAgainstPackagesMsbuildArg% %__unprocessedBuildArgs%

Copy link
Member

Choose a reason for hiding this comment

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

Why are we no longer passing the Priority argument? And do we need to pass TargetsWindowsMsbuildArg?

Copy link
Author

Choose a reason for hiding this comment

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

And this is why I asked for a review 👍 Let me take a look

@@ -321,7 +321,7 @@ for /l %%G in (1, 1, %__BuildLoopCount%) do (
set __msbuildErr=/flp2:ErrorsOnly;LogFile="%__BuildErr%";Append=!__AppendToLog!

set TestBuildSlice=%%G
call "%__ProjectDir%\run.cmd" build -Project=%__ProjectDir%\tests\build.proj -MsBuildLog=!__msbuildLog! -MsBuildWrn=!__msbuildWrn! -MsBuildErr=!__msbuildErr! %__RunArgs% %__BuildAgainstPackagesArg% %__PriorityArg% %__PassThroughArg% %__unprocessedBuildArgs%
call %__DotnetHost% msbuild %__ProjectDir%\tests\build.proj !__msbuildLog! !__msbuildWrn! !__msbuildErr! %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% %__PriorityArg% %__PassThroughArg% %__unprocessedBuildArgs%

Choose a reason for hiding this comment

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

Call in this form misses on default parameters passed by run.cmd to msbuild call. One of them already missed in TestWrapper builds is /clp:Summary which prints summary of run. All default parameters are available in config.json in repo root.

Copy link

@4creators 4creators left a comment

Choose a reason for hiding this comment

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

Pls restore default parameters

Copy link

@4creators 4creators left a comment

Choose a reason for hiding this comment

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

Thanks

@chsienki chsienki merged commit 763142a into dotnet:master Aug 1, 2018
@BruceForstall
Copy link
Member

This has broken pri-1 builds, e.g.:

16:37:14 BUILDTEST: Starting the Managed Tests Build
16:37:14 BUILDTEST: Building tests group 2 with 16 subgroups
16:37:14 
MSBUILD : error MSB1001: Unknown switch.
16:37:14 Switch: -priority=1
16:37:14 
16:37:14 For switch syntax, type "MSBuild /help"

https://ci.dot.net/job/dotnet_coreclr/job/master/view/x64/job/checked_windows_nt/10074/consoleFull#-208510365679494335-f7bd-47d0-8771-8661e00c2db2

I'm going to revert it.

BruceForstall added a commit to BruceForstall/coreclr that referenced this pull request Aug 2, 2018
BruceForstall added a commit that referenced this pull request Aug 2, 2018
Revert "Build managed tests with dotnet (#19230)"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants