Merge pull request #270 from heroku/cache-strict-with-bower

Cache strict with bower

CHANGELOG.md

@@ -2,7 +2,11 @@
 
 ## Pending
 
-Fixes modules-checked-in reference URL
+Caching improvements
+
+- Fixes modules-checked-in reference URL
+- When cache restoration is disabled, empties the cache instead of saving it
+- Adds bower_components as a default cache target
 
 ## v82 (2015-09-30)
 

bin/compile

@@ -8,6 +8,10 @@ set -o pipefail   # don't ignore exit codes when piping output
 set -o nounset    # fail on unset variables
 unset GIT_DIR     # Avoid GIT_DIR leak from previous build steps
 
+### Constants
+
+DEFAULT_CACHE="node_modules bower_components"
+
 ### Configure directories
 
 BUILD_DIR=${1:-}
@@ -99,14 +103,14 @@ restore_cache() {
   if [ "$cache_status" == "valid" ]; then
     local cache_directories=$(get_cache_directories)
     if [ "$cache_directories" == "" ]; then
-      echo "Loading 1 from cacheDirectories (default):"
-      restore_cache_directories "$BUILD_DIR" "$CACHE_DIR" "node_modules"
+      echo "Loading 2 from cacheDirectories (default):"
+      restore_cache_directories "$BUILD_DIR" "$CACHE_DIR" "$DEFAULT_CACHE"
     else
       echo "Loading $(echo $cache_directories | wc -w | xargs) from cacheDirectories (package.json):"
       restore_cache_directories "$BUILD_DIR" "$CACHE_DIR" $cache_directories
     fi
   else
-    echo "Skipping cache ($cache_status)"
+    echo "Skipping cache restore ($cache_status)"
   fi
 }
 
