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

Fix for changes in XPathNavigator for netcoreapp2.0 #1226

Merged
merged 11 commits into from
Oct 27, 2017
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,8 @@ private static string AddDefaultRunSettings(string runSettings)
var document = new XmlDocument();
document.Load(reader);

var navigator = document.CreateNavigator();

InferRunSettingsHelper.UpdateRunSettingsWithUserProvidedSwitches(navigator, architecture, framework, defaultResultsDirectory);
return navigator.OuterXml;
InferRunSettingsHelper.UpdateRunSettingsWithUserProvidedSwitches(document, architecture, framework, defaultResultsDirectory);
return document.OuterXml;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public static Dictionary<string, object> GetTestRunParameters(string settingsXml
/// </returns>
[SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver",
Justification = "XmlReaderSettings.XmlResolver is not available in core. Suppress until fxcop issue is fixed.")]
public static IXPathNavigable CreateDefaultRunSettings()
public static XmlDocument CreateDefaultRunSettings()
{
// Create a new default xml doc that looks like this:
// <?xml version="1.0" encoding="utf-8"?>
Expand All @@ -209,11 +209,7 @@ public static IXPathNavigable CreateDefaultRunSettings()
var dataCollectorsNode = doc.CreateElement(Constants.DataCollectorsSettingName);
dataCollectionRunSettingsNode.AppendChild(dataCollectorsNode);

#if NET451
return doc;
#else
return doc.ToXPathNavigable();
#endif
}

