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

C++ UWP Reflection based discovery #1336

Merged
merged 7 commits into from
Dec 21, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 4 additions & 2 deletions src/Microsoft.TestPlatform.ObjectModel/TestCase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel
using System.IO;
using System.Runtime.Serialization;

using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
using System.Globalization;
using System.Collections.Generic;
using System.Linq;

using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;

/// <summary>
/// Stores information about a test case.
/// </summary>
Expand Down Expand Up @@ -221,7 +223,7 @@ private Guid GetTestId()
// For example in the database adapter case this is not a file path.
string source = this.Source;

if (File.Exists(source))
if (PlatformFile.Exists(source))
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, but I do not want to create a new Instance of FileHelper to check for file Exists. Correct thing to do would be to move it out of coreutilities, & make it a static class in platform abstraction, but that would require a lot of tests changes.

I'm still thinking on how to do it better.

{
source = Path.GetFileName(source);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.PlatformAbstractions
{
using System.IO;

/// <summary>
/// File Abstraction
/// </summary>
public static class PlatformFile
{
/// <summary>
/// Checks if give file exists on disk
/// </summary>
/// <param name="filePath">input filePath</param>
/// <returns>True if file Exists</returns>
public static bool Exists(string filePath)
{
return File.Exists(filePath);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// 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.PlatformAbstractions
{
using System;

/// <summary>
/// File Abstraction
/// </summary>
public static class PlatformFile
{
/// <summary>
/// Checks if give file exists on disk
/// </summary>
/// <param name="filePath">input filePath</param>
/// <returns>True if file Exists</returns>
public static bool Exists(string filePath)
{
throw new NotImplementedException(filePath);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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.PlatformAbstractions
{
using System.IO;

/// <summary>
/// File Abstraction
/// </summary>
public static class PlatformFile
{
/// <summary>
/// Checks if give file exists on disk
/// </summary>
/// <param name="filePath">input filePath</param>
/// <returns>True if file Exists</returns>
public static bool Exists(string filePath)
{
// Appxrecipe gets renamed vs.appxrecipe, which test platform is unaware of, so if queried for appxrecipe, return true
if (filePath.EndsWith(".appxrecipe", System.StringComparison.OrdinalIgnoreCase))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is business logic getting in platform layer :) Would the consumer of platform APIs expect such filtering?

Copy link
Contributor Author

@mayankbansal018 mayankbansal018 Dec 19, 2017

Choose a reason for hiding this comment

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

This is the most unfortunate hack I'm introducing since we have to go back to doing UWP discovery in fullCLR process.

{
return true;
}

return File.Exists(filePath);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using Microsoft.TestPlatform.TestHostProvider.Hosting;
using Microsoft.TestPlatform.TestHostProvider.Resources;
using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions;
Expand Down Expand Up @@ -203,8 +204,21 @@ public IEnumerable<string> GetTestPlatformExtensions(IEnumerable<string> sources
/// <inheritdoc/>
public IEnumerable<string> GetTestSources(IEnumerable<string> sources)
{
// We do not have scenario where full CLR tests are deployed to remote machine, so no need to udpate sources
return sources;
List<string> actualSources = new List<string>();

// We are doing this specifically for UWP, should we extract it out to some other utility?
Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely. This code takes us back couple of years, albeit the if used to be #if in those days.

Copy link
Contributor Author

@mayankbansal018 mayankbansal018 Dec 19, 2017

Choose a reason for hiding this comment

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

I'm thinking of moving this to UWP provider, & invoke it via reflection so it always owns the ownership of giving us the correct exe, but that would still mean Desktop Provider would need to understand what all packages types it can support, however it need not understand the logic to extract sources from that package. Thoughts?

// Why? Lets say if we have to do same for someother source extension, would we just add another if check?
var uwpSources = sources.Where(source => source.EndsWith(".appxrecipe", StringComparison.OrdinalIgnoreCase));

foreach (var uwpSource in uwpSources)
{
XmlDocument xmlDocument = new XmlDocument();
xmlDocument.Load(XmlReader.Create(new StringReader(File.ReadAllText(uwpSource, Encoding.UTF8)), XmlRunSettingsUtilities.ReaderSettings));

actualSources.Add(Path.Combine(Path.GetDirectoryName(uwpSource), xmlDocument.GetElementsByTagName("PackagePath").Cast<XmlNode>().Where(node => node.InnerText.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)).FirstOrDefault().InnerText));
}

return actualSources.Concat(sources.Except(uwpSources));
}

/// <inheritdoc/>
Expand Down