Deduplicate XHarness Apple/Android code in Helix SDK

src/Common/Microsoft.Arcade.Common/FileSystem.cs

@@ -36,5 +36,7 @@ public void WriteToFile(string path, string content)
         public void FileCopy(string sourceFileName, string destFileName) => File.Copy(sourceFileName, destFileName);
 
         public Stream GetFileStream(string path, FileMode mode, FileAccess access) => new FileStream(path, mode, access);
+
+        public FileAttributes GetAttributes(string path) => File.GetAttributes(path);
     }
 }

src/Common/Microsoft.Arcade.Common/IFileSystem.cs

@@ -31,5 +31,7 @@ public interface IFileSystem
         void FileCopy(string sourceFileName, string destFileName);
 
         Stream GetFileStream(string path, FileMode mode, FileAccess access);
+
+        FileAttributes GetAttributes(string path);
     }
 }

src/Common/Microsoft.Arcade.Test.Common/MockFileSystem.cs

@@ -59,6 +59,18 @@ public void DeleteFile(string path)
         public Stream GetFileStream(string path, FileMode mode, FileAccess access)
             => FileExists(path) ? new MemoryStream() : new MockFileStream(this, path);
 
+        public FileAttributes GetAttributes(string path)
+        {
+            var attributes = FileAttributes.Normal;
+
+            if (Directories.Contains(path))
+            {
+                attributes |= FileAttributes.Directory;
+            }
+
+            return  attributes;
+        }
+
         #endregion
 
         /// <summary>

src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAndroidWorkItemsTests.cs

@@ -88,9 +88,9 @@ public void AndroidXHarnessWorkItemIsCreated()
             _zipArchiveManager
                 .Verify(x => x.ArchiveFile("/apks/System.Foo.apk", payloadArchive), Times.Once);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce);
         }
 
         [Fact]
@@ -104,7 +104,7 @@ public void ArchivePayloadIsOverwritten()
                 CreateApk("apks/System.Bar.apk", "System.Bar"),
             };
 
-            _fileSystem.Files.Add("apks/xharness-apk-payload-system.foo.zip", "archive");
+            _fileSystem.Files.Add("apks/xharness-payload-system.foo.zip", "archive");
 
             // Act
             using var provider = collection.BuildServiceProvider();
@@ -190,9 +190,9 @@ public void ZippedApkIsProvided()
             _zipArchiveManager
                 .Verify(x => x.ArchiveFile(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce);
         }
 
         [Fact]

src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAppleWorkItemsTests.cs

@@ -96,9 +96,9 @@ public void AppleXHarnessWorkItemIsCreated()
             _zipArchiveManager
                 .Verify(x => x.ArchiveDirectory("/apps/System.Foo.app", payloadArchive, true), Times.Once);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once);
             _zipArchiveManager
                 .Verify(x => x.AddContentToArchive(payloadArchive, "command.sh", It.Is<string>(s => s.Contains("xharness apple test"))), Times.Once);
         }
@@ -114,7 +114,7 @@ public void ArchivePayloadIsOverwritten()
                 CreateAppBundle("apps/System.Bar.app", "ios-simulator-64_13.5"),
             };
 
-            _fileSystem.Files.Add("apps/xharness-app-payload-system.foo.zip", "archive");
+            _fileSystem.Files.Add("apps/xharness-payload-system.foo.zip", "archive");
 
             // Act
             using var provider = collection.BuildServiceProvider();
@@ -242,9 +242,9 @@ public void ZippedAppIsProvided()
             _zipArchiveManager
                 .Verify(x => x.ArchiveDirectory("/apps/System.Foo.app", payloadArchive, true), Times.Never);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once);
             _zipArchiveManager
