Skip to content
This repository was archived by the owner on Apr 20, 2023. It is now read-only.

Conversation

@rainersigwald
Copy link
Member

Supersedes #8854.

This version has dotnet/msbuild#3126, which should resolve the TF problems from #8854.

@livarcocc
Copy link

Should this go in 15.7 (release/2.1.2xx) as well? Or there we should maintain 108?

@rainersigwald
Copy link
Member Author

@dotnet-bot test this please

(Forgot to make sure our packages were pushed to MyGet before starting this, sorry)

@livarcocc
Copy link

@dotnet-bot Test CentOS7.1 x64 Debug Build

@livarcocc
Copy link

@rainersigwald @dsplaisted there is some regression here affecting windows only. There are two tests who have been failing complaining about a direct reference to M.NC.App. Can you guys check? It is happening on the other msbuild insertion on 2.1.2xx as well.

@dsplaisted
Copy link
Member

The actual test failure is that the output doesn't contain strings such as Total tests: 3. Passed: 2. Failed: 1. Skipped: 0.. I'm not sure if the warning about the direct reference is related.

@rainersigwald rainersigwald force-pushed the AndyGerlicher-MSBuild-15.7.108 branch from c6d5dfa to fc97cd5 Compare March 24, 2018 17:15
@rainersigwald
Copy link
Member Author

The failure is due to microsoft/vstest#1503. Basically, the test infrastructure assumed that writes to stdout from the test-runner process would be piped all the way out through the top-level dotnet invocation, but that's not true any more--because the process may have been launched by a long-lived worker node that has no stdout.

There's a workaround: set MSBUILDNODEWINDOW, which keeps MSBuild from starting the worker nodes disconnected, and use /nr:false to ensure that tests never connect to a preexisting worker node. I propose to apply that now and take vstest fix before release.

@rainersigwald
Copy link
Member Author

I see a couple of options:

  • just break test 😞
  • revert node reuse 😞
  • figure out a smaller change msbuild-side to plumb stdout through again (not sure how easy this would be)
  • just fix VSTest (they're saying they did this on purpose to colorize output, which isn't good)

AndyGerlicher and others added 2 commits March 24, 2018 18:51
Work around microsoft/vstest#1503 by using the
MSBuild escape hatch variable MSBUILDENSURESTDOUTFORTASKPROCESSES and
ensuring that tests don't run in a disconnected MSBuild process by
passing /nr:false.
@rainersigwald rainersigwald force-pushed the AndyGerlicher-MSBuild-15.7.108 branch from fc97cd5 to d98928e Compare March 24, 2018 23:53
@rainersigwald
Copy link
Member Author

Just pushed a change that goes with option 3: a new escape hatch + opting into it here. It won't work until dotnet/msbuild#3130 is merged + packaged, but is that something you're amenable to @livarcocc?

@rainersigwald
Copy link
Member Author

this does work e2e, by the way (manually injected new MSBuild into CLI built from this CL):

s:\work\vstestmulti>dotnet msbuild /version
Microsoft (R) Build Engine version 15.7.123.48545 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

15.7.123.48545
s:\work\vstestmulti>dotnet test
s:\work\vstestmulti\VSTestDesktopAndNetCore.csproj : warning NU1604: Project dependency Microsoft.NET.Test.Sdk does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
Build started, please wait...
s:\work\vstestmulti\VSTestDesktopAndNetCore.csproj : warning NU1604: Project dependency Microsoft.NET.Test.Sdk does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
s:\work\vstestmulti\VSTestDesktopAndNetCore.csproj : warning NU1604: Project dependency Microsoft.NET.Test.Sdk does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
Build completed.

Test run for s:\work\vstestmulti\bin\Debug\net46\VSTestDesktopAndNetCore.dll(.NETFramework,Version=v4.6)
Microsoft (R) Test Execution Command Line Tool Version 15.7.0-preview-20180221-13
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
Failed   VSTestFailTest
Error Message:
 Assert.Fail failed.
Stack Trace:
   at TestNamespace.VSTestTests.VSTestFailTest() in s:\work\vstestmulti\Tests.cs:line 19


Total tests: 3. Passed: 2. Failed: 1. Skipped: 0.
Test Run Failed.
Test execution time: 1.7360 Seconds
Test run for s:\work\vstestmulti\bin\Debug\netcoreapp2.1\VSTestDesktopAndNetCore.dll(.NETCoreApp,Version=v2.1)
Microsoft (R) Test Execution Command Line Tool Version 15.7.0-preview-20180221-13
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
Failed   VSTestFailTest
Error Message:
 Assert.Fail failed.
Stack Trace:
   at TestNamespace.VSTestTests.VSTestFailTest() in s:\work\vstestmulti\Tests.cs:line 19

Failed   VSTestFailTestNetCoreApp
Error Message:
 Assert.Fail failed.
Stack Trace:
   at TestNamespace.VSTestTests.VSTestFailTestNetCoreApp() in s:\work\vstestmulti\Tests.cs:line 31


Total tests: 3. Passed: 1. Failed: 2. Skipped: 0.
Test Run Failed.
Test execution time: 1.6528 Seconds

@AndyGerlicher
Copy link
Collaborator

The updated MSBuild has an updated NuGet reference in the resolver. Had to pull in the NuGet insertion that went into 2.1.2xx to match MSBuild's assembly version to avoid a crossgen failure.

@AndyGerlicher
Copy link
Collaborator

@dotnet-bot test Windows_NT_ES x64 Debug Build please

@AndyGerlicher AndyGerlicher changed the title MSBuild 15.7.108 for 2.1.3xx MSBuild 15.7.124 for 2.1.3xx Mar 26, 2018
@livarcocc livarcocc added this to the 2.1.3xx milestone Mar 26, 2018
@livarcocc livarcocc merged commit b5a163c into dotnet:release/2.1.3xx Mar 26, 2018
@rainersigwald rainersigwald deleted the AndyGerlicher-MSBuild-15.7.108 branch April 11, 2018 14:05
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