From 7e0d28b74011a1ee6346b6b8f0781135064218ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 30 Mar 2022 14:52:02 +0200 Subject: [PATCH] Fix new issues --- StringUtils.cs | 17 ++++++++ .../AcceptanceTestBase.cs | 2 +- .../BannedSymbols.txt | 4 +- .../IntegrationTestBase.cs | 42 +++++++++---------- .../IntegrationTestEnvironment.cs | 12 +++--- ...icrosoft.TestPlatform.TestUtilities.csproj | 4 ++ 6 files changed, 50 insertions(+), 31 deletions(-) create mode 100644 StringUtils.cs diff --git a/StringUtils.cs b/StringUtils.cs new file mode 100644 index 0000000000..edc97e2c3d --- /dev/null +++ b/StringUtils.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics.CodeAnalysis; + +namespace Microsoft.TestPlatform; + +internal static class StringUtils +{ + [SuppressMessage("ApiDesign", "RS0030:Do not used banned APIs", Justification = "Replacement API to allow nullable hints for compiler")] + public static bool IsNullOrEmpty([NotNullWhen(returnValue: false)] this string? value) + => string.IsNullOrEmpty(value); + + [SuppressMessage("ApiDesign", "RS0030:Do not used banned APIs", Justification = "Replacement API to allow nullable hints for compiler")] + public static bool IsNullOrWhiteSpace([NotNullWhen(returnValue: false)] this string? value) + => string.IsNullOrWhiteSpace(value); +} diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/AcceptanceTestBase.cs b/test/Microsoft.TestPlatform.AcceptanceTests/AcceptanceTestBase.cs index 934b7de8e6..244ecbc7a6 100644 --- a/test/Microsoft.TestPlatform.AcceptanceTests/AcceptanceTestBase.cs +++ b/test/Microsoft.TestPlatform.AcceptanceTests/AcceptanceTestBase.cs @@ -61,7 +61,7 @@ public class AcceptanceTestBase : IntegrationTestBase public const string LATEST_TO_LEGACY = "Latest;LatestPreview;LatestStable;RecentStable;MostDownloaded;PreviousStable;LegacyStable"; public const string LATESTPREVIEW_TO_LEGACY = "LatestPreview;LatestStable;RecentStable;MostDownloaded;PreviousStable;LegacyStable"; public const string LATEST = "Latest"; - public const string LATESTSTABLE= "LatestStable"; + public const string LATESTSTABLE = "LatestStable"; internal const string MSTEST = "MSTest"; public static string And(string left, string right) diff --git a/test/Microsoft.TestPlatform.TestUtilities/BannedSymbols.txt b/test/Microsoft.TestPlatform.TestUtilities/BannedSymbols.txt index c8eaef56e1..87ce187c16 100644 --- a/test/Microsoft.TestPlatform.TestUtilities/BannedSymbols.txt +++ b/test/Microsoft.TestPlatform.TestUtilities/BannedSymbols.txt @@ -1,3 +1,5 @@ M:System.IO.Path.GetTempPath(); Use 'IntegrationTestBase.GetTempPath()' instead M:System.Environment.SetEnvironmentVariable(System.String,System.String); Use one of the overload accepting a dictionary of environment variables instead -M:System.Environment.SetEnvironmentVariable(System.String,System.String,System.EnvironmentVariableTarget); Use one of the overload accepting a dictionary of environment variables instead \ No newline at end of file +M:System.Environment.SetEnvironmentVariable(System.String,System.String,System.EnvironmentVariableTarget); Use one of the overload accepting a dictionary of environment variables instead +M:System.String.IsNullOrEmpty(System.String); Use 'StringUtils.IsNullOrEmpty' instead +M:System.String.IsNullOrWhiteSpace(System.String); Use 'StringUtils.IsNullOrWhiteSpace' instead diff --git a/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs b/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs index de93b4a8fe..2530d9f2e9 100644 --- a/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs +++ b/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs @@ -88,7 +88,7 @@ public void TempDirectoryCleanup() // // Locally delete the directory only when the test succeeded, so we can look // at results and logs of failed tests. - if (IsCI || TestContext.CurrentTestOutcome == UnitTestOutcome.Passed) + if (IsCI || TestContext?.CurrentTestOutcome == UnitTestOutcome.Passed) { TempDirectory.Dispose(); } @@ -104,7 +104,7 @@ public void TempDirectoryCleanup() /// /// Command line arguments string. public static string PrepareArguments(string[] testAssemblies, string testAdapterPath, string runSettings, - string framework, string inIsolation = "", string? resultsDirectory = null) + string framework, string? inIsolation = "", string? resultsDirectory = null) { var arguments = ""; foreach (var path in testAssemblies) @@ -124,19 +124,19 @@ public static string PrepareArguments(string[] testAssemblies, string testAdapte arguments = arguments.Trim(); - if (!string.IsNullOrWhiteSpace(testAdapterPath)) + if (!testAdapterPath.IsNullOrWhiteSpace()) { // Append adapter path arguments = string.Concat(arguments, " /testadapterpath:", testAdapterPath.AddDoubleQuote()); } - if (!string.IsNullOrWhiteSpace(runSettings)) + if (!runSettings.IsNullOrWhiteSpace()) { // Append run settings arguments = string.Concat(arguments, " /settings:", runSettings.AddDoubleQuote()); } - if (!string.IsNullOrWhiteSpace(framework)) + if (!framework.IsNullOrWhiteSpace()) { // Append run settings arguments = string.Concat(arguments, " /framework:", framework.AddDoubleQuote()); @@ -144,7 +144,7 @@ public static string PrepareArguments(string[] testAssemblies, string testAdapte arguments = string.Concat(arguments, " /logger:", "console;verbosity=normal".AddDoubleQuote()); - if (!string.IsNullOrWhiteSpace(inIsolation)) + if (!inIsolation.IsNullOrWhiteSpace()) { if (inIsolation != "/InIsolation") { @@ -154,7 +154,7 @@ public static string PrepareArguments(string[] testAssemblies, string testAdapte arguments = string.Concat(arguments, " ", inIsolation); } - if (!string.IsNullOrWhiteSpace(resultsDirectory)) + if (!resultsDirectory.IsNullOrWhiteSpace()) { // Append results directory arguments = string.Concat(arguments, " /ResultsDirectory:", resultsDirectory.AddDoubleQuote()); @@ -173,7 +173,7 @@ public static string PrepareArguments(string[] testAssemblies, string testAdapte /// /// Command line arguments string. public static string PrepareArguments(string testAssembly, string testAdapterPath, string runSettings, - string framework, string inIsolation = "", string? resultsDirectory = null) + string framework, string? inIsolation = "", string? resultsDirectory = null) => PrepareArguments(new string[] { testAssembly }, testAdapterPath, runSettings, framework, inIsolation, resultsDirectory); @@ -192,7 +192,7 @@ public void InvokeVsTest(string arguments, Dictionary? environme /// Invokes our local copy of dotnet that is patched with artifacts from the build with specified arguments. /// /// Arguments provided to vstest.console.exe - public void InvokeDotnetTest(string arguments, Dictionary environmentVariables = null) + public void InvokeDotnetTest(string arguments, Dictionary? environmentVariables = null) { var debugEnvironmentVariables = AddDebugEnvironmentVariables(environmentVariables); @@ -228,7 +228,7 @@ public void InvokeVsTestForExecution(string testAssembly, InvokeVsTest(arguments, environmentVariables); } - private Dictionary AddDebugEnvironmentVariables(Dictionary environmentVariables) + private Dictionary AddDebugEnvironmentVariables(Dictionary? environmentVariables) { environmentVariables ??= new Dictionary(); @@ -295,7 +295,7 @@ public void ExecuteNotSupportedRunnerFrameworkTests(string runnerFramework, stri public void ValidateSummaryStatus(int passed, int failed, int skipped) { // TODO: Switch on the actual version of vstest console when we have that set on test environment. - if (_testEnvironment.VSTestConsoleInfo != null && _testEnvironment.VSTestConsoleInfo.Path.Contains($"{Path.DirectorySeparatorChar}15.")) + if (_testEnvironment.VSTestConsoleInfo?.Path?.Contains($"{Path.DirectorySeparatorChar}15.") == true) { ValidateSummaryStatusv15(passed, failed, skipped); return; @@ -612,14 +612,9 @@ public virtual string GetConsoleRunnerPath() if (IsDesktopRunner()) { - if (!string.IsNullOrWhiteSpace(_testEnvironment.VSTestConsoleInfo?.Path)) - { - consoleRunnerPath = _testEnvironment.VSTestConsoleInfo.Path; - } - else - { - consoleRunnerPath = Path.Combine(_testEnvironment.PublishDirectory, "vstest.console.exe"); - } + consoleRunnerPath = StringUtils.IsNullOrWhiteSpace(_testEnvironment.VSTestConsoleInfo?.Path) + ? Path.Combine(_testEnvironment.PublishDirectory, "vstest.console.exe") + : _testEnvironment.VSTestConsoleInfo.Path; } else if (IsNetCoreRunner()) { @@ -699,10 +694,10 @@ public IVsTestConsoleWrapper GetVsTestConsoleWrapper(TraceLevel traceLevel = Tra // variables, unless we explicitly say to clean them. https://github.com/microsoft/vstest/pull/3433 // Remove this code later, and just pass the variables you want to add. var debugEnvironmentVariables = AddDebugEnvironmentVariables(new Dictionary()); - Dictionary environmentVariables = new(); + Dictionary environmentVariables = new(); if (debugEnvironmentVariables.Count > 0) { - Environment.GetEnvironmentVariables().OfType().ToList().ForEach(e => environmentVariables.Add(e.Key.ToString(), e.Value.ToString())); + Environment.GetEnvironmentVariables().OfType().ToList().ForEach(e => environmentVariables.Add(e.Key.ToString()!, e.Value?.ToString())); foreach (var pair in debugEnvironmentVariables) { environmentVariables[pair.Key] = pair.Value; @@ -760,7 +755,8 @@ protected void ExecuteVsTestConsole(string args, out string stdOut, out string s /// /// /// - private void ExecutePatchedDotnet(string command, string args, out string stdOut, out string stdError, out int exitCode, Dictionary environmentVariables = null) + private void ExecutePatchedDotnet(string command, string args, out string stdOut, out string stdError, out int exitCode, + Dictionary? environmentVariables = null) { if (environmentVariables is null) { @@ -777,7 +773,7 @@ private void ExecutePatchedDotnet(string command, string args, out string stdOut protected static void ExecuteApplication(string path, string args, out string stdOut, out string stdError, out int exitCode, Dictionary? environmentVariables = null, string? workingDirectory = null) { - if (string.IsNullOrWhiteSpace(path)) + if (path.IsNullOrWhiteSpace()) { throw new ArgumentException("Executable path must not be null or whitespace.", nameof(path)); } diff --git a/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs b/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs index be39dacd48..4bf3e6d7a7 100644 --- a/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs +++ b/test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs @@ -28,13 +28,13 @@ public class IntegrationTestEnvironment public IntegrationTestEnvironment() { // If the variables are not set, valid defaults are assumed. - if (string.IsNullOrEmpty(TargetFramework)) + if (TargetFramework.IsNullOrEmpty()) { // Run integration tests for net451 by default. TargetFramework = "net451"; } - if (string.IsNullOrEmpty(TestPlatformRootDirectory)) + if (TestPlatformRootDirectory.IsNullOrEmpty()) { // Running in VS/IDE. Use artifacts directory as root. // Get root directory from test assembly output directory @@ -118,7 +118,7 @@ public string? TargetRuntime { if (RunnerFramework == IntegrationTestBase.DesktopRunnerFramework) { - if (string.IsNullOrEmpty(_targetRuntime)) + if (_targetRuntime.IsNullOrEmpty()) { _targetRuntime = "win7-x64"; } @@ -165,8 +165,8 @@ public string? TargetRuntime // A known AzureDevOps env variable meaning we are running in CI. public static bool IsCI { get; } = Environment.GetEnvironmentVariable("TF_BUILD") == "True"; - public DebugInfo DebugInfo { get; set; } - public VSTestConsoleInfo VSTestConsoleInfo { get; set; } + public DebugInfo? DebugInfo { get; set; } + public VSTestConsoleInfo? VSTestConsoleInfo { get; set; } public List DllInfos { get; set; } = new(); /// @@ -258,7 +258,7 @@ private static Dictionary GetDependencies(string testPlatformRoo props.Read(); // Read thru the PropertyGroup node while (!props.EOF) { - if (props.IsStartElement() && !string.IsNullOrEmpty(props.Name)) + if (props.IsStartElement() && !props.Name.IsNullOrEmpty()) { if (!dependencyProps.ContainsKey(props.Name)) { diff --git a/test/Microsoft.TestPlatform.TestUtilities/Microsoft.TestPlatform.TestUtilities.csproj b/test/Microsoft.TestPlatform.TestUtilities/Microsoft.TestPlatform.TestUtilities.csproj index 527a9a44b3..874507d07d 100644 --- a/test/Microsoft.TestPlatform.TestUtilities/Microsoft.TestPlatform.TestUtilities.csproj +++ b/test/Microsoft.TestPlatform.TestUtilities/Microsoft.TestPlatform.TestUtilities.csproj @@ -16,6 +16,10 @@ + + + +