@@ -130,9 +134,11 @@ cache_build() {
 
   echo "Clearing previous node cache"
   clear_cache
-  if [ "$cache_directories" == "" ]; then
-    echo "Saving 1 cacheDirectories (default):"
-    save_cache_directories "$BUILD_DIR" "$CACHE_DIR" "node_modules"
+  if ! ${NODE_MODULES_CACHE:-true}; then
+    echo "Skipping cache save (disabled by config)"
+  elif [ "$cache_directories" == "" ]; then
+    echo "Saving 2 cacheDirectories (default):"
+    save_cache_directories "$BUILD_DIR" "$CACHE_DIR" "$DEFAULT_CACHE"
   else
     echo "Saving $(echo $cache_directories | wc -w | xargs) cacheDirectories (package.json):"
     save_cache_directories "$BUILD_DIR" "$CACHE_DIR" $cache_directories

lib/failure.sh

@@ -72,9 +72,11 @@ warn_untracked_dependencies() {
   local log_file="$1"
   if grep -qi 'gulp: not found' "$log_file"; then
     warning "Gulp may not be tracked in package.json" "https://devcenter.heroku.com/articles/troubleshooting-node-deploys#ensure-you-aren-t-relying-on-untracked-dependencies"
-  elif grep -qi 'grunt: not found' "$log_file"; then
+  fi
+  if grep -qi 'grunt: not found' "$log_file"; then
     warning "Grunt may not be tracked in package.json" "https://devcenter.heroku.com/articles/troubleshooting-node-deploys#ensure-you-aren-t-relying-on-untracked-dependencies"
-  elif grep -qi 'bower: not found' "$log_file"; then
+  fi
+  if grep -qi 'bower: not found' "$log_file"; then
     warning "Bower may not be tracked in package.json" "https://devcenter.heroku.com/articles/troubleshooting-node-deploys#ensure-you-aren-t-relying-on-untracked-dependencies"
   fi
 }

test/run

 #!/usr/bin/env bash
 # See README.md for info on running these tests.
 
+testDisableCache() {
+  cache=$(mktmpdir)
+  env_dir=$(mktmpdir)
+
+  compile "node-modules-cache-1" $cache
+  assertCaptured "lodash@1.0.0"
+  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep lodash | wc -l | tr -d ' ')"
+  assertCapturedSuccess
+
+  compile "node-modules-cache-2" $cache
+  assertCaptured "lodash@1.0.0"
+  assertCaptured "Saving 2 cacheDirectories"
+  assertCapturedSuccess
+
+  echo "false" > $env_dir/NODE_MODULES_CACHE
+  compile "node-modules-cache-2" $cache $env_dir
+  assertCaptured "lodash@1.3.1"
+  assertNotCaptured "Saving 2 cacheDirectories"
+  assertCapturedSuccess
+}
+
 testBowerAngularResolution() {
   compile "bower-angular-resolution"
   assertCaptured "Bower may need a resolution hint for angular"
@@ -21,6 +42,17 @@ testBadJson() {
   assertCapturedError 1 "Unable to parse"
 }
 
+testNodeModulesCached() {
+  cache=$(mktmpdir)
+
+  compile "caching" $cache
+  assertCaptured "Saving 2 cacheDirectories (default)"
+  assertCaptured "- node_modules"
+  assertCaptured "- bower_components (nothing to cache)"
+  assertEquals "1" "$(ls -1 $cache/node | grep node_modules | wc -l | tr -d ' ')"
+  assertCapturedSuccess
+}
+
 testBuildWithUserCacheDirectoriesCamel() {
   cache=$(mktmpdir)
 
@@ -98,7 +130,7 @@ testBuildWithCache() {
   cache=$(mktmpdir)
 
   compile "stable-node" $cache
-  assertCaptured "Skipping cache (new runtime"
+  assertCaptured "Skipping cache restore (new runtime"
   assertEquals "1" "$(ls -1 $cache/node | grep node_modules | wc -l | tr -d ' ')"
   assertCapturedSuccess
 
@@ -122,7 +154,7 @@ testSignatureInvalidation() {
 
   compile "node-0.12.7" $cache
   assertCaptured "Downloading and installing node 0.12.7"
-  assertCaptured "Skipping cache (new runtime"
+  assertCaptured "Skipping cache restore (new runtime"
   assertCapturedSuccess
 }
 
@@ -141,25 +173,6 @@ testModulesCheckedIn() {
   assertCapturedSuccess
 }
 
-testDisableCache() {
-  cache=$(mktmpdir)
-  env_dir=$(mktmpdir)
-
-  compile "node-modules-cache-1" $cache
-  assertCaptured "lodash@1.0.0"
-  assertEquals "1" "$(ls -1 $cache/node/node_modules | grep lodash | wc -l | tr -d ' ')"
-  assertCapturedSuccess
-
-  compile "node-modules-cache-2" $cache
-  assertCaptured "lodash@1.0.0"
-  assertCapturedSuccess
-
-  echo "false" > $env_dir/NODE_MODULES_CACHE
-  compile "node-modules-cache-2" $cache $env_dir
-  assertCaptured "lodash@1.3.1"
-  assertCapturedSuccess
-}
-
 testDetectWithPackageJson() {
   detect "stable-node"
   assertCaptured "Node.js"
@@ -312,16 +325,6 @@ testInvalidDependency() {
   assertCapturedError 1 ""
 }
 
-testNodeModulesCached() {
-  cache=$(mktmpdir)
-
-  compile "caching" $cache
-  assertCaptured "Saving 1 cacheDirectories (default)"
-  assertCaptured "- node_modules"
-  assertEquals "1" "$(ls -1 $cache/node | grep node_modules | wc -l | tr -d ' ')"
-  assertCapturedSuccess
-}
-
 testBuildWithUserCacheDirectories() {
   cache=$(mktmpdir)
 
@@ -338,10 +341,6 @@ testBuildWithUserCacheDirectories() {
   assertCapturedSuccess
 }
 
-
-
-
-
 testUserConfig() {
   compile "userconfig"
   assertCaptured "www.google.com"