From ef24a2d20844b53ded8f4ed6cd0069b98f9423d4 Mon Sep 17 00:00:00 2001 From: Premek Vysoky Date: Mon, 6 Sep 2021 16:49:01 +0200 Subject: [PATCH 1/2] Deduplicate XHarness Apple/Android code --- .../Microsoft.Arcade.Common/FileSystem.cs | 2 + .../Microsoft.Arcade.Common/IFileSystem.cs | 2 + .../Sdk/CreateXHarnessAndroidWorkItems.cs | 63 +++------------- .../Sdk/CreateXHarnessAppleWorkItems.cs | 70 +++--------------- .../Sdk/XharnessTaskBase.cs | 73 +++++++++++++++++++ 5 files changed, 99 insertions(+), 111 deletions(-) diff --git a/src/Common/Microsoft.Arcade.Common/FileSystem.cs b/src/Common/Microsoft.Arcade.Common/FileSystem.cs index d5a12c3d3eb..83fd1df0dcf 100644 --- a/src/Common/Microsoft.Arcade.Common/FileSystem.cs +++ b/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); } } diff --git a/src/Common/Microsoft.Arcade.Common/IFileSystem.cs b/src/Common/Microsoft.Arcade.Common/IFileSystem.cs index 538f7d71030..2aed8429879 100644 --- a/src/Common/Microsoft.Arcade.Common/IFileSystem.cs +++ b/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); } } diff --git a/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAndroidWorkItems.cs b/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAndroidWorkItems.cs index d533f984427..7752e58eb6a 100644 --- a/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAndroidWorkItems.cs +++ b/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAndroidWorkItems.cs @@ -67,20 +67,7 @@ public bool ExecuteTask(IZipArchiveManager zipArchiveManager, IFileSystem fileSy /// An ITaskItem instance representing the prepared HelixWorkItem. private async Task 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 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 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(outputZipPath, ScriptNamespace + PosixAndroidWrapperScript, PosixAndroidWrapperScript); - await zipArchiveManager.AddResourceFileToArchive(outputZipPath, ScriptNamespace + NonPosixAndroidWrapperScript, NonPosixAndroidWrapperScript); - await zipArchiveManager.AddContentToArchive(outputZipPath, CustomCommandsScript + (IsPosixShell ? ".sh" : ".ps1"), injectedCommands); - - return outputZipPath; - } } } diff --git a/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAppleWorkItems.cs b/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAppleWorkItems.cs index 43e164a9382..b9667c4bdc2 100644 --- a/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAppleWorkItems.cs +++ b/src/Microsoft.DotNet.Helix/Sdk/CreateXHarnessAppleWorkItems.cs @@ -91,30 +91,12 @@ private async Task 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 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 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(outputZipPath, ScriptNamespace + EntryPointScript, EntryPointScript); - await zipArchiveManager.AddResourceFileToArchive(outputZipPath, ScriptNamespace + RunnerScript, RunnerScript); - await zipArchiveManager.AddContentToArchive(outputZipPath, CustomCommandsScript + ".sh", injectedCommands); - - return outputZipPath; - } } } diff --git a/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs b/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs index 311aeed96d4..767c6a6eb12 100644 --- a/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs +++ b/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; @@ -107,5 +109,76 @@ protected Build.Utilities.TaskItem CreateTaskItem(string workItemName, string pa { "Timeout", timeout.ToString() }, }); } + + /// + /// 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. + /// + 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 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-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) + { + 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( + outputZipPath, + ScriptNamespace + payloadScript, + payloadScript); + } + + await zipArchiveManager.AddContentToArchive( + outputZipPath, + CustomCommandsScript + (isPosix ? ".sh" : ".ps1"), + injectedCommands); + + return outputZipPath; + } } } From 9d18360408b8ae42ca6719ad926778a12651456a Mon Sep 17 00:00:00 2001 From: Premek Vysoky Date: Mon, 6 Sep 2021 17:05:28 +0200 Subject: [PATCH 2/2] Fix unit tests --- .../Microsoft.Arcade.Test.Common/MockFileSystem.cs | 12 ++++++++++++ .../CreateXHarnessAndroidWorkItemsTests.cs | 10 +++++----- .../CreateXHarnessAppleWorkItemsTests.cs | 10 +++++----- src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs | 8 ++++---- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/Common/Microsoft.Arcade.Test.Common/MockFileSystem.cs b/src/Common/Microsoft.Arcade.Test.Common/MockFileSystem.cs index d1a17a32858..ed126d9d21a 100644 --- a/src/Common/Microsoft.Arcade.Test.Common/MockFileSystem.cs +++ b/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 /// diff --git a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAndroidWorkItemsTests.cs b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAndroidWorkItemsTests.cs index 82716bbea64..450f915700b 100644 --- a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAndroidWorkItemsTests.cs +++ b/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(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce); _zipArchiveManager - .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(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(), It.IsAny()), Times.Never); _zipArchiveManager - .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.sh")), "xharness-helix-job.android.sh"), Times.AtLeastOnce); _zipArchiveManager - .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.android.ps1")), "xharness-helix-job.android.ps1"), Times.AtLeastOnce); } [Fact] diff --git a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAppleWorkItemsTests.cs b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAppleWorkItemsTests.cs index 3033a9bd1dd..4652965fbaf 100644 --- a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateXHarnessAppleWorkItemsTests.cs +++ b/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(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once); _zipArchiveManager - .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once); _zipArchiveManager .Verify(x => x.AddContentToArchive(payloadArchive, "command.sh", It.Is(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(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-helix-job.apple.sh")), "xharness-helix-job.apple.sh"), Times.Once); _zipArchiveManager - .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once); + .Verify(x => x.AddResourceFileToArchive(payloadArchive, It.Is(s => s.Contains("xharness-runner.apple.sh")), "xharness-runner.apple.sh"), Times.Once); _zipArchiveManager .Verify(x => x.AddContentToArchive(payloadArchive, "command.sh", It.Is(s => s.Contains("xharness apple test"))), Times.Once); } diff --git a/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs b/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs index 767c6a6eb12..af666f56325 100644 --- a/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs +++ b/src/Microsoft.DotNet.Helix/Sdk/XharnessTaskBase.cs @@ -23,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"; /// /// Extra arguments that will be passed to the iOS/Android/... app that is being run @@ -137,7 +137,7 @@ protected async Task CreatePayloadArchive( string[] payloadScripts) { string appFolderDirectory = fileSystem.GetDirectoryName(pathToZip); - string fileName = $"xharness-app-payload-{workItemName.ToLowerInvariant()}.zip"; + string fileName = $"xharness-payload-{workItemName.ToLowerInvariant()}.zip"; string outputZipPath = fileSystem.PathCombine(appFolderDirectory, fileName); if (fileSystem.FileExists(outputZipPath)) @@ -167,7 +167,7 @@ protected async Task CreatePayloadArchive( foreach (var payloadScript in payloadScripts) { - await zipArchiveManager.AddResourceFileToArchive( + await zipArchiveManager.AddResourceFileToArchive( outputZipPath, ScriptNamespace + payloadScript, payloadScript);