/// <summary>
Expand Down
88 changes: 42 additions & 46 deletions src/Microsoft.TestPlatform.Utilities/InferRunSettingsHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static string MakeRunsettingsCompatible(string runsettingsXml)
{
EqtTrace.Error("InferRunSettingsHelper.MakeRunsettingsCompatible: Unable to navigate to RunConfiguration. Current node: " + runSettingsNavigator.LocalName);
}
else if(runSettingsNavigator.HasChildren)
else if (runSettingsNavigator.HasChildren)
{
var listOfInValidRunConfigurationSettings = new List<string>();

Expand All @@ -90,7 +90,7 @@ public static string MakeRunsettingsCompatible(string runsettingsXml)
runSettingsNavigator.MoveToFirstChild();
do
{
if(!listOfValidRunConfigurationSettings.Contains(runSettingsNavigator.LocalName))
if (!listOfValidRunConfigurationSettings.Contains(runSettingsNavigator.LocalName))
{
listOfInValidRunConfigurationSettings.Add(runSettingsNavigator.LocalName);
}
Expand All @@ -100,7 +100,7 @@ public static string MakeRunsettingsCompatible(string runsettingsXml)
// Delete all invalid RunConfiguration Settings
if (listOfInValidRunConfigurationSettings.Count > 0)
{
if(EqtTrace.IsWarningEnabled)
if (EqtTrace.IsWarningEnabled)
{
string settingsName = string.Join(", ", listOfInValidRunConfigurationSettings);
EqtTrace.Warning(string.Format("InferRunSettingsHelper.MakeRunsettingsCompatible: Removing the following settings: {0} from RunSettings file. To use those settings please move to latest version of Microsoft.NET.Test.Sdk", settingsName));
Expand All @@ -109,7 +109,7 @@ public static string MakeRunsettingsCompatible(string runsettingsXml)
// move navigator to RunConfiguration node
runSettingsNavigator.MoveToParent();

foreach(var s in listOfInValidRunConfigurationSettings)
foreach (var s in listOfInValidRunConfigurationSettings)
{
var nodePath = RunConfigurationNodePath + "/" + s;
XmlUtilities.RemoveChildNode(runSettingsNavigator, nodePath, s);
Expand All @@ -129,12 +129,14 @@ public static string MakeRunsettingsCompatible(string runsettingsXml)
/// <summary>
/// Updates the run settings XML with the specified values.
/// </summary>
/// <param name="runSettingsNavigator"> The navigator of the XML. </param>
/// <param name="architecture"> The architecture. </param>
/// <param name="framework"> The framework. </param>
/// <param name="resultsDirectory"> The results directory. </param>
public static void UpdateRunSettingsWithUserProvidedSwitches(XPathNavigator runSettingsNavigator, Architecture architecture, Framework framework, string resultsDirectory)
/// <param name="runSettingsDocument">XmlDocument of runsettings xml. </param>
/// <param name="architecture">The architecture. </param>
/// <param name="framework">The framework. </param>
/// <param name="resultsDirectory">The results directory. </param>
public static void UpdateRunSettingsWithUserProvidedSwitches(XmlDocument runSettingsDocument, Architecture architecture, Framework framework, string resultsDirectory)
{
var runSettingsNavigator = runSettingsDocument.CreateNavigator();

ValidateRunConfiguration(runSettingsNavigator);

// when runsettings specifies platform, that takes precedence over the user specified platform via command line arguments.
Expand Down Expand Up @@ -164,67 +166,65 @@ public static void UpdateRunSettingsWithUserProvidedSwitches(XPathNavigator runS
VerifyCompatibilityWithOSArchitecture(architecture);

// Check if inputRunSettings has results directory configured.
var hasResultsDirectory = runSettingsNavigator.SelectSingleNode(ResultsDirectoryNodePath) != null;
var hasResultsDirectory = runSettingsDocument.SelectSingleNode(ResultsDirectoryNodePath) != null;

// Regenerate the effective settings.
if (shouldUpdatePlatform || shouldUpdateFramework || !hasResultsDirectory)
{
UpdateRunConfiguration(runSettingsNavigator, architecture, framework, resultsDirectory);
UpdateRunConfiguration(runSettingsDocument, architecture, framework, resultsDirectory);
}

runSettingsNavigator.MoveToRoot();
}

/// <summary>
/// Updates the <c>RunConfiguration.DesignMode</c> value for a run settings. Doesn't do anything if the value is already set.
/// </summary>
/// <param name="runSettingsNavigator">Navigator for runsettings xml</param>
/// <param name="runSettingsDocument">XmlDocument for runsettings xml</param>
/// <param name="designModeValue">Value to set</param>
public static void UpdateDesignMode(XPathNavigator runSettingsNavigator, bool designModeValue)
public static void UpdateDesignMode(XmlDocument runSettingsDocument, bool designModeValue)
{
AddNodeIfNotPresent<bool>(runSettingsNavigator, DesignModeNodePath, DesignModeNodeName, designModeValue);
AddNodeIfNotPresent<bool>(runSettingsDocument, DesignModeNodePath, DesignModeNodeName, designModeValue);
}

/// <summary>
/// Updates the <c>RunConfiguration.CollectSourceInformation</c> value for a run settings. Doesn't do anything if the value is already set.
/// </summary>
/// <param name="runSettingsNavigator">Navigator for runsettings xml</param>
/// <param name="runSettingsDocument">XmlDocument for runsettings xml</param>
/// <param name="collectSourceInformationValue">Value to set</param>
public static void UpdateCollectSourceInformation(XPathNavigator runSettingsNavigator, bool collectSourceInformationValue)
public static void UpdateCollectSourceInformation(XmlDocument runSettingsDocument, bool collectSourceInformationValue)
{
AddNodeIfNotPresent<bool>(runSettingsNavigator, CollectSourceInformationNodePath, CollectSourceInformationNodeName, collectSourceInformationValue);
AddNodeIfNotPresent<bool>(runSettingsDocument, CollectSourceInformationNodePath, CollectSourceInformationNodeName, collectSourceInformationValue);
}

/// <summary>
/// Updates the <c>RunConfiguration.TargetDevice</c> value for a run settings. Doesn't do anything if the value is already set.
/// </summary>
/// <param name="runSettingsNavigator">Navigator for runsettings xml</param>
/// <param name="runSettingsDocument">XmlDocument for runsettings xml</param>
/// <param name="targetDevice">Value to set</param>
public static void UpdateTargetDevice(XPathNavigator runSettingsNavigator, string targetDevice)
public static void UpdateTargetDevice(XmlDocument runSettingsDocument, string targetDevice)
{
AddNodeIfNotPresent<string>(runSettingsNavigator, TargetDeviceNodePath, TargetDevice, targetDevice);
AddNodeIfNotPresent<string>(runSettingsDocument, TargetDeviceNodePath, TargetDevice, targetDevice);
}

/// <summary>
/// Updates the <c>RunConfiguration.TargetFrameworkVersion</c> value for a run settings. if the value is already set, behavior depends on overwrite.
/// </summary>
/// <param name="runSettingsNavigator">Navigator for runsettings xml</param>
/// <param name="runSettingsDocument">XmlDocument for runsettings xml</param>
/// <param name="framework">Value to set</param>
/// <param name="overwrite">Overwrite option.</param>
public static void UpdateTargetFramework(XPathNavigator runSettingsNavigator, string framework, bool overwrite=false)
public static void UpdateTargetFramework(XmlDocument runSettingsDocument, string framework, bool overwrite = false)
{
AddNodeIfNotPresent<string>(runSettingsNavigator, TargetFrameworkNodePath, TargetFrameworkNodeName, framework, overwrite);
AddNodeIfNotPresent<string>(runSettingsDocument, TargetFrameworkNodePath, TargetFrameworkNodeName, framework, overwrite);
}

/// <summary>
/// Updates the <c>RunConfiguration.TargetPlatform</c> value for a run settings. if the value is already set, behavior depends on overwrite.
/// </summary>
/// <param name="runSettingsNavigator">Navigator for runsettings xml</param>
/// <param name="runSettingsDocument">XmlDocument for runsettings xml</param>
/// <param name="platform">Value to set</param>
/// <param name="overwrite">Overwrite option.</param>
public static void UpdateTargetPlatform(XPathNavigator runSettingsNavigator, string platform, bool overwrite = false)
public static void UpdateTargetPlatform(XmlDocument runSettingsDocument, string platform, bool overwrite = false)
{
AddNodeIfNotPresent<string>(runSettingsNavigator, TargetPlatformNodePath, TargetPlatformNodeName, platform, overwrite);
AddNodeIfNotPresent<string>(runSettingsDocument, TargetPlatformNodePath, TargetPlatformNodeName, platform, overwrite);
}

public static bool TryGetDeviceXml(XPathNavigator runSettingsNavigator, out String deviceXml)
Expand Down Expand Up @@ -267,7 +267,7 @@ public static bool IsTestSettingsEnabled(string runsettingsXml)
}

var node = runSettingsNavigator.SelectSingleNode(@"/RunSettings/MSTest/SettingsFile");
if(node != null && !string.IsNullOrEmpty(node.InnerXml))
if (node != null && !string.IsNullOrEmpty(node.InnerXml))
{
return true;
}
Expand All @@ -280,21 +280,21 @@ public static bool IsTestSettingsEnabled(string runsettingsXml)
/// <summary>
/// Adds node under RunConfiguration setting. Noop if node is already present.
/// </summary>
private static void AddNodeIfNotPresent<T>(XPathNavigator runSettingsNavigator, string nodePath, string nodeName, T nodeValue, bool overwrite = false)
private static void AddNodeIfNotPresent<T>(XmlDocument xmlDocument, string nodePath, string nodeName, T nodeValue, bool overwrite = false)
{
// Navigator should be at Root of runsettings xml, attempt to move to /RunSettings/RunConfiguration
if (!runSettingsNavigator.MoveToChild(RunSettingsNodeName, string.Empty) ||
!runSettingsNavigator.MoveToChild(RunConfigurationNodeName, string.Empty))
var root = xmlDocument.DocumentElement;

if (root.SelectSingleNode(string.Format("//{0}/{1}", RunSettingsNodeName, RunConfigurationNodeName)) == null)
{
EqtTrace.Error("InferRunSettingsHelper.UpdateNodeIfNotPresent: Unable to navigate to RunConfiguration. Current node: " + runSettingsNavigator.LocalName);
EqtTrace.Error("InferRunSettingsHelper.UpdateNodeIfNotPresent: Unable to navigate to RunConfiguration. Current node: " + xmlDocument.LocalName);
return;
}

var node = runSettingsNavigator.SelectSingleNode(nodePath);
var node = xmlDocument.SelectSingleNode(nodePath);
if (node == null || overwrite)
{
XmlUtilities.AppendOrModifyChild(runSettingsNavigator, nodePath, nodeName, nodeValue.ToString());
runSettingsNavigator.MoveToRoot();
XmlUtilities.AppendOrModifyChild(xmlDocument, nodePath, nodeName, nodeValue.ToString());
}
}

Expand Down Expand Up @@ -370,26 +370,22 @@ private static void VerifyCompatibilityWithOSArchitecture(Architecture architect
/// Regenerates the RunConfiguration node with new values under runsettings.
/// </summary>
private static void UpdateRunConfiguration(
XPathNavigator navigator,
XmlDocument xmlDocument,
Architecture effectivePlatform,
Framework effectiveFramework,
string resultsDirectory)
{
var resultsDirectoryNavigator = navigator.SelectSingleNode(ResultsDirectoryNodePath);
var resultsDirectoryNavigator = xmlDocument.SelectSingleNode(ResultsDirectoryNodePath);
if (null != resultsDirectoryNavigator)
{
resultsDirectory = resultsDirectoryNavigator.InnerXml;
}

XmlUtilities.AppendOrModifyChild(navigator, RunConfigurationNodePath, RunConfigurationNodeName, null);
navigator.MoveToChild(RunConfigurationNodeName, string.Empty);

XmlUtilities.AppendOrModifyChild(navigator, ResultsDirectoryNodePath, ResultsDirectoryNodeName, resultsDirectory);

XmlUtilities.AppendOrModifyChild(navigator, TargetPlatformNodePath, TargetPlatformNodeName, effectivePlatform.ToString());
XmlUtilities.AppendOrModifyChild(navigator, TargetFrameworkNodePath, TargetFrameworkNodeName, effectiveFramework.ToString());
XmlUtilities.AppendOrModifyChild(xmlDocument, RunConfigurationNodePath, RunConfigurationNodeName, null);
XmlUtilities.AppendOrModifyChild(xmlDocument, ResultsDirectoryNodePath, ResultsDirectoryNodeName, resultsDirectory);

navigator.MoveToRoot();
XmlUtilities.AppendOrModifyChild(xmlDocument, TargetPlatformNodePath, TargetPlatformNodeName, effectivePlatform.ToString());
XmlUtilities.AppendOrModifyChild(xmlDocument, TargetFrameworkNodePath, TargetFrameworkNodeName, effectiveFramework.ToString());
}

public static bool TryGetPlatformXml(XPathNavigator runSettingsNavigator, out string platformXml)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
<Reference Include="Microsoft.CSharp" />
<Reference Include="System.Xml" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.4' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

Not ideal :-/ This will require all the nuget packages to declare an extra dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove it.

<PackageReference Include="System.Xml.XPath.XmlDocument">
<Version>4.0.1</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
<Compile Update="Resources\Resources.Designer.cs">
<DesignTime>True</DesignTime>
Expand Down
26 changes: 5 additions & 21 deletions src/Microsoft.TestPlatform.Utilities/XmlUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ internal static bool IsValidNodeXmlValue(string xmlNodeValue, Func<string, bool>
[SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver",
Justification = "XmlDocument.XmlResolver is not available in core. Suppress until fxcop issue is fixed.")]
internal static void AppendOrModifyChild(
XPathNavigator parentNavigator,
XmlDocument xmlDocument,
string nodeXPath,
string nodeName,
string innerXml)
{
var childNodeNavigator = parentNavigator.SelectSingleNode(nodeXPath);
var childNodeNavigator = xmlDocument.SelectSingleNode(nodeXPath);

// Todo: There isn't an equivalent API to SecurityElement.Escape in Core yet.
// So trusting that the XML is always valid for now.
Expand All @@ -67,35 +67,19 @@ internal static void AppendOrModifyChild(
#endif
if (childNodeNavigator == null)
{
var doc = new XmlDocument();
var childElement = doc.CreateElement(nodeName);
var childElement = xmlDocument.CreateElement(nodeName);

if (!string.IsNullOrEmpty(innerXml))
{
childElement.InnerXml = secureInnerXml;
}

childNodeNavigator = childElement.CreateNavigator();
parentNavigator.AppendChild(childNodeNavigator);
var parentNode = xmlDocument.SelectSingleNode(nodeXPath.Substring(0, nodeXPath.LastIndexOf('/')));
parentNode.AppendChild(childElement);
}
else if (!string.IsNullOrEmpty(innerXml))
{
#if NET451
childNodeNavigator.InnerXml = secureInnerXml;
#else
// .Net Core has a bug where calling childNodeNavigator.InnerXml throws an XmlException with "Data at the root level is invalid".
// So doing the below instead.
var doc = new XmlDocument();

var childElement = doc.CreateElement(nodeName);

if (!string.IsNullOrEmpty(innerXml))
{
childElement.InnerXml = secureInnerXml;
}

childNodeNavigator.ReplaceSelf(childElement.CreateNavigator().OuterXml);
#endif
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,7 @@ private void UpdateWithCodeCoverageSettingsIfNotConfigured()
{
var document = new XmlDocument();
document.Load(reader);
#if NET451
runSettingsDocument = document;
#else
runSettingsDocument = document.ToXPathNavigable();
#endif
}

var runSettingsNavigator = runSettingsDocument.CreateNavigator();
Expand Down
4 changes: 0 additions & 4 deletions src/vstest.console/Processors/RunSettingsArgumentProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ private IXPathNavigable GetRunSettingsDocument(string runSettingsFile)
var settingsDocument = new XmlDocument();
settingsDocument.Load(reader);
ClientUtilities.FixRelativePathsInRunSettings(settingsDocument, runSettingsFile);
#if NET451
runSettingsDocument = settingsDocument;
#else
runSettingsDocument = settingsDocument.ToXPathNavigable();
#endif
}
}
else
Expand Down
Loading