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

Conversation

smadala
Copy link
Contributor

@smadala smadala commented Jul 30, 2018

Description

  • Fix Settings Provider named 'GoogleTestAdapterSettings' was not found.
  • In Tpv1 we don't filter by SettingsProvider.dll More details
<?xml version="1.0" encoding="utf-8"?>
<RunSettings>
  <GoogleTestAdapterSettings>
    <SolutionSettings>
      <Settings>
        <AdditionalTestExecutionParam>-testdirectory=$(TestDir)</AdditionalTestExecutionParam>
        <BatchForTestSetup>$(SolutionDir)Tests\Returns0.bat</BatchForTestSetup>
        <BatchForTestTeardown>$(SolutionDir)Tests\Returns1.bat</BatchForTestTeardown>
        <WorkingDir>$(SolutionDir)</WorkingDir>
      </Settings>
    </SolutionSettings>
    <ProjectSettings>
      <Settings ProjectRegex="LoadTests_gta\.exe|CrashingTests_gta\.exe">
        <TestDiscoveryRegex>.*Tests.*_gta.exe</TestDiscoveryRegex>
      </Settings>
      <Settings ProjectRegex=".*\\Tests_gta\.exe">
        <TestDiscoveryTimeoutInSeconds>60</TestDiscoveryTimeoutInSeconds>
      </Settings>
    </ProjectSettings>
  </GoogleTestAdapterSettings>
</RunSettings>

Related issue

#1652
#1673

@@ -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 ?

/// <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

@@ -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

@smadala smadala merged commit aad2f43 into microsoft:master Jul 30, 2018
@smadala smadala deleted the fix-isettings-provider-02 branch July 30, 2018 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants