Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing runsettings as command line arguments. #297

Merged
merged 19 commits into from
Dec 27, 2016
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Copyright (c) .NET Foundation. All rights reserved.
VSTestLogger="$(VSTestLogger)"
VSTestListTests="$(VSTestListTests)"
VSTestDiag="$(VSTestDiag)"
VSTestCLIRunSettings="$(VSTestCLIRunSettings)"
/>
</Target>

Expand Down Expand Up @@ -72,6 +73,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<Message Text="VSTestLogger = $(VSTestLogger)" Importance="low" />
<Message Text="VSTestListTests = $(VSTestListTests)" Importance="low" />
<Message Text="VSTestDiag = $(VSTestDiag)" Importance="low" />
<Message Text="VSTestCLIRunSettings = $(VSTestCLIRunSettings)" Importance="low" />
</Target>

</Project>
12 changes: 12 additions & 0 deletions src/Microsoft.TestPlatform.Build/Tasks/VSTestTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public string VSTestDiag
set;
}

public string[] VSTestCLIRunSettings
{
get;
set;
}

public override bool Execute()
{
var traceEnabledValue = Environment.GetEnvironmentVariable("VSTEST_BUILD_TRACE");
Expand Down Expand Up @@ -159,6 +165,12 @@ private IEnumerable<string> CreateArgument()
}
}

if (this.VSTestCLIRunSettings!=null && this.VSTestCLIRunSettings.Length>0)
{
allArgs.Add("--");
allArgs.AddRange(this.VSTestCLIRunSettings);
}

return allArgs;
}
}
Expand Down
14 changes: 12 additions & 2 deletions src/vstest.console/CommandLine/Executor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal class Executor
/// <summary>
/// Default constructor.
/// </summary>
public Executor(IOutput output): this(output, TestPlatformEventSource.Instance)
public Executor(IOutput output) : this(output, TestPlatformEventSource.Instance)
{
}

