Forward merge fixes from master to rc2 (#2581)

* Avoid logging >Task returned false but did not log an error.< (#2557)

* Avoid logging >Task returned false but did not log an error.< on test failure

* Add VSTEST_BUILD_DEBUG env var

* Using namespaces

* Invert the switch because it will be still backwards in the final release

* Use bitness from process or OS (#2571)

* Do not force .NET4.5 in case legacy test settings are provided (#2545)

* Do not force .NET4.5 in case legacy test settings are provided

* Net core app

* Fix runconfig

* Default platform

Author: nohwnd

src/Microsoft.TestPlatform.Build/Tasks/VSTestTask.cs

@@ -5,7 +5,9 @@ namespace Microsoft.TestPlatform.Build.Tasks
 {
     using System;
     using System.Collections.Generic;
+    using System.Diagnostics;
     using System.Linq;
+    using System.Threading;
     using Microsoft.Build.Framework;
     using Microsoft.Build.Utilities;
     using Microsoft.TestPlatform.Build.Resources;
@@ -161,6 +163,28 @@ public override bool Execute()
             var traceEnabledValue = Environment.GetEnvironmentVariable("VSTEST_BUILD_TRACE");
             Tracing.traceEnabled = !string.IsNullOrEmpty(traceEnabledValue) && traceEnabledValue.Equals("1", StringComparison.OrdinalIgnoreCase);
 
+            var debugEnabled = Environment.GetEnvironmentVariable("VSTEST_BUILD_DEBUG");
+            if (!string.IsNullOrEmpty(debugEnabled) && debugEnabled.Equals("1", StringComparison.Ordinal))
+            {
+                Console.WriteLine("Waiting for debugger attach...");
+
+                var currentProcess = Process.GetCurrentProcess();
+                Console.WriteLine(string.Format("Process Id: {0}, Name: {1}", currentProcess.Id, currentProcess.ProcessName));
+
+                while (!Debugger.IsAttached)
+                {
+                    Thread.Sleep(1000);
+                }
+
+                Debugger.Break();
+            }
+
+            // Avoid logging "Task returned false but did not log an error." on test failure, because we don't
+            // write MSBuild error. https://github.com/dotnet/msbuild/blob/51a1071f8871e0c93afbaf1b2ac2c9e59c7b6491/src/Framework/IBuildEngine7.cs#L12
+            var allowfailureWithoutError = BuildEngine.GetType().GetProperty("AllowFailureWithoutError");
+            // setting this to false because the switch is implemented backwards and it won't be fixed till next release
+            allowfailureWithoutError?.SetValue(BuildEngine, false);
+
             vsTestForwardingApp = new VSTestForwardingApp(this.VSTestConsolePath, this.CreateArgument());
             if (!string.IsNullOrEmpty(this.VSTestFramework))
             {

src/Microsoft.TestPlatform.Utilities/MSTestSettingsUtilities.cs

@@ -25,12 +25,10 @@ public static class MSTestSettingsUtilities
         /// Settings file which need to be imported. The file extension of the settings file will be specified by <paramref name="SettingsFileExtension"/> property.
         /// </param>
         /// <param name="defaultRunSettings"> Input RunSettings document to which settings file need to be imported. </param>
-        /// <param name="architecture"> The architecture. </param>
-        /// <param name="frameworkVersion"> The framework Version. </param>
         /// <returns> Updated RunSetting Xml document with imported settings. </returns>
         [SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver",
             Justification = "XmlDocument.XmlResolver is not available in core. Suppress until fxcop issue is fixed.")]
-        public static XmlDocument Import(string settingsFile, XmlDocument defaultRunSettings, Architecture architecture, FrameworkVersion frameworkVersion)
+        public static XmlDocument Import(string settingsFile, XmlDocument defaultRunSettings)
         {
             ValidateArg.NotNull(settingsFile, "settingsFile");
             ValidateArg.NotNull(defaultRunSettings, "defaultRunSettings");
@@ -57,14 +55,6 @@ public static XmlDocument Import(string settingsFile, XmlDocument defaultRunSett
                 var doc = new XmlDocument();
                 var runConfigurationNode = doc.CreateElement(Constants.RunConfigurationSettingsName);
 
-                var targetPlatformNode = doc.CreateElement("TargetPlatform");
-                targetPlatformNode.InnerXml = architecture.ToString();
-                runConfigurationNode.AppendChild(targetPlatformNode);
-
-                var targetFrameworkVersionNode = doc.CreateElement("TargetFrameworkVersion");
-                targetFrameworkVersionNode.InnerXml = frameworkVersion.ToString();
-                runConfigurationNode.AppendChild(targetFrameworkVersionNode);
-
                 defaultRunSettings.DocumentElement.PrependChild(defaultRunSettings.ImportNode(runConfigurationNode, true));
             }
 

src/vstest.console/Processors/RunSettingsArgumentProcessor.cs

@@ -199,7 +199,7 @@ private XmlDocument GetRunSettingsDocument(string runSettingsFile)
             else
             {
                 runSettingsDocument = XmlRunSettingsUtilities.CreateDefaultRunSettings();
-                runSettingsDocument = MSTestSettingsUtilities.Import(runSettingsFile, runSettingsDocument, Architecture.X86, FrameworkVersion.Framework45);
+                runSettingsDocument = MSTestSettingsUtilities.Import(runSettingsFile, runSettingsDocument);
             }
 
             return runSettingsDocument;

src/vstest.console/TestPlatformHelpers/TestRequestManager.cs

@@ -451,7 +451,19 @@ private bool UpdateRunSettingsIfRequired(string runsettingsXml, List<string> sou
                         || chosenFramework.Name.IndexOf("netcoreapp", StringComparison.OrdinalIgnoreCase) >= 0
                         || chosenFramework.Name.IndexOf("net5", StringComparison.OrdinalIgnoreCase) >= 0)
                     {
-                        defaultArchitecture =  Environment.Is64BitOperatingSystem ? Architecture.X64 : Architecture.X86;
+#if NETCOREAPP
+                        // We are running in vstest.console that is either started via dotnet.exe or via vstest.console.exe .NET Core 
+                        // executable. For AnyCPU dlls this should resolve 32-bit SDK when running from 32-bit dotnet process and 
+                        // 64-bit SDK when running from 64-bit dotnet process.
+                        defaultArchitecture = Environment.Is64BitProcess ? Architecture.X64 : Architecture.X86;
+#else
+                        // We are running in vstest.console.exe that was built against .NET Framework. This console prefers 32-bit
+                        // because it needs to run as 32-bit to be compatible with QTAgent. It runs as 32-bit both under VS and 
+                        // in Developer console. Set the default architecture based on the OS architecture, to find 64-bit dotnet SDK
+                        // when running AnyCPU dll on 64-bit system, and 32-bit SDK when running AnyCPU dll on 32-bit OS.
+                        // We want to find 64-bit SDK because it is more likely to be installed.
+                        defaultArchitecture = Environment.Is64BitOperatingSystem ? Architecture.X64 : Architecture.X86;
+#endif
                     }
 
                     settingsUpdated |= this.UpdatePlatform(document, navigator, sources, sourcePlatforms, defaultArchitecture, out Architecture chosenPlatform);

test/Microsoft.TestPlatform.Utilities.UnitTests/MSTestSettingsUtilitiesTests.cs

@@ -8,7 +8,6 @@ namespace Microsoft.TestPlatform.Utilities.Tests
 
     using Microsoft.VisualStudio.TestPlatform.Utilities;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
-    using VisualStudio.TestPlatform.ObjectModel;
     using MSTest.TestFramework.AssertExtensions;
 
     [TestClass]
@@ -49,9 +48,7 @@ public void ImportShouldThrowIfNotLegacySettingsFile()
                 () =>
                 MSTestSettingsUtilities.Import(
                     "C:\\temp\\r.runsettings",
-                    xmlDocument,
-                    Architecture.X86,
-                    FrameworkVersion.Framework45);
+                    xmlDocument);
             Assert.That.Throws<XmlException>(action).WithMessage("Unexpected settings file specified.");
         }
 
@@ -66,9 +63,7 @@ public void ImportShouldThrowIfDefaultRunSettingsIsIncorrect()
                 () =>
                 MSTestSettingsUtilities.Import(
                     "C:\\temp\\r.testsettings",
-                    xmlDocument,
-                    Architecture.X86,
-                    FrameworkVersion.Framework45);
+                    xmlDocument);
             Assert.That.Throws<XmlException>(action).WithMessage("Could not find 'RunSettings' node.");
         }
 
@@ -80,14 +75,18 @@ public void ImportShouldEmbedTestSettingsInformation()
             xmlDocument.LoadXml(defaultRunSettingsXml);
             var finalxPath = MSTestSettingsUtilities.Import(
                 "C:\\temp\\r.testsettings",
-                xmlDocument,
-                Architecture.X86,
-                FrameworkVersion.Framework45);
+                xmlDocument);
 
             var finalSettingsXml = finalxPath.CreateNavigator().OuterXml;
 
             var expectedSettingsXml =
-                "<RunSettings>\r\n  <MSTest>\r\n    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n  </MSTest>\r\n  <RunConfiguration></RunConfiguration>\r\n</RunSettings>";
+                "<RunSettings>\r\n" +
+                "  <MSTest>\r\n" +
+                "    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n" +
+                "    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n" +
+                "  </MSTest>\r\n" +
+                "  <RunConfiguration></RunConfiguration>\r\n" +
+                "</RunSettings>";
 
             Assert.AreEqual(expectedSettingsXml, finalSettingsXml);
         }
