Correct behavior for setting buildMode/NODE_ENV

tmeasday · tmeasday · commit 96c802edb410 · 2016-03-14T11:18:23.000+11:00
diff --git a/tools/runners/run-all.js b/tools/runners/run-all.js
@@ -280,20 +280,32 @@ exports.run = function (options) {
   // Ensure process.env.NODE_ENV matches the build mode, with the following precedence:
   // 1. Passed in build mode (if development or production)
   // 2. Existing process.env.NODE_ENV (if it's valid) 
-  // 3. Default to development
-  let nodeEnv = process.env.NODE_ENV;
+  // 3. Default to development (in both cases) otherwise
+
+  // NOTE: because this code only runs when using `meteor run` or `meteor test[-packages`,
+  // We *don't* end up defaulting NODE_ENV in this way when bundling/deploying.
+  // In those cases, it will default to "production" in packages/meteor/*_env.js
+
+  // We *override* NODE_ENV if build mode is one of these values
   let buildMode = runOptions.buildOptions.buildMode;
   if (buildMode === "development" || buildMode === "production") {
     process.env.NODE_ENV = buildMode;
-  } else if (nodeEnv === "development" || nodeEnv === "production") {
-    runOptions.buildOptions.buildMode = nodeEnv;
-  } else {
-    runOptions.buildOptions.buildMode = "development";
-    if (! nodeEnv) {
-      process.env.NODE_ENV = "development";
+  }
+
+  let nodeEnv = process.env.NODE_ENV;
+  // We *never* override buildMode (it can be "test")
+  if (!buildMode) {
+    if (nodeEnv === "development" || nodeEnv === "production") {
+      runOptions.buildOptions.buildMode = nodeEnv;
+    } else {
+      runOptions.buildOptions.buildMode = "development";
     }
   }
 
+  if (!nodeEnv) {
+    process.env.NODE_ENV = "development";
+  }
+
   var runner = new Runner(runOptions);
   runner.start();
   var result = promise.await();
