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

Find ISettingsProvider in TestAdapter assembly #1704

Merged
merged 1 commit into from
Jul 30, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Client/TestPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private void AddExtensionAssemblies(string runSettings)
continue;
}

var extensionAssemblies = new List<string>(this.fileHelper.EnumerateFiles(adapterPath, SearchOption.AllDirectories, TestPlatformConstants.TestAdapterEndsWithPattern, TestPlatformConstants.TestLoggerEndsWithPattern, TestPlatformConstants.RunTimeEndsWithPattern, TestPlatformConstants.SettingsProviderEndsWithPattern));
var extensionAssemblies = new List<string>(this.fileHelper.EnumerateFiles(adapterPath, SearchOption.AllDirectories, TestPlatformConstants.TestAdapterEndsWithPattern, TestPlatformConstants.TestLoggerEndsWithPattern, TestPlatformConstants.RunTimeEndsWithPattern));
Copy link
Contributor

Choose a reason for hiding this comment

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

This will also need to remain

if (extensionAssemblies.Count > 0)
{
this.UpdateExtensions(extensionAssemblies, skipExtensionFilters: false);
Expand Down
5 changes: 0 additions & 5 deletions src/Microsoft.TestPlatform.Common/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,5 @@ public static class TestPlatformConstants
/// Pattern used to find the run time providers library using String.EndWith
/// </summary>
public const string RunTimeEndsWithPattern = @"RuntimeProvider.dll";

/// <summary>
/// Pattern used to find the settings providers library using String.EndWith
/// </summary>
public const string SettingsProviderEndsWithPattern = @"SettingsProvider.dll";
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need to remain, if other comment is honored

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,26 @@ public List<string> GetExtensionPaths(string endsWithPattern, bool skipDefaultEx
{
var extensions = this.GetFilteredExtensions(this.filterableExtensionPaths, endsWithPattern);

if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose(
"TestPluginCache.GetExtensionPaths: Filtered extension paths: {0}", string.Join(Environment.NewLine, extensions));
}

if (!skipDefaultExtensions)
{
extensions = extensions.Concat(this.defaultExtensionPaths);
if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose(
"TestPluginCache.GetExtensionPaths: Added default extension paths: {0}", string.Join(Environment.NewLine, this.defaultExtensionPaths));
}
}

if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose(
"TestPluginCache.GetExtensionPaths: Added unfilterableExtensionPaths: {0}", string.Join(Environment.NewLine, this.unfilterableExtensionPaths));
}

return extensions.Concat(this.unfilterableExtensionPaths).ToList();
Expand All @@ -123,6 +140,7 @@ public List<string> GetExtensionPaths(string endsWithPattern, bool skipDefaultEx
public Dictionary<string, TPluginInfo> DiscoverTestExtensions<TPluginInfo, TExtension>(string endsWithPattern)
where TPluginInfo : TestPluginInformation
{
EqtTrace.Verbose("TestPluginCache.DiscoverTestExtensions: finding test extensions in assemblies endswith: {0} TPluginInfo: {1} TExtension: {2}", endsWithPattern, typeof(TPluginInfo), typeof(TExtension));
// Return the cached value if cache is valid.
if (this.TestExtensions != null && this.TestExtensions.AreTestExtensionsCached<TPluginInfo>())
{
Expand All @@ -142,11 +160,17 @@ public Dictionary<string, TPluginInfo> DiscoverTestExtensions<TPluginInfo, TExte

try
{
EqtTrace.Verbose("TestPluginCache: Discovering the extensions using extension path.");
EqtTrace.Verbose("TestPluginCache.DiscoverTestExtensions: Discovering the extensions using extension path.");

// Combine all the possible extensions - both default and additional.
var allExtensionPaths = this.GetExtensionPaths(endsWithPattern);

if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose(
"TestPluginCache.DiscoverTestExtensions: Discovering the extensions using allExtensionPaths: {0}", string.Join(Environment.NewLine, allExtensionPaths));
}

// Discover the test extensions from candidate assemblies.
pluginInfos = this.GetTestExtensions<TPluginInfo, TExtension>(allExtensionPaths);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static SettingsProviderExtensionManager Create()

TestPluginManager.Instance
.GetSpecificTestExtensions<TestSettingsProviderPluginInformation, ISettingsProvider, ISettingsProviderCapabilities, TestSettingsProviderMetadata>(
TestPlatformConstants.SettingsProviderEndsWithPattern,
TestPlatformConstants.TestAdapterEndsWithPattern,
Copy link
Contributor

Choose a reason for hiding this comment

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

TestAdapterEndsWithPattern [](start = 54, length = 26)

Should we keep SetttingsProvider pattern as well, in case(extreme rare) for folks who might have used this pattern to define their ISettings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating separate assembly for just to implement ISettingsProvider don't give better experience for adapter authors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please add tests for this change if possible ?

out unfilteredTestExtensions,
out testExtensions);

Expand Down