@@ -100,14 +99,18 @@ public void ImportShouldEmbedTestSettingsAndDefaultRunConfigurationInformation()
             xmlDocument.LoadXml(defaultRunSettingsXml);
             var finalxPath = MSTestSettingsUtilities.Import(
                 "C:\\temp\\r.testsettings",
-                xmlDocument,
-                Architecture.X86,
-                FrameworkVersion.Framework45);
+                xmlDocument);
 
             var finalSettingsXml = finalxPath.CreateNavigator().OuterXml;
 
             var expectedSettingsXml =
-                "<RunSettings>\r\n  <RunConfiguration>\r\n    <TargetPlatform>X86</TargetPlatform>\r\n    <TargetFrameworkVersion>Framework45</TargetFrameworkVersion>\r\n  </RunConfiguration>\r\n  <MSTest>\r\n    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n  </MSTest>\r\n</RunSettings>";
+                "<RunSettings>\r\n" +
+                "  <RunConfiguration />\r\n" +
+                "  <MSTest>\r\n" +
+                "    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n" +
+                "    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n" +
+                "  </MSTest>\r\n" +
+                "</RunSettings>";
 
             Assert.AreEqual(expectedSettingsXml, finalSettingsXml);
         }

test/vstest.console.UnitTests/Processors/RunSettingsArgumentProcessorTests.cs