Expand Down Expand Up @@ -152,8 +152,18 @@ private int GetArgumentProcessors(string[] args, out List<IArgumentProcessor> pr
int result = 0;
var processorFactory = ArgumentProcessorFactory.Create();

foreach (string arg in args)
for (var index = 0; index < args.Length; index++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test in Acceptance Tests project.

{
var arg = args[index];

// If argument is '--', following arguments are key=value pairs for run settings.
if (arg.Equals("--"))
{
var cliRunSettingsProcessor = processorFactory.CreateArgumentProcessor(arg, args.Skip(index + 1).ToArray());
processors.Add(cliRunSettingsProcessor);
break;
}

var processor = processorFactory.CreateArgumentProcessor(arg);

if (processor != null)
Expand Down
226 changes: 226 additions & 0 deletions src/vstest.console/Processors/CLIRunSettingsArgumentProcessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors
{
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Contracts;
using System.Globalization;
using System.IO;
using System.Net;
using System.Text.RegularExpressions;
using System.Xml;
using System.Xml.XPath;

using Microsoft.VisualStudio.TestPlatform.Common;
using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
using Microsoft.VisualStudio.TestPlatform.Utilities;
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers;
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;

using CommandLineResources = Microsoft.VisualStudio.TestPlatform.CommandLine.Resources.Resources;

/// <summary>
/// The argument processor for runsettings passed as argument through cli
/// </summary>
internal class CLIRunSettingsArgumentProcessor : IArgumentProcessor
{
#region Constants

/// <summary>
/// The name of the command line argument that the PortArgumentExecutor handles.
/// </summary>
public const string CommandName = "--";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you try passing --- for CLIRunSettingsArgument, I think it will work two. Because at the time of creating BuildCommandMaps( https://github.com/Microsoft/vstest/blob/master/src/vstest.console/Processors/Utilities/ArgumentProcessorFactory.cs#L226) we replace first character of CommandName with "--" because to create xplat command. All the CommandName is starting with "/", shouldn't we have something similar??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having "/" is good to have, but I am not sure if we need it or not.
@codito Any thought on this?

Also, as mentioned by Faizan, we are also getting "---" by default, do we need to remove it?

Copy link
Contributor

@codito codito Dec 23, 2016

Choose a reason for hiding this comment

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

/ will create confusion with root path on unix. -- looks good to me.

All the CommandName is starting with "/",

We can keep supporting it for back compact but, (IMO) we should remove this from help text for simplicity.


#endregion

private Lazy<IArgumentProcessorCapabilities> metadata;

private Lazy<IArgumentExecutor> executor;

/// <summary>
/// Gets the metadata.
/// </summary>
public Lazy<IArgumentProcessorCapabilities> Metadata
{
get
{
if (this.metadata == null)
{
this.metadata = new Lazy<IArgumentProcessorCapabilities>(() => new CLIRunSettingsArgumentProcessorCapabilities());
}

return this.metadata;
}
}

/// <summary>
/// Gets or sets the executor.
/// </summary>
public Lazy<IArgumentExecutor> Executor
{
get
{
if (this.executor == null)
{
this.executor = new Lazy<IArgumentExecutor>(() => new CLIRunSettingsArgumentExecutor(RunSettingsManager.Instance));
}

return this.executor;
}

set
{
this.executor = value;
}
}
}

internal class CLIRunSettingsArgumentProcessorCapabilities : BaseArgumentProcessorCapabilities
{
public override string CommandName => CLIRunSettingsArgumentProcessor.CommandName;

public override bool AllowMultiple => false;

public override bool IsAction => false;

public override ArgumentProcessorPriority Priority => ArgumentProcessorPriority.CLIRunSettings;

public override string HelpContentResourceName => CommandLineResources.CLIRunSettingsArgumentHelp;

public override HelpContentPriority HelpPriority => HelpContentPriority.CLIRunSettingsArgumentProcessorHelpPriority;
}

internal class CLIRunSettingsArgumentExecutor : IArgumentsExecutor
{
private IRunSettingsProvider runSettingsManager;

internal CLIRunSettingsArgumentExecutor(IRunSettingsProvider runSettingsManager)
{
this.runSettingsManager = runSettingsManager;
}

public void Initialize(string argument)
{
throw new NotImplementedException();
}

public void Initialize(string[] arguments)
{
// if argument is null or white space, don't do anything.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think white space from comment should be removed.

if (arguments == null || arguments.Length == 0)
{
return;
}

Contract.EndContractBlock();

// Load up the run settings and set it as the active run settings.
try
{
var doc = new XmlDocument();

if (this.runSettingsManager.ActiveRunSettings != null && !string.IsNullOrEmpty(this.runSettingsManager.ActiveRunSettings.SettingsXml))
{
var settingsXml = this.runSettingsManager.ActiveRunSettings.SettingsXml;

#if net46
using (var reader = XmlReader.Create(new StringReader(settingsXml), new XmlReaderSettings() { XmlResolver = null, CloseInput = true, DtdProcessing = DtdProcessing.Prohibit }))
{
#else
using (var reader = XmlReader.Create(new StringReader(settingsXml), new XmlReaderSettings() { CloseInput = true, DtdProcessing = DtdProcessing.Prohibit }))
{
#endif
doc.Load(reader);
}
}
else
{
#if net46
doc = (XmlDocument)XmlRunSettingsUtilities.CreateDefaultRunSettings();
#else
using (var reader = XmlReader.Create(new StringReader(XmlRunSettingsUtilities.CreateDefaultRunSettings().CreateNavigator().OuterXml), new XmlReaderSettings() { CloseInput = true, DtdProcessing = DtdProcessing.Prohibit }))
{
doc.Load(reader);
}
#endif
}

// Append / Override run settings supplied in CLI
CreateOrOverwriteRunSettings(doc, arguments);

// Set Active Run Settings.
var runSettings = new RunSettings();
runSettings.LoadSettingsXml(doc.OuterXml);
this.runSettingsManager.SetActiveRunSettings(runSettings);
}
catch (SettingsException exception)
{
throw new CommandLineException(exception.Message, exception);
}
}

public ArgumentProcessorResult Execute()
{
// Nothing to do here, the work was done in initialization.
return ArgumentProcessorResult.Success;
}

private void CreateOrOverwriteRunSettings(XmlDocument xmlDoc, string[] args)
{
var length = args.Length;

for (int index = 0; index < length; index++)
{
var keyValuePair = args[index];
var indexOfSeparator = keyValuePair.IndexOf("=");
if (indexOfSeparator <= 0|| indexOfSeparator>=keyValuePair.Length-1)
{
continue;
}
var key = keyValuePair.Substring(0, indexOfSeparator);
var value = keyValuePair.Substring(indexOfSeparator + 1);

if (string.IsNullOrWhiteSpace(key))
{
continue;
}

// Check if the key exists.
var xPath = key.Replace('.', '/');
var node = xmlDoc.SelectSingleNode(string.Format("//RunSettings/{0}", xPath));

if (node == null)
{
node = CreateNode(xmlDoc, key.Split('.'));
}

node.InnerText = WebUtility.HtmlEncode(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not encode value in HTML as our reader is not aware of that. We need this encoder just to pass value from dotnet test to vstest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@codito : all the runsettings reader are not aware that xml values could be encoded.
IMO as well, we should not encode values for now.
Any thoughts on this?

}
}

private XmlNode CreateNode(XmlDocument doc, string[] xPath)
{
XmlNode node = null;
XmlNode parent = doc.DocumentElement;

for (int i = 0; i < xPath.Length; i++)
{
node = parent.SelectSingleNode(xPath[i]);

if (node == null)
{
node = parent.AppendChild(doc.CreateElement(xPath[i]));
}

parent = node;
}

return node;
}
}
}
18 changes: 18 additions & 0 deletions src/vstest.console/Processors/Interfaces/IArgumentsExecutor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors
{
/// <summary>
/// Defines interface for interacting with a command line arguments executor.
/// Items exporting this interface will be used in processing command line arguments.
/// </summary>
internal interface IArgumentsExecutor : IArgumentExecutor
{
/// <summary>
/// Initializes the Argument Processor with the arguments that was provided with the command.
/// </summary>
/// <param name="arguments">Arguments that are provided with the command.</param>
void Initialize(string[] arguments);
}
}
Loading