From 1f92811ce0d599c6d4a04a4ae35786dc45d981d7 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Fri, 24 Nov 2017 11:02:05 +0000 Subject: [PATCH] Make AppVeyor builds more consistent by allowing retry for "npm install" --- test/Templates.Test/Helpers/Npm.cs | 64 +++++++++++++++++++ test/Templates.Test/Helpers/ProcessEx.cs | 10 +++ .../Helpers/TemplateTestBase.cs | 24 ------- .../SpaTemplateTest/SpaTemplateTestBase.cs | 7 +- 4 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 test/Templates.Test/Helpers/Npm.cs diff --git a/test/Templates.Test/Helpers/Npm.cs b/test/Templates.Test/Helpers/Npm.cs new file mode 100644 index 0000000000..ac71af2697 --- /dev/null +++ b/test/Templates.Test/Helpers/Npm.cs @@ -0,0 +1,64 @@ +using System; +using System.IO; +using Xunit.Abstractions; + +namespace Templates.Test.Helpers +{ + public static class Npm + { + private static object NpmInstallLock = new object(); + + public static void RestoreWithRetry(ITestOutputHelper output, string workingDirectory) + { + // "npm restore" sometimes fails randomly in AppVeyor with errors like: + // EPERM: operation not permitted, scandir ... + // This appears to be a general NPM reliability issue on Windows which has + // been reported many times (e.g., https://github.com/npm/npm/issues/18380) + // So, allow multiple attempts at the restore. + const int maxAttempts = 3; + var attemptNumber = 0; + while (true) + { + try + { + attemptNumber++; + Restore(output, workingDirectory); + break; // Success + } + catch (Exception ex) + { + if (attemptNumber < maxAttempts) + { + output.WriteLine( + $"NPM restore in {workingDirectory} failed on attempt {attemptNumber} of {maxAttempts}. " + + $"Error was: {ex}"); + + // Clean up the possibly-incomplete node_modules dir before retrying + var nodeModulesDir = Path.Combine(workingDirectory, "node_modules"); + if (Directory.Exists(nodeModulesDir)) + { + Directory.Delete(nodeModulesDir, recursive: true); + } + } + else + { + output.WriteLine( + $"Giving up attempting NPM restore in {workingDirectory} after {attemptNumber} attempts."); + throw; + } + } + } + } + + private static void Restore(ITestOutputHelper output, string workingDirectory) + { + // It's not safe to run multiple NPM installs in parallel + // https://github.com/npm/npm/issues/2500 + lock (NpmInstallLock) + { + output.WriteLine($"Restoring NPM packages in '{workingDirectory}' using npm..."); + ProcessEx.RunViaShell(output, workingDirectory, "npm install"); + } + } + } +} diff --git a/test/Templates.Test/Helpers/ProcessEx.cs b/test/Templates.Test/Helpers/ProcessEx.cs index 31840eddc9..b244d7df88 100644 --- a/test/Templates.Test/Helpers/ProcessEx.cs +++ b/test/Templates.Test/Helpers/ProcessEx.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Text; using Xunit.Abstractions; @@ -42,6 +43,15 @@ namespace Templates.Test.Helpers return new ProcessEx(output, proc); } + public static void RunViaShell(ITestOutputHelper output, string workingDirectory, string commandAndArgs) + { + var (shellExe, argsPrefix) = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? ("cmd", "/c") + : ("bash", "-c"); + Run(output, workingDirectory, shellExe, $"{argsPrefix} \"{commandAndArgs}\"") + .WaitForExit(assertSuccess: true); + } + public ProcessEx(ITestOutputHelper output, Process proc) { _output = output; diff --git a/test/Templates.Test/Helpers/TemplateTestBase.cs b/test/Templates.Test/Helpers/TemplateTestBase.cs index f85470db4a..a4c9c40c46 100644 --- a/test/Templates.Test/Helpers/TemplateTestBase.cs +++ b/test/Templates.Test/Helpers/TemplateTestBase.cs @@ -4,7 +4,6 @@ using System; using System.IO; using System.Reflection; -using System.Runtime.InteropServices; using System.Threading; using Microsoft.Extensions.CommandLineUtils; using Templates.Test.Helpers; @@ -16,7 +15,6 @@ namespace Templates.Test public class TemplateTestBase : IDisposable { private static object DotNetNewLock = new object(); - private static object NpmInstallLock = new object(); protected string ProjectName { get; set; } protected string TemplateOutputDir { get; private set; } @@ -74,28 +72,6 @@ namespace Templates.Test } } - protected void InstallNpmPackages(string relativePath) - { - // It's not safe to run multiple NPM installs in parallel - // https://github.com/npm/npm/issues/2500 - lock (NpmInstallLock) - { - var fullPath = Path.Combine(TemplateOutputDir, relativePath); - Output.WriteLine($"Restoring NPM packages in '{relativePath}' using npm..."); - RunViaShell(fullPath, "npm install"); - } - } - - private void RunViaShell(string workingDirectory, string commandAndArgs) - { - var (shellExe, argsPrefix) = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? ("cmd", "/c") - : ("bash", "-c"); - ProcessEx - .Run(Output, workingDirectory, shellExe, $"{argsPrefix} \"{commandAndArgs}\"") - .WaitForExit(assertSuccess: true); - } - protected void AssertDirectoryExists(string path, bool shouldExist) { var fullPath = Path.Combine(TemplateOutputDir, path); diff --git a/test/Templates.Test/SpaTemplateTest/SpaTemplateTestBase.cs b/test/Templates.Test/SpaTemplateTest/SpaTemplateTestBase.cs index cf00f6eb52..d9a62356c5 100644 --- a/test/Templates.Test/SpaTemplateTest/SpaTemplateTestBase.cs +++ b/test/Templates.Test/SpaTemplateTest/SpaTemplateTestBase.cs @@ -29,13 +29,14 @@ namespace Templates.Test.SpaTemplateTest // build time, but by doing it up front we can avoid having multiple NPM // installs run concurrently which otherwise causes errors when tests run // in parallel. - if (File.Exists(Path.Combine(TemplateOutputDir, "ClientApp", "package.json"))) + var clientAppSubdirPath = Path.Combine(TemplateOutputDir, "ClientApp"); + if (File.Exists(Path.Combine(clientAppSubdirPath, "package.json"))) { - InstallNpmPackages("ClientApp"); + Npm.RestoreWithRetry(Output, clientAppSubdirPath); } else if (File.Exists(Path.Combine(TemplateOutputDir, "package.json"))) { - InstallNpmPackages("."); + Npm.RestoreWithRetry(Output, TemplateOutputDir); } TestApplication(targetFrameworkOverride, publish: false);