@@ -235,9 +235,27 @@ public void InitializeShouldSetActiveRunSettingsForTestSettingsFiles()
             // Act.
             executor.Initialize(fileName);
 
+
             // Assert.
             Assert.IsNotNull(this.settingsProvider.ActiveRunSettings);
-            StringAssert.Contains(this.settingsProvider.ActiveRunSettings.SettingsXml, $"<?xml version=\"1.0\" encoding=\"utf-16\"?>\r\n<RunSettings>\r\n  <RunConfiguration>\r\n    <TargetPlatform>{Constants.DefaultPlatform}</TargetPlatform>\r\n    <TargetFrameworkVersion>{Framework.FromString(FrameworkVersion.Framework45.ToString()).Name}</TargetFrameworkVersion>\r\n    <ResultsDirectory>{Constants.DefaultResultsDirectory}</ResultsDirectory>\r\n  </RunConfiguration>\r\n  <MSTest>\r\n    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n  </MSTest>\r\n  <DataCollectionRunSettings>\r\n    <DataCollectors />\r\n  </DataCollectionRunSettings>\r\n</RunSettings>");
+
+            var expected = 
+                $"<?xml version=\"1.0\" encoding=\"utf-16\"?>\r\n" +
+                $"<RunSettings>\r\n" +
+                $"  <RunConfiguration>\r\n" +
+                $"    <ResultsDirectory>{Constants.DefaultResultsDirectory}</ResultsDirectory>\r\n" +
+                $"    <TargetPlatform>{Constants.DefaultPlatform}</TargetPlatform>\r\n" +
+                $"    <TargetFrameworkVersion>{Framework.DefaultFramework.Name}</TargetFrameworkVersion>\r\n" +
+                $"  </RunConfiguration>\r\n" +
+                $"  <MSTest>\r\n" +
+                $"    <SettingsFile>C:\\temp\\r.testsettings</SettingsFile>\r\n" +
+                $"    <ForcedLegacyMode>true</ForcedLegacyMode>\r\n" +
+                $"  </MSTest>\r\n" +
+                $"  <DataCollectionRunSettings>\r\n" +
+                $"    <DataCollectors />\r\n" +
+                $"  </DataCollectionRunSettings>\r\n" +
+                $"</RunSettings>";
+            StringAssert.Contains(this.settingsProvider.ActiveRunSettings.SettingsXml, expected);
         }