Allow toolset that tests run against to be controlled by command-line options #1674

dsplaisted · 2017-10-20T15:33:34Z

Tests no longer need to be run from within an enlistment of dotnet/sdk. You should be able to build the tests, then copy the test folder (bin\Debug\Tests) and the TestAssets folder to another machine and run the tests there.
Command-line options can be used to specify what "toolset" to run against:

  -useFullMSBuild         : Use full framework (instead of .NET Core) version of MSBuild found in PATH
  -fullMSBuildPath <path> : Use full framework version of MSBuild in specified path
  -dotnetPath <path>      : Use specified path for dotnet host
  -sdkRepo <path>         : Use specified SDK repo for Microsoft.NET.SDK tasks / targets
  -sdkConfig <config>     : Use specified configuration for SDK repo
  -noRepoInference        : Don't automatically find SDK repo to use based on path to test binaries
  -help                   : Show help

livarcocc · 2017-10-20T19:24:43Z

@tannergooding please review this change and validate that it does not break onboarding repotoolset.

tannergooding · 2017-10-20T19:47:19Z

build/Targets/Test.targets

@@ -9,7 +9,7 @@
  </PropertyGroup>

  <ItemGroup Condition="'$(OutputType)' == 'Exe' ">
-    <Compile Include="$(MSBuildThisFileDirectory)TestProgram.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)TestProgram.cs" Visible="false" />


I don't think this should be 'Visible=false'

It is nice to be able to view this file and open it from within VS.

tannergooding · 2017-10-20T19:47:46Z

build/Targets/TestProgram.cs


 class Program
 {
    public static int Main(string[] args)
    {
-        var newArgs = args.ToList();
-        newArgs.Insert(0, typeof(Program).Assembly.Location);
+        var newArgs = TestCommandLine.HandleCommandLine(args, out bool showHelp);


I think Environment.GetCommandLineArgs() achieves the same thing and would work here.

tannergooding

LGTM. I was about to do something similar to finish resolving the hard coded dependency that the tests had.

There will be some minimal conflicts, but they should be easy for me to resolve.

…not using repo version

…ainst

… values

nguerrera · 2017-10-23T21:31:37Z

src/Tasks/Common/CommandLine/NOPCommandLine.cs

+
+namespace Microsoft.NET.TestFramework
+{
+    public class TestCommandLine


Let's name the file TestCommandLine.Default.cs instead of NOPCommandLine.cs

nguerrera · 2017-10-23T21:39:21Z

test/Microsoft.NET.Build.Tests/DeleteNuGetArtifactsFixture.cs

+            get
+            {
+                return Path.Combine(TestContext.Current.TestExecutionDirectory,
+                                    TestDirectoriesNamePrefix + NuGetSharedDirectoryNamePostfix);


It's unfortunately pervasive in CLI/SDK code to do stuff like this in property getters, but we shouldn't produce fresh strings on every property access. If I review calling code that calls this more than once, I'm not going to imagine that to be problematic, because it looks like field access. Now, this is test code, so my care factor is not high, but it's something that I'm going to be pushing back on in product code.

nguerrera · 2017-10-23T21:40:52Z

test/Microsoft.NET.TestFramework/Commands/NuGetRestoreCommand.cs

+                throw new InvalidOperationException("Path to nuget.exe not set");
+            }
+
+            SdkCommandSpec ret = new SdkCommandSpec()


nit: var since type is obvious

Also, can we name this spec instead of ret.

You have ret all over the place. I won't comment on all of them, but I'd prefer readable names.

nguerrera · 2017-10-23T21:41:57Z

test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs

+
+        public string WorkingDirectory { get; set; }
+
+        string EscapeArgs()


private static

nguerrera · 2017-10-23T21:42:18Z

test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs

+
+        public ProcessStartInfo ToProcessStartInfo()
+        {
+            ProcessStartInfo ret = new ProcessStartInfo();


nguerrera · 2017-10-23T21:42:40Z

test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs

+        public ICommand ToCommand()
+        {
+            var commandSpec = new CommandSpec(FileName, EscapeArgs(), CommandResolutionStrategy.Path);
+            ICommand ret = Command.Create(commandSpec);


nguerrera · 2017-10-23T21:42:50Z

test/Microsoft.NET.TestFramework/Commands/SdkCommandSpec.cs

+        }
+    }
+
+


extra whitespace

tannergooding · 2017-10-26T15:54:47Z

Is this currently blocked on anything? I need this to go in to finish up the repo-toolset changes.

…727.15 (#1674) [main] Update dependencies from dotnet/arcade

dsplaisted added 3 commits October 19, 2017 22:50

Refactor test commands to support getting ProcessStartInfo

cb31a20

Support getting ProcessStartInfo from TestCommand

b01345a

Remove workaround for dotnet/msbuild#1428

a1a3bd6

dsplaisted requested review from nguerrera and livarcocc October 20, 2017 15:33

dnfclas added the cla-already-signed label Oct 20, 2017

tannergooding reviewed Oct 20, 2017

View reviewed changes

tannergooding approved these changes Oct 20, 2017

View reviewed changes

dsplaisted added 13 commits October 20, 2017 16:53

Refactor RepoInfo into TestContext and ToolsetInfo

cd2d76f

Refactor functionality from MSBuildTest to ToolsetInfo

3b5f29b

Only read DOTNET_SDK_TEST_MSBUILD_PATH environment variable in one place

70609a4

Allow toolset under test to be controlled by command line options

7fc48ff

Add option to use full framework MSBuild by resolving it from PATH

cfe1a09

Add command-line help

b1d6aba

Fix tests which rely on the Microsoft.NET.Build.Extensions path when …

0e1c941

…not using repo version

Workaround for running tests against bootstrapped MSBuild

9e3005e

Try to find nuget.exe for tests even when not using a repo to test ag…

ed5ee78

…ainst

Fix search for NuGetCachePath

f5e3f4b

Fix running tests in test explorer

5379489

Use unique output folders for framework targeting test

7710328

Fix issue where test environment variables wouldn't override existing…

259a032

… values

dsplaisted force-pushed the test-drop-2.0.0 branch from c22ab54 to 259a032 Compare October 20, 2017 23:55

nguerrera approved these changes Oct 23, 2017

View reviewed changes

Apply code review feedback

11dc9c1

dsplaisted merged commit 3174e2a into dotnet:release/2.0.0 Oct 26, 2017

JL03-Yue pushed a commit that referenced this pull request Mar 19, 2024

Update dependencies from https://github.com/dotnet/arcade build 20220…

a38206a

…727.15 (#1674) [main] Update dependencies from dotnet/arcade

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow toolset that tests run against to be controlled by command-line options #1674

Allow toolset that tests run against to be controlled by command-line options #1674

dsplaisted commented Oct 20, 2017

livarcocc commented Oct 20, 2017

tannergooding Oct 20, 2017

tannergooding Oct 20, 2017

tannergooding left a comment

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

nguerrera Oct 23, 2017

tannergooding commented Oct 26, 2017


		public string WorkingDirectory { get; set; }

		string EscapeArgs()

Allow toolset that tests run against to be controlled by command-line options #1674

Allow toolset that tests run against to be controlled by command-line options #1674

Conversation

dsplaisted commented Oct 20, 2017

livarcocc commented Oct 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding commented Oct 26, 2017