Enable C# 8.0 nullability for the CLI, Android and iOS projects, impr…

…ove help command behaviour (dotnet#145)

This enables the nullability and resolves all warnings connected to that in the projects.

There were a couple of changes I had to do (arguments re-work) in order to achieve this and some I did to improve working with the code:
- Moved option definition to Argument classes. This makes the code easier to work with because the option definitions are together with the properties that they fill and their validation.
- Improved the help command so that you can run `help [command] [subcommand]` which was not possible before. Now it's easier to explore the tool using calling `help xy`. It now also works properly even when you mess up the subcommand name.
- Moved the Darwin class to the iOS project and kept the CLI project strictly CLI.
- Dedicate the `ios test t=` argument to the `targets` option. It was originally assigned to `timeout` but `timeout` is optional and `targets` not. This change allows to call `ios test` with all required args using shortcuts (`-o -a -t`)

Fixes: #16

README.md

@@ -20,8 +20,11 @@ The tool requires **.NET 3.1.201** and later to be run. It is packaged as a `dot
 
 To install the tool run:
 
-```console
-dotnet tool install --global --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json Microsoft.DotNet.XHarness.CLI --version 1.0.0-prerelease.20229.6
+```bash
+dotnet tool install Microsoft.DotNet.XHarness.CLI \
+    --global \
+    --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json \
+    --version 1.0.0-prerelease.20229.6
 ```
 
 You can get the specific version from [the dotnet-eng feed](https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=dotnet-eng&view=versions&package=Microsoft.DotNet.XHarness.CLI&protocolType=NuGet) where it is published.
@@ -34,22 +37,56 @@ To run the tool, use the `dotnet xharness` command. The tool always expects the
 
 Example:
 
-```console
+```bash
 dotnet xharness android state
 ```
 
 To list all the possible commands, use the `help` command:
 
-```console
+```bash
 dotnet xharness help
 ```
 
-To get help for a sub-command command:
+To get help for a specific command or sub-command, run:
+
+```bash
+dotnet xharness help ios
+dotnet xharness help ios package
+```
+
+## Examples
+
+To run an iOS app bundle on a 64bit iPhone Simulator:
 
+```bash
+dotnet xharness ios test \
+    --app=/path/to/an.app \
+    --output-directory=out \
+    --targets=ios-simulator-64
+```
+
+or the same can be achieved via the shorthand versions of the same options:
+
+```bash
+dotnet xharness ios test -a=/path/to/an.app -o=out -t=ios-simulator-64
+```
+
+The `out` dir will then contain log files such as these:
 ```console
-dotnet xharness ios test help
+iPhone X (iOS 13.3) - created by xharness.log
+run-Simulator_iOS64.log
+simulator-list-20200430_025916.log
+test-ios-simulator-64-20200430_025916.log
+test-ios-simulator-64-20200430_025916.xml
 ```
 
+These files are:
+- logs from the Simulator
+- logs from the tool itself
+- logs from getting the list of available Simulators
+- Test results in human readable format
+- Test results in XML format (default is xUnit but can be changed via options)
+
 ## Test Runners
 
 The repository also contains several TestRunners that are bundled inside of the application and execute the tests.

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

@@ -293,7 +293,7 @@ public void PushFiles(string localDirectory, string devicePath, bool removeIfPre
             RunApkInstrumentation(apkName, "", args, timeout);
 
 
-        public (string StandardOutput, string StandardError, int ExitCode) RunApkInstrumentation(string apkName, string instrumentationClassName, Dictionary<string, string> args, TimeSpan timeout)
+        public (string StandardOutput, string StandardError, int ExitCode) RunApkInstrumentation(string apkName, string? instrumentationClassName, Dictionary<string, string> args, TimeSpan timeout)
         {
             string displayName = string.IsNullOrEmpty(instrumentationClassName) ? "{default}" : instrumentationClassName;
 

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

@@ -2,6 +2,7 @@
 
   <PropertyGroup>
     <TargetFramework>netstandard2.1</TargetFramework>
+    <nullable>enable</nullable>
   </PropertyGroup>
 
   <ItemGroup>

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

@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Mono.Options;
+
+namespace Microsoft.DotNet.XHarness.CLI.CommandArguments.Android
+{
+    internal class AndroidGetStateCommandArguments : GetStateCommandArguments
+    {
+        protected override OptionSet GetCommandOptions() => new OptionSet();
+
+        public override void Validate()
+        {
+        }
+    }
+}

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

@@ -2,35 +2,75 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Collections.Generic;
+using Mono.Options;
 
 namespace Microsoft.DotNet.XHarness.CLI.CommandArguments.Android
 {
     internal class AndroidTestCommandArguments : TestCommandArguments
     {
+        private string? _packageName;
+
         /// <summary>
         /// If specified, attempt to run instrumentation with this name instead of the default for the supplied APK.
         /// If a given package has multiple instrumentations, failing to specify this may cause execution failure.
         /// </summary>
-        public string InstrumentationName { get; set; }
+        public string? InstrumentationName { get; set; }
 
         /// <summary>
         /// If specified, attempt to run instrumentation with this name instead of the default for the supplied APK
         /// </summary>
-        public string PackageName { get; set; }
+        public string PackageName
+        {
+            get => _packageName ?? throw new ArgumentException("Package name not specified");
+            set => _packageName = value;
+        }
 
         /// <summary>
         /// Folder to copy off for output of executing the specified APK
         /// </summary>
-        public string DeviceOutputFolder { get; set; }
+        public string? DeviceOutputFolder { get; set; }
+
+        public Dictionary<string, string> InstrumentationArguments { get; } = new Dictionary<string, string>();
+
+        protected override OptionSet GetTestCommandOptions() => new OptionSet
+        {
+            { "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)
+            },
+            { "instrumentation:|i:", "If specified, attempt to run instrumentation with this name instead of the default for the supplied APK.",
+                v => InstrumentationName = v
+            },
+            { "package-name=|p=", "Package name contained within the supplied APK",
+                v => PackageName = v
+            },
+            { "arg=", "Argument to pass to the instrumentation, in form key=value", v =>
+                {
+                    var argPair = v.Split('=');
+
+                    if (argPair.Length != 2)
+                    {
+                        throw new ArgumentException($"The --arg argument expects 'key=value' format. Invalid format found in '{v}'");
+                    }
+
+                    if (InstrumentationArguments.ContainsKey(argPair[0]))
+                    {
+                        throw new ArgumentException($"Duplicate arg name '{argPair[0]}' found");
+                    }
 
-        public Dictionary<string, string> InstrumentationArguments { get; set; } = new Dictionary<string, string>();
+                    InstrumentationArguments.Add(argPair[0].Trim(), argPair[1].Trim());
+                }
+            },
+        };
 
-        public override IList<string> GetValidationErrors()
+        public override void Validate()
         {
-            var errors = base.GetValidationErrors();
+            base.Validate();
 
-            return errors;
+            // Validate this field
+            PackageName = PackageName;
+            AppPackagePath = AppPackagePath;
         }
     }
 }

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

@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace Microsoft.DotNet.XHarness.CLI.CommandArguments
+{
+    internal abstract class GetStateCommandArguments : XHarnessCommandArguments
+    {
+    }
+}

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

@@ -5,68 +5,85 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using Microsoft.Extensions.Logging;
+using Mono.Options;
 
 namespace Microsoft.DotNet.XHarness.CLI.CommandArguments
 {
-    internal interface ITestCommandArguments : ICommandArguments
+    internal abstract class TestCommandArguments : XHarnessCommandArguments
     {
+        private string? _appPackagePath = null;
+        private string? _outputDirectory = null;
+
         /// <summary>
         /// Path to packaged app
         /// </summary>
-        string AppPackagePath { get; set; }
+        public string AppPackagePath
+        {
+            get => _appPackagePath ?? throw new ArgumentException("You must provide a path for the app bundle that will be tested.");
+            set => _appPackagePath = value;
+        }
 
         /// <summary>
-        /// List of targets to test
+        /// Path where the outputs of execution will be stored
         /// </summary>
-        IReadOnlyCollection<string> Targets { get; set; }
+        public string OutputDirectory
+        {
+            get => _outputDirectory ?? throw new ArgumentException("You must provide an output directory where results will be stored.");
+            set => _outputDirectory = value;
+        }
 
         /// <summary>
-        /// How long XHarness should wait until a test execution completes before clean up (kill running apps, uninstall, etc)
+        /// List of targets to test
         /// </summary>
-        TimeSpan Timeout { get; set; }
+        public virtual IReadOnlyCollection<string> Targets { get; protected set; } = Array.Empty<string>();
 
         /// <summary>
-        /// Path where the outputs of execution will be stored
+        /// How long XHarness should wait until a test execution completes before clean up (kill running apps, uninstall, etc)
         /// </summary>
-        string OutputDirectory { get; set; }
-    }
-
-    internal abstract class TestCommandArguments : ITestCommandArguments
-    {
-        public string AppPackagePath { get; set; }
-        public IReadOnlyCollection<string> Targets { get; set; }
         public TimeSpan Timeout { get; set; } = TimeSpan.FromMinutes(15);
-        public string OutputDirectory { get; set; }
-        public LogLevel Verbosity { get; set; }
 
-        public virtual IList<string> GetValidationErrors()
+        protected sealed override OptionSet GetCommandOptions()
         {
-            var errors = new List<string>();
-
-            if (string.IsNullOrEmpty(AppPackagePath))
+            var options = new OptionSet
             {
-                errors.Add("You must provide a name for the application that will be tested.");
-            }
+                { "app|a=", "Path to already-packaged app",
+                    v => AppPackagePath = RootPath(v)
+                },
+                { "output-directory=|o=", "Directory in which the resulting package will be outputted",
+                    v => OutputDirectory = RootPath(v)
+                },
+                { "targets=|t=", "Comma-delineated list of targets to test for",
+                    v => Targets = v.Split(',')
+                },
+                { "timeout=", "Time span, in seconds, to wait for instrumentation to complete.",
+                    v =>
+                    {
+                        if (!int.TryParse(v, out var timeout))
+                        {
+                            throw new ArgumentException("timeout must be an integer - a number of seconds");
+                        }
+
+                        Timeout = TimeSpan.FromSeconds(timeout);
+                    }
+                },
+            };
 
-            if (string.IsNullOrEmpty(OutputDirectory))
+            foreach (var option in GetTestCommandOptions())
             {
-                errors.Add("Output directory path missing.");
+                options.Add(option);
             }
-            else
-            {
-                if (!Path.IsPathRooted(OutputDirectory))
-                {
-                    OutputDirectory = Path.Combine(Directory.GetCurrentDirectory(), OutputDirectory);
-                }
 
-                if (!Directory.Exists(OutputDirectory))
-                {
-                    Directory.CreateDirectory(OutputDirectory);
-                }
-            }
+            return options;
+        }
+
+        protected abstract OptionSet GetTestCommandOptions();
 
-            return errors;
+        public override void Validate()
+        {
+            if (!Directory.Exists(OutputDirectory))
+            {
+                Directory.CreateDirectory(OutputDirectory);
+            }
         }
     }
 }