From abb08ae770d86efa90c3d06776dff9a20a29f285 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 23 Sep 2019 15:17:49 -0700 Subject: [PATCH] Fixed Node's prune_dev_dependencies build property based scenarios and added verification tests (#360) --- .../Node/NodeBashBuildSnippet.sh.tpl | 50 +++++----- .../Node/NodeJsSampleAppsOtherTests.cs | 97 +++++++++++++++++++ .../Node/NodeTestUsingZippedNodeModules.cs | 42 ++++++++ .../Node/NodeWebFrontEndAppTest.cs | 89 +++++++++++++++++ .../nodejs/webfrontend-yarnlock/server.js | 2 +- 5 files changed, 255 insertions(+), 25 deletions(-) diff --git a/src/BuildScriptGenerator/Node/NodeBashBuildSnippet.sh.tpl b/src/BuildScriptGenerator/Node/NodeBashBuildSnippet.sh.tpl index 5009cdc50..364b78c91 100644 --- a/src/BuildScriptGenerator/Node/NodeBashBuildSnippet.sh.tpl +++ b/src/BuildScriptGenerator/Node/NodeBashBuildSnippet.sh.tpl @@ -33,25 +33,25 @@ fi # if node modules exist separately for dev & prod (like from an earlier build), # rename the folders back appropriately for the current build -if [ -d $allModulesDirName ] +if [ -d "$allModulesDirName" ] then - if [ -d node_modules ] - then - if [ "$copyOnlyProdModulesToOutput" == "true" ] - then - # Rename existing node_modules back to prod modules since current build wants them separate - mv node_modules $prodModulesDirName - else - rm -rf node_modules - fi - fi - - # Rename the folder which has all the node modules to reuse for later builds to improve perf - mv $allModulesDirName node_modules + echo + echo "Found existing folder '$SOURCE_DIR/$allModulesDirName'." + echo "Copying modules from '$SOURCE_DIR/$allModulesDirName' to '$SOURCE_DIR/node_modules'..." + cd "$SOURCE_DIR" + mkdir -p node_modules + rsync -rtE --links "$allModulesDirName/" node_modules fi if [ "$copyOnlyProdModulesToOutput" == "true" ] then + # Delete existing prod modules folder so that we do not publish + # any unused modules to final destination directory. + if [ -d "$prodModulesDirName" ]; then + echo "Found existing '$SOURCE_DIR/$prodModulesDirName'. Deleting it..." + rm -rf "$prodModulesDirName" + fi + mkdir -p "$prodModulesDirName" cd "$prodModulesDirName" @@ -68,15 +68,16 @@ then fi echo - echo "Installing production dependencies in '$prodModulesDirName'..." + echo "Installing production dependencies in '$SOURCE_DIR/$prodModulesDirName'..." + echo echo "Running '{{ ProductionOnlyPackageInstallCommand }}'..." echo {{ ProductionOnlyPackageInstallCommand }} echo - echo "Copying production dependencies from '$prodModulesDirName' to '$SOURCE_DIR'..." + echo "Copying production dependencies from '$SOURCE_DIR/$prodModulesDirName' to '$SOURCE_DIR/node_modules'..." START_TIME=$SECONDS - rsync -rtE --links node_modules "$SOURCE_DIR" + rsync -rtE --links "node_modules/" "$SOURCE_DIR/node_modules" ELAPSED_TIME=$(($SECONDS - $START_TIME)) echo "Done in $ELAPSED_TIME sec(s)." fi @@ -115,12 +116,13 @@ npm pack if [ "$copyOnlyProdModulesToOutput" == "true" ] then - # Rename folder having all node_modules as we want only prod dependencies - # to be synced with destination directory - mv node_modules $allModulesDirName + echo + echo "Copy '$SOURCE_DIR/node_modules' with all dependencies to '$SOURCE_DIR/$allModulesDirName'..." + rsync -rtE --links "node_modules/" "$allModulesDirName" --delete - # Rename the folder having prod modules to be the one which we want to be present in output directory - mv $prodModulesDirName node_modules + echo + echo "Copying production dependencies from '$SOURCE_DIR/$prodModulesDirName/node_modules' to '$SOURCE_DIR/node_modules'..." + rsync -rtE --links "$prodModulesDirName/node_modules/" node_modules --delete fi {{ if CompressNodeModulesCommand | IsNotBlank }} @@ -128,14 +130,14 @@ if [ "$SOURCE_DIR" != "$DESTINATION_DIR" ] then if [ -f $zippedModulesFileName ]; then echo - echo "File '$zippedModulesFileName' already exists under '$SOURCE_DIR'. Deleting it ..." + echo "File '$zippedModulesFileName' already exists under '$SOURCE_DIR'. Deleting it..." rm -f $zippedModulesFileName fi if [ -d node_modules ] then echo - echo Zipping existing 'node_modules' folder ... + echo Zipping existing 'node_modules' folder... START_TIME=$SECONDS # Make the contents of the node_modules folder appear in the zip file, not the folder itself cd node_modules diff --git a/tests/Oryx.BuildImage.Tests/Node/NodeJsSampleAppsOtherTests.cs b/tests/Oryx.BuildImage.Tests/Node/NodeJsSampleAppsOtherTests.cs index 7f438dac4..373ff1cfb 100644 --- a/tests/Oryx.BuildImage.Tests/Node/NodeJsSampleAppsOtherTests.cs +++ b/tests/Oryx.BuildImage.Tests/Node/NodeJsSampleAppsOtherTests.cs @@ -10,6 +10,7 @@ using System.Runtime.InteropServices; using Microsoft.Oryx.BuildScriptGenerator.Node; using Microsoft.Oryx.Common; using Microsoft.Oryx.Tests.Common; +using Newtonsoft.Json; using Xunit; using Xunit.Abstractions; @@ -27,12 +28,16 @@ namespace Microsoft.Oryx.BuildImage.Tests public void GeneratesScript_AndBuilds(string buildImageName) { // Arrange + var devPackageName = "nodemon"; + var prodPackageName = "express"; var volume = CreateWebFrontEndVolume(); var appDir = volume.ContainerDir; var appOutputDir = "/tmp/webfrontend-output"; var script = new ShellScriptBuilder() .AddBuildCommand($"{appDir} -i /tmp/int -o {appOutputDir}") .AddDirectoryExistsCheck($"{appOutputDir}/node_modules") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules/{devPackageName}") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules/{prodPackageName}") .ToString(); // Act @@ -639,5 +644,97 @@ namespace Microsoft.Oryx.BuildImage.Tests }, result.GetDebugInfo()); } + + [Theory] + [InlineData("webfrontend")] + [InlineData("webfrontend-yarnlock")] + public void BuildsNodeApp_AndDoesNotCopyDevDependencies_IfPruneDevDependenciesIsTrue(string appName) + { + // Arrange + var volume = DockerVolume.CreateMirror(Path.Combine(_hostSamplesDir, "nodejs", appName)); + + // Make sure there is a package in devDependencies node that we verify is not copied to + // destination folder + var unexpectedPackageName = "nodemon"; + var expectedPackageName = "express"; + var packageJsonContent = File.ReadAllText(Path.Combine(volume.OriginalHostDir, "package.json")); + dynamic packageJson = JsonConvert.DeserializeObject(packageJsonContent); + Assert.NotNull(packageJson); + Assert.NotNull(packageJson.devDependencies); + Assert.NotNull(packageJson.devDependencies.nodemon); + + var appDir = volume.ContainerDir; + var appOutputDir = "/tmp/webfrontend-output"; + var script = new ShellScriptBuilder() + .AddBuildCommand( + $"{appDir} -i /tmp/int -o {appOutputDir} -p {NodePlatform.PruneDevDependenciesPropertyKey}=true") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules") + .AddFileDoesNotExistCheck($"{appOutputDir}/node_modules.zip") + .AddDirectoryDoesNotExistCheck($"{appOutputDir}/node_modules/{unexpectedPackageName}") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules/{expectedPackageName}") + .ToString(); + + // Act + var result = _dockerCli.Run(new DockerRunArguments + { + ImageId = Settings.BuildImageName, + Volumes = new List { volume }, + CommandToExecuteOnRun = "/bin/bash", + CommandArguments = new[] { "-c", script } + }); + + // Assert + RunAsserts( + () => + { + Assert.True(result.IsSuccess); + }, + result.GetDebugInfo()); + } + + [Fact] + public void BuildsNodeApp_DoesNotPruneDevDependencies_IfPruneDevDependenciesIsFalse() + { + // Arrange + var volume = DockerVolume.CreateMirror(Path.Combine(_hostSamplesDir, "nodejs", "webfrontend")); + + // Make sure there is a package in devDependencies node that we verify is still copied to + // destination folder + var devPackageName = "nodemon"; + var prodPackageName = "express"; + var packageJsonContent = File.ReadAllText(Path.Combine(volume.OriginalHostDir, "package.json")); + dynamic packageJson = JsonConvert.DeserializeObject(packageJsonContent); + Assert.NotNull(packageJson); + Assert.NotNull(packageJson.devDependencies); + Assert.NotNull(packageJson.devDependencies.nodemon); + + var appDir = volume.ContainerDir; + var appOutputDir = "/tmp/webfrontend-output"; + var script = new ShellScriptBuilder() + .AddBuildCommand( + $"{appDir} -i /tmp/int -o {appOutputDir} " + + $"-p {NodePlatform.PruneDevDependenciesPropertyKey}=false") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules/{devPackageName}") + .AddDirectoryExistsCheck($"{appOutputDir}/node_modules/{prodPackageName}") + .ToString(); + + // Act + var result = _dockerCli.Run(new DockerRunArguments + { + ImageId = Settings.BuildImageName, + Volumes = new List { volume }, + CommandToExecuteOnRun = "/bin/bash", + CommandArguments = new[] { "-c", script } + }); + + // Assert + RunAsserts( + () => + { + Assert.True(result.IsSuccess); + }, + result.GetDebugInfo()); + } } } \ No newline at end of file diff --git a/tests/Oryx.Integration.Tests/Node/NodeTestUsingZippedNodeModules.cs b/tests/Oryx.Integration.Tests/Node/NodeTestUsingZippedNodeModules.cs index c7e2457dd..62c051d51 100644 --- a/tests/Oryx.Integration.Tests/Node/NodeTestUsingZippedNodeModules.cs +++ b/tests/Oryx.Integration.Tests/Node/NodeTestUsingZippedNodeModules.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. // -------------------------------------------------------------------------------------------- +using Microsoft.Oryx.BuildScriptGenerator.Node; using Microsoft.Oryx.Common; using Microsoft.Oryx.Tests.Common; using System; @@ -122,5 +123,46 @@ namespace Microsoft.Oryx.Integration.Tests }); } + [Fact] + public async Task BuildsAndRunsNodeApp_WhenPruneDevDependenciesIsTrue_AndNodeModulesAreCompressed() + { + // Arrange + // Use a separate volume for output due to rsync errors + var appOutputDirPath = Directory.CreateDirectory(Path.Combine(_tempRootDir, Guid.NewGuid().ToString("N"))) + .FullName; + var nodeVersion = "10"; + var appOutputDirVolume = DockerVolume.CreateMirror(appOutputDirPath); + var appOutputDir = appOutputDirVolume.ContainerDir; + var appName = "webfrontend"; + var volume = CreateAppVolume(appName); + var appDir = volume.ContainerDir; + var runAppScript = new ShellScriptBuilder() + .AddCommand($"oryx -appPath {appOutputDir} -bindPort {ContainerPort}") + .AddCommand(DefaultStartupFilePath) + .ToString(); + var buildScript = new ShellScriptBuilder() + .AddCommand( + $"oryx build {appDir} -i /tmp/int -o /tmp/out --platform nodejs " + + $"--platform-version {nodeVersion} -p {NodePlatform.CompressNodeModulesPropertyKey}=zip" + + $" -p {NodePlatform.PruneDevDependenciesPropertyKey}=true") + .AddCommand($"cp -rf /tmp/out/* {appOutputDir}") + .ToString(); + + await EndToEndTestHelper.BuildRunAndAssertAppAsync( + appName, + _output, + new List { appOutputDirVolume, volume }, Settings.SlimBuildImageName, + "/bin/bash", + new[] { "-c", buildScript }, + $"oryxdevmcr.azurecr.io/public/oryx/node-{nodeVersion}", + ContainerPort, + "/bin/sh", + new[] { "-c", runAppScript }, + async (hostPort) => + { + var data = await _httpClient.GetStringAsync($"http://localhost:{hostPort}/"); + Assert.Contains("Say It Again", data); + }); + } } } \ No newline at end of file diff --git a/tests/Oryx.Integration.Tests/Node/NodeWebFrontEndAppTest.cs b/tests/Oryx.Integration.Tests/Node/NodeWebFrontEndAppTest.cs index 7bd77e7f1..6a1378c41 100644 --- a/tests/Oryx.Integration.Tests/Node/NodeWebFrontEndAppTest.cs +++ b/tests/Oryx.Integration.Tests/Node/NodeWebFrontEndAppTest.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. // -------------------------------------------------------------------------------------------- +using Microsoft.Oryx.BuildScriptGenerator.Node; using Microsoft.Oryx.Common; using Microsoft.Oryx.Tests.Common; using System; @@ -62,5 +63,93 @@ namespace Microsoft.Oryx.Integration.Tests Assert.Contains("Say It Again", data); }); } + + [Theory] + [InlineData("webfrontend")] + [InlineData("webfrontend-yarnlock")] + public async Task CanBuildAndRun_NodeWebFrontEndApp_WhenPruneDevDependenciesIsTrue(string appName) + { + // Arrange + var nodeVersion = "10"; + var volume = CreateAppVolume(appName); + var appDir = volume.ContainerDir; + var buildScript = new ShellScriptBuilder() + .AddCommand( + $"oryx build {appDir} --platform nodejs --platform-version {nodeVersion} " + + $"-p {NodePlatform.PruneDevDependenciesPropertyKey}=true") + .ToString(); + var runScript = new ShellScriptBuilder() + .AddCommand($"oryx -appPath {appDir} -bindPort {ContainerPort}") + .AddCommand(DefaultStartupFilePath) + .ToString(); + + await EndToEndTestHelper.BuildRunAndAssertAppAsync( + appName, + _output, + volume, + "/bin/sh", + new[] + { + "-c", + buildScript + }, + $"oryxdevmcr.azurecr.io/public/oryx/node-{nodeVersion}", + ContainerPort, + "/bin/sh", + new[] + { + "-c", + runScript + }, + async (hostPort) => + { + var data = await _httpClient.GetStringAsync($"http://localhost:{hostPort}/"); + Assert.Contains("Say It Again", data); + }); + } + + [Fact] + public async Task CanBuildAndRun_NodeWebFrontEndApp_AfterRebuild_WhenPruneDevDependenciesIsTrue() + { + // Arrange + var nodeVersion = "10"; + var appName = "webfrontend"; + var volume = CreateAppVolume(appName); + var appDir = volume.ContainerDir; + var buildCommand = $"oryx build {appDir} --platform nodejs --platform-version {nodeVersion} " + + $"-p {NodePlatform.PruneDevDependenciesPropertyKey}=true"; + var buildScript = new ShellScriptBuilder() + .AddCommand(buildCommand) + .AddCommand(buildCommand) + .ToString(); + var runScript = new ShellScriptBuilder() + .AddCommand($"oryx -appPath {appDir} -bindPort {ContainerPort}") + .AddCommand(DefaultStartupFilePath) + .ToString(); + + await EndToEndTestHelper.BuildRunAndAssertAppAsync( + appName, + _output, + volume, + "/bin/sh", + new[] + { + "-c", + buildScript + }, + $"oryxdevmcr.azurecr.io/public/oryx/node-{nodeVersion}", + ContainerPort, + "/bin/sh", + new[] + { + "-c", + runScript + }, + async (hostPort) => + { + var data = await _httpClient.GetStringAsync($"http://localhost:{hostPort}/"); + Assert.Contains("Say It Again", data); + }); + } } } \ No newline at end of file diff --git a/tests/SampleApps/nodejs/webfrontend-yarnlock/server.js b/tests/SampleApps/nodejs/webfrontend-yarnlock/server.js index 887efacef..d05749b64 100644 --- a/tests/SampleApps/nodejs/webfrontend-yarnlock/server.js +++ b/tests/SampleApps/nodejs/webfrontend-yarnlock/server.js @@ -10,7 +10,7 @@ app.get('/api', function (req, res) { res.send('Hello from webfrontend'); }); -var port = process.env.PORT || 80; +var port = process.env.PORT || 8080; var server = app.listen(port, function () { console.log('Listening on port ' + port); });