Multi-target XHarness at .NET 3.1 and 6.0 (#516)

- Adds the .NET 6.0 target to XHarness CLI
  - We need both first so that Helix SDK doesn't break, switch it there and remove 3.1 after
- Changes libraries from `netstandard2.1` to `net6.0` and multitargets to both where needed (Apple, Android)
- Fixes warnings that would break the build
  - WebClient is deprecated
  - Nullability issues
- Resolves most of the build messages to clean the solution
  - Naming
  - Static methods where possible
  - Calling `GC.SuppressFinalize`
  - using `new ()`

#456

azure-pipelines.yml

@@ -80,7 +80,7 @@ stages:
           - task: ComponentGovernanceComponentDetection@0
             displayName: Component Governance scan
             inputs:
-              ignoreDirectories: '$(Build.SourcesDirectory)/.packages,$(Build.SourcesDirectory)/artifacts/obj/Microsoft.DotNet.XHarness.CLI/$(_BuildConfig)/netcoreapp3.1/android-tools-unzipped'
+              ignoreDirectories: '$(Build.SourcesDirectory)/.packages,$(Build.SourcesDirectory)/artifacts/obj/Microsoft.DotNet.XHarness.CLI/$(_BuildConfig)/net6.0/android-tools-unzipped'
 
 - ${{ if eq(variables._RunAsPublic, True) }}:
   - stage: Build_OSX

eng/download-mlaunch.ps1

@@ -4,17 +4,17 @@
 
     Revision is cached in a temp dir and re-used for new builds.
 #>
-[CmdletBinding(PositionalBinding=$false)]
+[CmdletBinding(PositionalBinding = $false)]
 Param(
     [ValidateNotNullOrEmpty()]
-    [Parameter(Mandatory=$True)]
+    [Parameter(Mandatory = $True)]
     [string] $Commit,
 
     [ValidateNotNullOrEmpty()]
-    [Parameter(Mandatory=$True)]
+    [Parameter(Mandatory = $True)]
     [string] $TargetDir,
 
-    [switch] $RemoveSymbols=$False
+    [switch] $RemoveSymbols = $False
 )
 
 function Copy-Mlaunch([string] $sourceDir, [string] $targetDir, [bool] $removeSymbols) {
@@ -37,6 +37,8 @@ function Copy-Mlaunch([string] $sourceDir, [string] $targetDir, [bool] $removeSy
 
 New-Item -Path $TargetDir -ItemType Directory -ErrorAction SilentlyContinue
 
+$ErrorActionPreference = 'Stop'
+
 Write-Host "Getting mlaunch revision $commit into $TargetDir"
 
 $tagFile = Join-Path $TargetDir "$commit.tag"

eng/download-mlaunch.sh

@@ -15,27 +15,29 @@ set -e
 source="${BASH_SOURCE[0]}"
 # resolve $source until the file is no longer a symlink
 while [[ -h "$source" ]]; do
-  scriptroot="$( cd -P "$( dirname "$source" )" && pwd )"
+  script_root="$( cd -P "$( dirname "$source" )" && pwd )"
   source="$(readlink "$source")"
   # if $source was a relative symlink, we need to resolve it relative to the path where the
   # symlink file was located
-  [[ $source != /* ]] && source="$scriptroot/$source"
+  [[ $source != /* ]] && source="$script_root/$source"
 done
-scriptroot="$( cd -P "$( dirname "$source" )" && pwd )"
 
-. "$scriptroot/common/tools.sh"
+script_root="$( cd -P "$( dirname "$source" )" && pwd )"
+
+# shellcheck source=./common/tools.sh
+source "$script_root/common/tools.sh"
 
 copy_mlaunch () {
   # Copy mlaunch to the target folder
-  cp -Rv $1 $2
+  cp -Rv "$1" "$2"
 
   # Clean what we don't need
   rm -rf "$2/lib/mlaunch/mlaunch.app/Contents/MacOS/mlaunch.dSYM"
 
   if [ "$3" = true ]; then
     echo "Removing debug symbols"
-    rm -v $2/lib/mlaunch/mlaunch.app/Contents/MonoBundle/*.pdb
-    rm -v $2/lib/mlaunch/mlaunch.app/Contents/MonoBundle/*.mdb
+    rm -v "$2"/lib/mlaunch/mlaunch.app/Contents/MonoBundle/*.pdb
+    rm -v "$2"/lib/mlaunch/mlaunch.app/Contents/MonoBundle/*.mdb
   fi
 }
 
@@ -44,7 +46,7 @@ target_dir="$artifacts_dir/mlaunch"
 remove_symbols=false
 
 while (($# > 0)); do
-  lowerI="$(echo $1 | awk '{print tolower($0)}')"
+  lowerI="$(echo "$1" | awk '{print tolower($0)}')"
   case $lowerI in
     --commit)
       shift

src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs

@@ -25,9 +25,11 @@ public class AdbRunner
         private readonly string _absoluteAdbExePath;
         private readonly ILogger _log;
         private readonly IAdbProcessManager _processManager;
-        private readonly Dictionary<string, string> CommandList = new Dictionary<string, string>{
-        { "architecture", "shell getprop ro.product.cpu.abi"},
-        { "app", "shell pm list packages -3"} }; 
+        private readonly Dictionary<string, string> _commandList = new()
+        {
+            { "architecture", "shell getprop ro.product.cpu.abi"},
+            { "app", "shell pm list packages -3"}
+        }; 
 
 
         public AdbRunner(ILogger log, string adbExePath = "") : this(log, new AdbProcessManager(log), adbExePath) { }
@@ -40,7 +42,7 @@ public AdbRunner(ILogger log, IAdbProcessManager processManager, string adbExePa
             _processManager = processManager ?? throw new ArgumentNullException(nameof(processManager));
 
             // We need to find ADB.exe somewhere
-            string environmentPath = Environment.GetEnvironmentVariable(AdbEnvironmentVariableName);
+            string? environmentPath = Environment.GetEnvironmentVariable(AdbEnvironmentVariableName);
             if (!string.IsNullOrEmpty(environmentPath))
             {
                 _log.LogDebug($"Using {AdbEnvironmentVariableName} environment variable ({environmentPath}) for ADB path.");
@@ -114,7 +116,7 @@ public void DumpAdbLog(string outputFilePath, string filterSpec = "")
             }
             else
             {
-                Directory.CreateDirectory(Path.GetDirectoryName(outputFilePath));
+                Directory.CreateDirectory(Path.GetDirectoryName(outputFilePath) ?? throw new ArgumentNullException(nameof(outputFilePath)));
                 File.WriteAllText(outputFilePath, result.StandardOutput);
                 _log.LogInformation($"Wrote current ADB log to {outputFilePath}");
             }
@@ -199,7 +201,7 @@ public int InstallApk(string apkPath)
             _log.LogInformation($"Attempting to install {apkPath}: ");
             if (string.IsNullOrEmpty(apkPath))
             {
-                throw new ArgumentNullException($"No value supplied for {nameof(apkPath)} ");
+                throw new ArgumentException($"No value supplied for {nameof(apkPath)} ");
             }
             if (!File.Exists(apkPath))
             {
@@ -330,7 +332,7 @@ public List<string> PullFiles(string devicePath, string localPath)
                         }
                         else
                         {
-                            Directory.CreateDirectory(Path.GetDirectoryName(destinationPath));
+                            Directory.CreateDirectory(Path.GetDirectoryName(destinationPath) ?? throw new ArgumentException(nameof(destinationPath)));
                             File.Move(filePath, destinationPath);
                             copiedFiles.Add(destinationPath);
                         }
@@ -349,7 +351,7 @@ public List<string> PullFiles(string devicePath, string localPath)
         {
             var devicesAndProperties = new Dictionary<string, string?>();
 
-            string command = CommandList[property];
+            string command = _commandList[property];
 
             var result = RunAdbCommand("devices -l", TimeSpan.FromSeconds(30));
             string[] standardOutputLines = result.StandardOutput.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries);
@@ -445,7 +447,7 @@ public List<string> PullFiles(string devicePath, string localPath)
         public Dictionary<string, string> GetAllDevicesToUse(ILogger logger, string apkRequiredProperty, string propertyName)
         {
 
-            Dictionary<string, string?> allDevicesAndTheirProperties = new Dictionary<string, string?>();
+            var allDevicesAndTheirProperties = new Dictionary<string, string?>();
             try
             {
                 allDevicesAndTheirProperties = GetAttachedDevicesWithProperties(propertyName);
@@ -466,11 +468,12 @@ public Dictionary<string, string> GetAllDevicesToUse(ILogger logger, string apkR
                 .Where(kvp => !string.IsNullOrEmpty(kvp.Value) && kvp.Value.Split().Contains(apkRequiredProperty))
                 .ToDictionary(kvp => kvp.Key, kvp => kvp.Value) as Dictionary<string, string>;
 
-            if (result.Count() == 0)
+            if (result.Count == 0)
             {
                 // In this case, the enumeration worked, we found one or more devices, but nothing matched the APK's architecture; fail out.
                 logger.LogError($"No devices with {propertyName} '{apkRequiredProperty}' was found among attached devices.");
             }
+
             return result;
         }
 

src/Microsoft.DotNet.XHarness.Android/Execution/AdbProcessManager.cs

@@ -30,7 +30,7 @@ public ProcessExecutionResults Run(string adbExePath, string arguments, TimeSpan
             {
                 CreateNoWindow = true,
                 UseShellExecute = false,
-                WorkingDirectory = Path.GetDirectoryName(adbExePath),
+                WorkingDirectory = Path.GetDirectoryName(adbExePath) ?? throw new ArgumentNullException(nameof(adbExePath)),
                 RedirectStandardOutput = true,
                 RedirectStandardError = true,
                 FileName = adbExePath,

src/Microsoft.DotNet.XHarness.Android/Microsoft.DotNet.XHarness.Android.csproj

@@ -1,8 +1,9 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netstandard2.1</TargetFramework>
+    <TargetFrameworks>netstandard2.1;net6.0</TargetFrameworks>
     <nullable>enable</nullable>
+    <LangVersion>9.0</LangVersion>
   </PropertyGroup>
 
   <ItemGroup>

src/Microsoft.DotNet.XHarness.Apple/AppRunner.cs

@@ -285,7 +285,7 @@ private static MlaunchArguments GetCommonArguments(IEnumerable<string> extraAppA
             return args;
         }
 
-        private MlaunchArguments GetSimulatorArguments(
+        private static MlaunchArguments GetSimulatorArguments(
             AppBundleInformation appInformation,
             ISimulatorDevice simulator,
             IEnumerable<string> extraAppArguments,
@@ -315,7 +315,7 @@ private MlaunchArguments GetSimulatorArguments(
             return args;
         }
 
-        private MlaunchArguments GetDeviceArguments(
+        private static MlaunchArguments GetDeviceArguments(
             AppBundleInformation appInformation,
             string deviceName,
             bool isWatchTarget,

src/Microsoft.DotNet.XHarness.Apple/AppTester.cs

@@ -60,7 +60,7 @@ public AppTester(
             _snapshotReporterFactory = snapshotReporterFactory ?? throw new ArgumentNullException(nameof(snapshotReporterFactory));
             _captureLogFactory = captureLogFactory ?? throw new ArgumentNullException(nameof(captureLogFactory));
             _deviceLogCapturerFactory = deviceLogCapturerFactory ?? throw new ArgumentNullException(nameof(deviceLogCapturerFactory));
-            _testReporterFactory = reporterFactory ?? throw new ArgumentNullException(nameof(_testReporterFactory));
+            _testReporterFactory = reporterFactory ?? throw new ArgumentNullException(nameof(reporterFactory));
             _resultParser = resultParser ?? throw new ArgumentNullException(nameof(resultParser));
             _mainLog = mainLog ?? throw new ArgumentNullException(nameof(mainLog));
             _logs = logs ?? throw new ArgumentNullException(nameof(logs));

src/Microsoft.DotNet.XHarness.Apple/ErrorKnowledgeBase.cs

@@ -13,7 +13,7 @@ namespace Microsoft.DotNet.XHarness.Apple
 {
     public class ErrorKnowledgeBase : IErrorKnowledgeBase
     {
-        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_testErrorMaps = new Dictionary<string, (string HumanMessage, string? IssueLink)>
+        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_testErrorMaps = new()
         {
             ["Failed to communicate with the device"] = // Known issue but not a failure.
                 ("Failed to communicate with the device. Please ensure the cable is properly connected, and try rebooting the device", null),
@@ -28,9 +28,9 @@ public class ErrorKnowledgeBase : IErrorKnowledgeBase
                 ("This application requires a newer version of MacOS", null),
         };
 
-        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_buildErrorMaps = new Dictionary<string, (string HumanMessage, string? IssueLink)>();
+        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_buildErrorMaps = new();
 
-        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_installErrorMaps = new Dictionary<string, (string HumanMessage, string? IssueLink)>
+        private static readonly Dictionary<string, (string HumanMessage, string? IssueLink)> s_installErrorMaps = new()
         {
             ["IncorrectArchitecture"] =
                 ("IncorrectArchitecture: Failed to find matching device arch for the application", null), // known failure, but not an issue

src/Microsoft.DotNet.XHarness.Apple/ExitCodeDetector.cs

@@ -23,7 +23,7 @@ public int DetectExitCode(AppBundleInformation appBundleInfo, IReadableLog syste
             {
                 var line = reader.ReadLine();
 
-                if (IsSignalLine(appBundleInfo, line))
+                if (!string.IsNullOrEmpty(line) && IsSignalLine(appBundleInfo, line))
                 {
                     var match = ExitCodeRegex.Match(line);
 

src/Microsoft.DotNet.XHarness.Apple/Microsoft.DotNet.XHarness.Apple.csproj

@@ -1,9 +1,10 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netstandard2.1</TargetFramework>
+    <TargetFrameworks>netstandard2.1;net6.0</TargetFrameworks>
     <Nullable>enable</Nullable>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <LangVersion>9.0</LangVersion>
   </PropertyGroup>
 
   <ItemGroup>

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidGetDeviceCommandArguments.cs

@@ -14,7 +14,7 @@ internal class AndroidGetDeviceCommandArguments : TestCommandArguments
         /// </summary>
         public string? DeviceArchitecture { get; set; }
 
-        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        protected override OptionSet GetTestCommandOptions() => new()
         {
             { "device-arch=", "If specified, forces running on a device with given architecture (x86, x86_64, arm64-v8a or armeabi-v7a). Otherwise inferred from supplied APK",
                 v => DeviceArchitecture = v
@@ -23,8 +23,10 @@ internal class AndroidGetDeviceCommandArguments : TestCommandArguments
 
         public override void Validate()
         {
+            base.Validate();
+
             // Validate this field
-            AppPackagePath = AppPackagePath;
+            _ = AppPackagePath;
         }
     }
 }

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidGetStateCommandArguments.cs

@@ -8,7 +8,7 @@ namespace Microsoft.DotNet.XHarness.CLI.CommandArguments.Android
 {
     internal class AndroidGetStateCommandArguments : GetStateCommandArguments
     {
-        protected override OptionSet GetCommandOptions() => new OptionSet();
+        protected override OptionSet GetCommandOptions() => new();
 
         public override void Validate()
         {

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidInstallCommandArguments.cs

@@ -25,7 +25,7 @@ public string PackageName
         /// </summary>
         public string? DeviceArchitecture { get; set; }
 
-        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        protected override OptionSet GetTestCommandOptions() => new()
         {
             { "package-name=|p=", "Package name contained within the supplied APK",
                 v => PackageName = v
@@ -38,10 +38,11 @@ public string PackageName
 
         public override void Validate()
         {
+            base.Validate();
+
             // Validate this field
-            PackageName = PackageName;
-            AppPackagePath = AppPackagePath;
-            DeviceId = DeviceId;
+            _ = PackageName;
+            _ = AppPackagePath;
         }
     }
 }

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidRunCommandArguments.cs

@@ -38,7 +38,7 @@ public string PackageName
         /// </summary>
         public int ExpectedExitCode { get; set; } = (int)Common.CLI.ExitCode.SUCCESS;
 
-        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        protected override OptionSet GetTestCommandOptions() => new()
         {
             { "device-out-folder=|dev-out=", "If specified, copy this folder recursively off the device to the path specified by the output directory",
                 v => DeviceOutputFolder = RootPath(v)
@@ -88,8 +88,7 @@ public override void Validate()
             base.Validate();
 
             // Validate this field
-            PackageName = PackageName;
-            DeviceId = DeviceId;
+            _ = PackageName;
         }
     }
 }

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidTestCommandArguments.cs

@@ -42,7 +42,7 @@ public string PackageName
         /// </summary>
         public int ExpectedExitCode { get; set; } = (int)Common.CLI.ExitCode.SUCCESS;
 
-        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        protected override OptionSet GetTestCommandOptions() => new()
         {
             { "device-arch=", "If specified, only run on a device with the listed architecture (x86, x86_64, arm64-v8a or armeabi-v7a).  Otherwise infer from supplied APK",
                 v => DeviceArchitecture = v
@@ -91,8 +91,8 @@ public override void Validate()
             base.Validate();
 
             // Validate this field
-            PackageName = PackageName;
-            AppPackagePath = AppPackagePath;
+            _ = PackageName;
+            _ = AppPackagePath;
         }
     }
 }

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Android/AndroidUninstallCommandArguments.cs

@@ -19,7 +19,7 @@ public string PackageName
 
         public string? DeviceId { get; set; }
 
-        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        protected override OptionSet GetTestCommandOptions() => new()
         {
             { "package-name=|p=", "Package name contained within the supplied APK",
                 v => PackageName = v
@@ -33,8 +33,7 @@ public string PackageName
         public override void Validate()
         {
             // Validate this field
-            PackageName = PackageName;
-            DeviceId = DeviceId;
+            _ = PackageName;
         }
     }
 }

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/AppRunCommandArguments.cs

@@ -43,7 +43,7 @@ public string OutputDirectory
         /// </summary>
         public TimeSpan Timeout { get; set; } = TimeSpan.FromMinutes(15);
 
-        protected override OptionSet GetCommandOptions() => new OptionSet
+        protected override OptionSet GetCommandOptions() => new()
         {
             {
                 "app|a=", "Path to already-packaged app",

src/Microsoft.DotNet.XHarness.CLI/CommandArguments/Apple/AppleAppRunArguments.cs

@@ -41,7 +41,7 @@ internal abstract class AppleAppRunArguments : AppRunCommandArguments
         /// Environmental variables set when executing the application.
         /// </summary>
         public IReadOnlyCollection<(string, string)> EnvironmentalVariables => _environmentalVariables;
-        private readonly List<(string, string)> _environmentalVariables = new List<(string, string)>();
+        private readonly List<(string, string)> _environmentalVariables = new();
 
         /// <summary>
         /// Kills running simulator processes and removes any previous data before running.
@@ -55,7 +55,7 @@ protected set
             {
                 var testTargets = new List<TestTargetOs>();
 
-                foreach (var targetName in value ?? throw new ArgumentNullException("Targets cannot be empty"))
+                foreach (var targetName in value ?? throw new ArgumentException("Targets cannot be empty"))
                 {
                     try
                     {