-                .Verify(x => x.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(payloadArchive, It.Is<string>(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once);
+                .Verify(x => x.AddResourceFileToArchive<XHarnessTaskBase>(payloadArchive, It.Is<string>(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once);
             _zipArchiveManager
                 .Verify(x => x.AddContentToArchive(payloadArchive, "command.sh", It.Is<string>(s => s.Contains("xharness apple test"))), Times.Once);
         }

src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAndroidWorkItems.cs

@@ -67,20 +67,7 @@ public bool ExecuteTask(IZipArchiveManager zipArchiveManager, IFileSystem fileSy
         /// <returns>An ITaskItem instance representing the prepared HelixWorkItem.</returns>
         private async Task<ITaskItem> PrepareWorkItem(IZipArchiveManager zipArchiveManager, IFileSystem fileSystem, ITaskItem appPackage)
         {
-            // The user can re-use the same .apk for 2 work items so the name of the work item will come from ItemSpec and path from metadata
-            // This can be useful when we want to run the same app with different parameters or run the same app on different test targets, e.g. iOS 13 and 13.5
-            string workItemName;
-            string apkPath;
-            if (appPackage.TryGetMetadata(MetadataNames.ApkPath, out string apkPathMetadata) && !string.IsNullOrEmpty(apkPathMetadata))
-            {
-                workItemName = appPackage.ItemSpec;
-                apkPath = apkPathMetadata;
-            }
-            else
-            {
-                workItemName = fileSystem.GetFileNameWithoutExtension(appPackage.ItemSpec);
-                apkPath = appPackage.ItemSpec;
-            }
+            var (workItemName, apkPath) = GetNameAndPath(appPackage, MetadataNames.ApkPath, fileSystem);
 
             if (!fileSystem.FileExists(apkPath))
             {
@@ -120,13 +107,20 @@ private async Task<ITaskItem> PrepareWorkItem(IZipArchiveManager zipArchiveManag
 
             string command = GetHelixCommand(appPackage, apkName, androidPackageName, testTimeout, expectedExitCode);
 
-            string workItemZip = await CreateZipArchiveOfPackageAsync(
+            string workItemZip = await CreatePayloadArchive(
                 zipArchiveManager,
                 fileSystem,
                 workItemName,
                 isAlreadyArchived,
+                IsPosixShell,
                 apkPath,
-                customCommands);
+                customCommands,
+                new[]
+                {
+                    // WorkItem payloads of APKs can be reused if sent to multiple queues at once,
+                    // so we'll always include both scripts (very small)
+                    PosixAndroidWrapperScript, NonPosixAndroidWrapperScript
+                });
 
             return CreateTaskItem(workItemName, workItemZip, command, workItemTimeout);
         }
@@ -179,42 +173,5 @@ private string GetHelixCommand(ITaskItem appPackage, string apkName, string andr
 
             return xharnessRunCommand;
         }
-
-        private async Task<string> CreateZipArchiveOfPackageAsync(
-            IZipArchiveManager zipArchiveManager,
-            IFileSystem fileSystem,
-            string workItemName,
-            bool isAlreadyArchived,
-            string fileToZip,
-            string injectedCommands)
-        {
-            string fileName = $"xharness-apk-payload-{workItemName.ToLowerInvariant()}.zip";
-            string outputZipPath = fileSystem.PathCombine(fileSystem.GetDirectoryName(fileToZip), fileName);
-
-            if (fileSystem.FileExists(outputZipPath))
-            {
-                Log.LogMessage($"Zip archive '{outputZipPath}' already exists, overwriting..");
-                fileSystem.DeleteFile(outputZipPath);
-            }
-
-            if (!isAlreadyArchived)
-            {
-                zipArchiveManager.ArchiveFile(fileToZip, outputZipPath);
-            }
-            else
-            {
-                Log.LogMessage($"App payload '{workItemName}` has already been zipped. Copying to '{outputZipPath}` instead");
-                fileSystem.FileCopy(fileToZip, outputZipPath);
-            }
-
-            // WorkItem payloads of APKs can be reused if sent to multiple queues at once,
-            // so we'll always include both scripts (very small)
-            Log.LogMessage($"Adding the XHarness job scripts into the payload archive");
-            await zipArchiveManager.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(outputZipPath, ScriptNamespace + PosixAndroidWrapperScript, PosixAndroidWrapperScript);
-            await zipArchiveManager.AddResourceFileToArchive<CreateXHarnessAndroidWorkItems>(outputZipPath, ScriptNamespace + NonPosixAndroidWrapperScript, NonPosixAndroidWrapperScript);
-            await zipArchiveManager.AddContentToArchive(outputZipPath, CustomCommandsScript + (IsPosixShell ? ".sh" : ".ps1"), injectedCommands);
-
-            return outputZipPath;
-        }
     }
 }

src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAppleWorkItems.cs

@@ -91,30 +91,12 @@ private async Task<ITaskItem> PrepareWorkItem(
             IFileSystem fileSystem,
             ITaskItem appBundleItem)
         {
-            // The user can re-use the same .apk for 2 work items so the name of the work item will come from ItemSpec and path from metadata
-            string workItemName;
-            string appFolderPath;
-            if (appBundleItem.TryGetMetadata(MetadataNames.AppBundlePath, out string appPathMetadata) && !string.IsNullOrEmpty(appPathMetadata))
-            {
-                workItemName = appBundleItem.ItemSpec;
-                appFolderPath = appPathMetadata;
-            }
-            else
-            {
-                workItemName = fileSystem.GetFileName(appBundleItem.ItemSpec);
-                appFolderPath = appBundleItem.ItemSpec;
-            }
+            var (workItemName, appFolderPath) = GetNameAndPath(appBundleItem, MetadataNames.AppBundlePath, fileSystem);
 
             appFolderPath = appFolderPath.TrimEnd(Path.DirectorySeparatorChar);
 
-            bool isAlreadyArchived = workItemName.EndsWith(".zip");
-
-            if (isAlreadyArchived)
-            {
-                workItemName = workItemName.Substring(0, workItemName.Length - 4);
-            }
-
-            if (workItemName.EndsWith(".app"))
+            bool isAlreadyArchived = appFolderPath.EndsWith(".zip");
+            if (isAlreadyArchived && workItemName.EndsWith(".app"))
             {
                 // If someone named the zip something.app.zip, we want both gone
                 workItemName = workItemName.Substring(0, workItemName.Length - 4);
@@ -180,7 +162,15 @@ private async Task<ITaskItem> PrepareWorkItem(
 
             string appName = isAlreadyArchived ? $"{fileSystem.GetFileNameWithoutExtension(appFolderPath)}.app" : fileSystem.GetFileName(appFolderPath);
             string helixCommand = GetHelixCommand(appName, target, testTimeout, launchTimeout, includesTestRunner, expectedExitCode, resetSimulator);
-            string payloadArchivePath = await CreateZipArchiveOfFolder(zipArchiveManager, fileSystem, workItemName, isAlreadyArchived, appFolderPath, customCommands);
+            string payloadArchivePath = await CreatePayloadArchive(
+                zipArchiveManager,
+                fileSystem,
+                workItemName,
+                isAlreadyArchived,
+                isPosix: true,
+                appFolderPath,
+                customCommands,
+                new[] { EntryPointScript, RunnerScript });
 
             return CreateTaskItem(workItemName, payloadArchivePath, helixCommand, workItemTimeout);
         }
@@ -227,41 +217,5 @@ private string GetHelixCommand(
             $"--expected-exit-code \"{expectedExitCode}\" " +
             (!string.IsNullOrEmpty(XcodeVersion) ? $" --xcode-version \"{XcodeVersion}\"" : string.Empty) +
             (!string.IsNullOrEmpty(AppArguments) ? $" --app-arguments \"{AppArguments}\"" : string.Empty);
-
-        private async Task<string> CreateZipArchiveOfFolder(
-            IZipArchiveManager zipArchiveManager,
-            IFileSystem fileSystem,
-            string workItemName,
-            bool isAlreadyArchived,
-            string folderToZip,
-            string injectedCommands)
-        {
-            string appFolderDirectory = fileSystem.GetDirectoryName(folderToZip);
-            string fileName = $"xharness-app-payload-{workItemName.ToLowerInvariant()}.zip";
-            string outputZipPath = fileSystem.PathCombine(appFolderDirectory, fileName);
-
-            if (fileSystem.FileExists(outputZipPath))
-            {
-                Log.LogMessage($"Zip archive '{outputZipPath}' already exists, overwriting..");
-                fileSystem.DeleteFile(outputZipPath);
-            }
-
-            if (!isAlreadyArchived)
-            {
-                zipArchiveManager.ArchiveDirectory(folderToZip, outputZipPath, true);
-            }
-            else
-            {
-                Log.LogMessage($"App payload '{workItemName}` has already been zipped. Copying to '{outputZipPath}` instead");
-                fileSystem.FileCopy(folderToZip, outputZipPath);
-            }
-
-            Log.LogMessage($"Adding the XHarness job scripts into the payload archive");
-            await zipArchiveManager.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(outputZipPath, ScriptNamespace + EntryPointScript, EntryPointScript);
-            await zipArchiveManager.AddResourceFileToArchive<CreateXHarnessAppleWorkItems>(outputZipPath, ScriptNamespace + RunnerScript, RunnerScript);
-            await zipArchiveManager.AddContentToArchive(outputZipPath, CustomCommandsScript + ".sh", injectedCommands);
-
-            return outputZipPath;
-        }
     }
 }

src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs

@@ -1,5 +1,7 @@
 using System;
 using System.Collections.Generic;
+using System.IO;
+using System.Threading.Tasks;
 using Microsoft.Arcade.Common;
 using Microsoft.Build.Framework;
 
@@ -21,8 +23,8 @@ public class MetadataName
             public const string CustomCommands = "CustomCommands";
         }
 
-        protected const string ScriptNamespace = "tools.xharness_runner.";
-        protected const string CustomCommandsScript = "command";
+        private const string ScriptNamespace = "tools.xharness_runner.";
+        private const string CustomCommandsScript = "command";
 
         /// <summary>
         /// Extra arguments that will be passed to the iOS/Android/... app that is being run
@@ -107,5 +109,76 @@ protected Build.Utilities.TaskItem CreateTaskItem(string workItemName, string pa
                 { "Timeout", timeout.ToString() },
             });
         }
+
+        /// <summary>
+        /// This method parses the name for the Helix work item and path of the app from the item's metadata.
+        /// The user can re-use the same .apk for 2 work items so the name of the work item will come from ItemSpec and path from metadata.
+        /// </summary>
+        protected (string WorkItemName, string AppPath) GetNameAndPath(ITaskItem item, string pathMetadataName, IFileSystem fileSystem)
+        {
+            if (item.TryGetMetadata(pathMetadataName, out string appPathMetadata) && !string.IsNullOrEmpty(appPathMetadata))
+            {
+                return (item.ItemSpec, appPathMetadata);
+            }
+            else
+            {
+                return (fileSystem.GetFileNameWithoutExtension(item.ItemSpec), item.ItemSpec);
+            }
+        }
+
+        protected async Task<string> CreatePayloadArchive(
+            IZipArchiveManager zipArchiveManager,
+            IFileSystem fileSystem,
+            string workItemName,
+            bool isAlreadyArchived,
+            bool isPosix,
+            string pathToZip,
+            string injectedCommands,
+            string[] payloadScripts)
+        {
+            string appFolderDirectory = fileSystem.GetDirectoryName(pathToZip);
+            string fileName = $"xharness-payload-{workItemName.ToLowerInvariant()}.zip";
+            string outputZipPath = fileSystem.PathCombine(appFolderDirectory, fileName);
+
+            if (fileSystem.FileExists(outputZipPath))
+            {
+                Log.LogMessage($"Zip archive '{outputZipPath}' already exists, overwriting..");
+                fileSystem.DeleteFile(outputZipPath);
+            }
+
+            if (!isAlreadyArchived)
+            {
+                if (fileSystem.GetAttributes(pathToZip).HasFlag(FileAttributes.Directory))
+                {
+                    zipArchiveManager.ArchiveDirectory(pathToZip, outputZipPath, true);
+                }
+                else
+                {
+                    zipArchiveManager.ArchiveFile(pathToZip, outputZipPath);
+                }
+            }
+            else
+            {
+                Log.LogMessage($"App payload '{workItemName}` has already been zipped. Copying to '{outputZipPath}` instead");
+                fileSystem.FileCopy(pathToZip, outputZipPath);
+            }
+
+            Log.LogMessage($"Adding the XHarness job scripts into the payload archive");
+
+            foreach (var payloadScript in payloadScripts)
+            {
+                await zipArchiveManager.AddResourceFileToArchive<XHarnessTaskBase>(
+                    outputZipPath,
+                    ScriptNamespace + payloadScript,
+                    payloadScript);
+            }
+
+            await zipArchiveManager.AddContentToArchive(
+                outputZipPath,
+                CustomCommandsScript + (isPosix ? ".sh" : ".ps1"),
+                injectedCommands);
+
+            return outputZipPath;
+        }
     }
 }