Merge pull request #187 from heroku/test-bad-json

test and implementation for failing on incorrectly formatted json

bin/compile

@@ -2,9 +2,10 @@
 
 ####### Configure environment
 
-set -e            # fail fast
-set -o pipefail   # don't ignore exit codes when piping output
+set -o errexit    # always exit on error
 set -o errtrace   # trap errors in functions as well
+set -o pipefail   # don't ignore exit codes when piping output
+set -o posix      # more strict failures in subshells
 # set -x          # enable debugging
 
 # Configure directories
@@ -30,26 +31,7 @@ trap build_failed ERR
 ####### Determine current state
 
 head "Reading application state"
-
-info "package.json..."
-iojs_engine=$(read_json "$build_dir/package.json" ".engines.iojs")
-node_engine=$(read_json "$build_dir/package.json" ".engines.node")
-npm_engine=$(read_json "$build_dir/package.json" ".engines.npm")
-
-info "build directory..."
-start_method=$(get_start_method "$build_dir")
-modules_source=$(get_modules_source "$build_dir")
-
-info "cache directory..."
-npm_previous=$(file_contents "$cache_dir/node/npm-version")
-node_previous=$(file_contents "$cache_dir/node/node-version")
-modules_cached=$(get_modules_cached "$cache_dir")
-
-info "environment variables..."
-export_env_dir $env_dir
-export NPM_CONFIG_PRODUCTION=${NPM_CONFIG_PRODUCTION:-true}
-export NODE_MODULES_CACHE=${NODE_MODULES_CACHE:-true}
-
+read_current_state
 show_current_state
 
 if [ "$iojs_engine" == "" ]; then

bin/test

@@ -18,6 +18,15 @@ testDetectWithoutPackageJson() {
   assertCapturedError 1 ""
 }
 
+testBadJson() {
+  compile "bad-json"
+  assertCaptured "Build failed"
+  assertCaptured "We're sorry this build is failing"
+  assertNotCaptured "build directory..."
+  assertNotCaptured "Installing binaries"
+  assertCapturedError 1 "Unable to parse"
+}
+
 testIoJs() {
   compile "iojs"
   assertCaptured "Node engine:         1.0."
@@ -92,8 +101,10 @@ testNpmVersionSpecific() {
 
 testFailingBuild() {
   compile "failing-build"
+  assertCaptured "Building dependencies"
   assertCaptured "Build failed"
   assertCaptured "We're sorry this build is failing"
+  assertNotCaptured "Checking startup method"
   assertCapturedError 1 ""
 }
 
@@ -195,6 +206,7 @@ testProcfileAbsentNpmStartPresent() {
 testProcfileAbsentNpmStartAbsent() {
   compile "procfile-absent-npm-start-absent"
   assertCaptured "Start mechanism:     none"
+  assertCaptured "None found"
   assertNotCaptured "new Procfile"
   assertCapturedSuccess
 }

lib/build.sh

@@ -53,6 +53,39 @@ get_modules_cached() {
   fi
 }
 
+# Sets:
+# iojs_engine
+# node_engine
+# npm_engine
+# start_method
+# modules_source
+# npm_previous
+# node_previous
+# modules_cached
+# environment variables (from ENV_DIR)
+
+read_current_state() {
+  info "package.json..."
+  assert_json "$build_dir/package.json"
+  iojs_engine=$(read_json "$build_dir/package.json" ".engines.iojs")
+  node_engine=$(read_json "$build_dir/package.json" ".engines.node")
+  npm_engine=$(read_json "$build_dir/package.json" ".engines.npm")
+
+  info "build directory..."
+  start_method=$(get_start_method "$build_dir")
+  modules_source=$(get_modules_source "$build_dir")
+
+  info "cache directory..."
+  npm_previous=$(file_contents "$cache_dir/node/npm-version")
+  node_previous=$(file_contents "$cache_dir/node/node-version")
+  modules_cached=$(get_modules_cached "$cache_dir")
+
+  info "environment variables..."
+  export_env_dir $env_dir
+  export NPM_CONFIG_PRODUCTION=${NPM_CONFIG_PRODUCTION:-true}
+  export NODE_MODULES_CACHE=${NODE_MODULES_CACHE:-true}
+}
+
 show_current_state() {
   echo ""
   if [ "$iojs_engine" == "" ]; then
@@ -172,6 +205,8 @@ ensure_procfile() {
   elif [ "$start_method" == "server.js" ]; then
     info "No Procfile; Adding 'web: node server.js' to new Procfile"
     echo "web: node server.js" > $build_dir/Procfile
+  else
+    info "None found"
   fi
 }
 

lib/common.sh

 error() {
   echo " !     $*" >&2
-  exit 1
+  echo ""
+  return 1
 }
 
 head() {
@@ -27,6 +28,15 @@ achievement() {
   echo ""
 }
 
+assert_json() {
+  local file=$1
+  if test -f $file; then
+    if ! cat $file | $bp_dir/vendor/jq '.' > /dev/null; then
+      error "Unable to parse $file as JSON"
+    fi
+  fi
+}
+
 file_contents() {
   if test -f $1; then
     echo "$(cat $1)"
@@ -39,12 +49,7 @@ read_json() {
   local file=$1
   local node=$2
   if test -f $file; then
-    local result="$(cat $file | $bp_dir/vendor/jq -r $node)"
-    if [ "$result" == "null" ]; then
-      echo ""
-    else
-      echo "$result"
-    fi
+    cat $file | $bp_dir/vendor/jq --raw-output "$node // \"\"" || return 1
   else
     echo ""
   fi

test/bad-json/README.md

+A fake README, to keep npm from polluting stderr.
\ No newline at end of file

test/bad-json/package.json

+{
+  "name": "node-buildpack-test-app",
+  "version": "0.0.1",
+  "description": "node buildpack integration test app",
+  "repository" : {
+    "type" : "git",
+    "url" : "http://github.com/example/example.git"
+  },
+  "dependencies": {
+    "hashish": "*"
+    "express": "*"
+  },
+  "engines": {
+    "node": "0.10.29"
+  }
+}