-
Notifications
You must be signed in to change notification settings - Fork 331
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
Moving Desktop,& Dotnet runtime providers to separate dll #659
Changes from 1 commit
fd93ee8
1b61e28
5cb7d2c
db3a328
335fdd8
160529d
701029e
3289513
e634f3f
3c5574e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,14 @@ | |
namespace Microsoft.VisualStudio.TestPlatform.Client | ||
{ | ||
using System; | ||
using System.IO; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
|
||
using Microsoft.VisualStudio.TestPlatform.Client.Discovery; | ||
using Microsoft.VisualStudio.TestPlatform.Client.Execution; | ||
using Microsoft.VisualStudio.TestPlatform.Common; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Hosting; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Logging; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Utilities; | ||
using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine; | ||
|
@@ -20,19 +21,20 @@ namespace Microsoft.VisualStudio.TestPlatform.Client | |
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities; | ||
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers; | ||
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Hosting; | ||
|
||
/// <summary> | ||
/// Implementation for TestPlatform | ||
/// </summary> | ||
public class TestPlatform : ITestPlatform | ||
{ | ||
private IFileHelper fileHelper; | ||
private readonly TestRuntimeProviderManager testHostProviderManager; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: rename, maybe ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do this in clean up PR. Have to rename lot of variables, & a few classes, & namespaces as well |
||
|
||
private readonly IFileHelper fileHelper; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="TestPlatform"/> class. | ||
/// </summary> | ||
public TestPlatform() : this(new TestEngine(), new FileHelper()) | ||
public TestPlatform() : this(new TestEngine(), new FileHelper(), TestRuntimeProviderManager.Instance) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm super confused: Whose responsibility is the TestRuntimeProvider? Not the engine's anymore? How would this look if another engine came in instead of the CrossPlatEngine? Is that even interesting anymore? We might need a doc for this please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The responsibility to provide TestRuntime, is moved to TestRuntimeProviderManager, which for a given session identifies all available TestRuntime, for instance (Desktop, DotNet), & then returns back a particular instance of Runtime, which would work for current settings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. And is this tied to the test engine in any way? What happens if a substitute engine wants to provide its own host process as a default?(say TAEF) Is that supported? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, If Taef wants to be the test runtime(i.e replace testhost), then they can provide their runtime whose job is to understand how to launch TAEF, once that is discovered, Taef can continue to host our crossplat engine to do the communication, & running test work, or they can implement their own communication framework which understands the protocol send across wire. |
||
{ | ||
this.testHostProviderManager = TestRuntimeProviderManager.Instance; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need this statement anymore. |
||
} | ||
|
@@ -43,19 +45,24 @@ public class TestPlatform : ITestPlatform | |
/// <param name="testEngine"> | ||
/// The test engine. | ||
/// </param> | ||
protected TestPlatform(ITestEngine testEngine, IFileHelper filehelper) | ||
/// <param name="filehelper"> | ||
/// The filehelper. | ||
/// </param> | ||
/// <param name="testHostProviderManager"> | ||
/// The data. | ||
/// </param> | ||
internal TestPlatform(ITestEngine testEngine, IFileHelper filehelper, TestRuntimeProviderManager testHostProviderManager) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not protected? We already use the Testable approach for the unit tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. resolved |
||
{ | ||
this.TestEngine = testEngine; | ||
this.fileHelper = filehelper; | ||
this.testHostProviderManager = testHostProviderManager; | ||
} | ||
|
||
/// <summary> | ||
/// Gets or sets Test Engine instance | ||
/// </summary> | ||
private ITestEngine TestEngine { get; set; } | ||
|
||
private TestRuntimeProviderManager testHostProviderManager; | ||
|
||
/// <summary> | ||
/// The create discovery request. | ||
/// </summary> | ||
|
@@ -66,16 +73,13 @@ public IDiscoveryRequest CreateDiscoveryRequest(DiscoveryCriteria discoveryCrite | |
{ | ||
if (discoveryCriteria == null) | ||
{ | ||
throw new ArgumentNullException("discoveryCriteria"); | ||
throw new ArgumentNullException(nameof(discoveryCriteria)); | ||
} | ||
|
||
UpdateTestAdapterPaths(discoveryCriteria.RunSettings); | ||
this.UpdateTestAdapterPaths(discoveryCriteria.RunSettings); | ||
|
||
var runconfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(discoveryCriteria.RunSettings); | ||
//var testHostManager = this.testHostProviderManager.GetTestHostManagerByRunConfiguration(runconfiguration); | ||
var testHostManager = this.testHostProviderManager.GetTestHostManagerByRunConfiguration(discoveryCriteria.RunSettings); | ||
|
||
var testHostManager = this.TestEngine.GetDefaultTestHostManager(runconfiguration); | ||
|
||
var discoveryManager = this.TestEngine.GetDiscoveryManager(testHostManager, discoveryCriteria); | ||
discoveryManager.Initialize(); | ||
|
||
|
@@ -92,26 +96,24 @@ public ITestRunRequest CreateTestRunRequest(TestRunCriteria testRunCriteria) | |
{ | ||
if (testRunCriteria == null) | ||
{ | ||
throw new ArgumentNullException("testRunCriteria"); | ||
throw new ArgumentNullException(nameof(testRunCriteria)); | ||
} | ||
|
||
UpdateTestAdapterPaths(testRunCriteria.TestRunSettings); | ||
this.UpdateTestAdapterPaths(testRunCriteria.TestRunSettings); | ||
|
||
var runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(testRunCriteria.TestRunSettings); | ||
|
||
//var testHostManager = this.testHostProviderManager.GetTestHostManagerByRunConfiguration(runConfiguration); | ||
|
||
// Update and initialize loggers only when DesignMode is false | ||
if (runConfiguration.DesignMode == false) | ||
{ | ||
UpdateTestLoggerPath(testRunCriteria); | ||
this.UpdateTestLoggerPath(testRunCriteria); | ||
|
||
// Initialize loggers | ||
TestLoggerManager.Instance.InitializeLoggers(); | ||
} | ||
|
||
var testHostManager = this.TestEngine.GetDefaultTestHostManager(runConfiguration); | ||
testHostManager.Initialize(TestSessionMessageLogger.Instance); | ||
var testHostManager = this.testHostProviderManager.GetTestHostManagerByRunConfiguration(testRunCriteria.TestRunSettings); | ||
testHostManager.Initialize(TestSessionMessageLogger.Instance, testRunCriteria.TestRunSettings); | ||
|
||
if (testRunCriteria.TestHostLauncher != null) | ||
{ | ||
|
@@ -159,6 +161,9 @@ public void UpdateExtensions(IEnumerable<string> pathToAdditionalExtensions, boo | |
/// <summary> | ||
/// Update the test adapter paths provided through run settings to be used by the test service | ||
/// </summary> | ||
/// <param name="runSettings"> | ||
/// The run Settings. | ||
/// </param> | ||
private void UpdateTestAdapterPaths(string runSettings) | ||
{ | ||
IEnumerable<string> customTestAdaptersPaths = RunSettingsUtilities.GetTestAdaptersPaths(runSettings); | ||
|
@@ -175,8 +180,10 @@ private void UpdateTestAdapterPaths(string runSettings) | |
} | ||
|
||
List<string> adapterFiles = new List<string>( | ||
this.fileHelper.EnumerateFiles(adapterPath, TestPlatformConstants.TestAdapterRegexPattern, SearchOption.AllDirectories) | ||
); | ||
this.fileHelper.EnumerateFiles( | ||
adapterPath, | ||
TestPlatformConstants.TestAdapterRegexPattern, | ||
SearchOption.AllDirectories)); | ||
if (adapterFiles.Count > 0) | ||
{ | ||
this.UpdateExtensions(adapterFiles, true); | ||
|
@@ -188,6 +195,9 @@ private void UpdateTestAdapterPaths(string runSettings) | |
/// <summary> | ||
/// Update the test logger paths from source directory | ||
/// </summary> | ||
/// <param name="testRunCriteria"> | ||
/// The test Run Criteria. | ||
/// </param> | ||
private void UpdateTestLoggerPath(TestRunCriteria testRunCriteria) | ||
{ | ||
IEnumerable<string> sources = testRunCriteria.Sources; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to keep the name generic? Should the name say that these are NET runtime providers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clean up