Fixed bug and added test coverage for case. Now obeys production s3_host option when package.json has only host key specified.

Author: ronilan

lib/util/versioning.js

@@ -367,9 +367,8 @@ module.exports.evaluate = function(package_json, options, napi_build_version) {
   // the environment variable has priority over the the command line.
   let targetHost = process.env.node_pre_gyp_s3_host || options.s3_host;
 
-  // if value is not one of the allowed or the matching key is not found in package.json
-  // silently ignore the option
-  if (['production', 'staging', 'development'].indexOf(targetHost) === -1 || !package_json.binary[`${targetHost}_host`]) {
+  // if value is not one of the allowed silently ignore the option
+  if (['production', 'staging', 'development'].indexOf(targetHost) === -1) {
     targetHost = '';
   }
 
@@ -379,17 +378,20 @@ module.exports.evaluate = function(package_json, options, napi_build_version) {
   let hostData = package_json.binary.host;
 
   // when a valid target is specified by user, the host is from that target (or 'host')
-  if (targetHost === 'staging') {
+  if (targetHost === 'production') {
+    // all set. catch case so as to not change host based on commands.
+  }
+  else if (targetHost === 'staging' && package_json.binary.staging_host) {
     hostData = package_json.binary.staging_host;
-  } else if (targetHost === 'development') {
+  } else if (targetHost === 'development' && package_json.binary.development_host) {
     hostData = package_json.binary.development_host;
-  } else if (!targetHost && (package_json.binary.development_host || package_json.binary.staging_host)) {
+  } else if ((package_json.binary.development_host || package_json.binary.staging_host)) {
     // when host not specifically set via command line or environment variable
     // but staging and/or development host are present in package.json
     // for any command (or command chain) that includes publish or unpublish
     // default to lower host (development, and if not preset, staging).
     if (options.argv && options.argv.remain.some((item) => (item === 'publish' || item === 'unpublish'))) {
-      if (package_json.binary.development_host) {
+      if (!targetHost && package_json.binary.development_host) {
         hostData = package_json.binary.development_host;
       } else if (package_json.binary.staging_host) {
         hostData = package_json.binary.staging_host;

test/versioning.test.js

@@ -759,6 +759,66 @@ test('should use host specified by the --s3_host option', (t) => {
     };
   };
 
+  const parsed_package_json = {
+    name: 'test',
+    main: 'test.js',
+    version: '0.1.0',
+    binary: {
+      module_name: 'binary-module-name',
+      module_path: 'binary-module-path',
+      host: 's3-production-path',
+      development_host: 's3-development-path',
+      staging_host: 's3-staging-path'
+    }
+  };
+
+  const hosts = ['production', 'staging', 'development'];
+  const cmds = ['install', 'info', 'publish', 'unpublish'];
+  cmds.forEach((cmd) => {
+    hosts.forEach((host) => {
+      const checkAgainst = host !== 'production' ? `${host}_host` : 'host';
+      const cloned = JSON.parse(JSON.stringify(parsed_package_json));
+      const opts = versioning.evaluate(cloned, makeOoptions(cmd, host));
+
+      t.equal(opts.host, parsed_package_json.binary[checkAgainst] + '/');
+      t.equal(opts.hosted_path, parsed_package_json.binary[checkAgainst] + '/');
+      t.equal(opts.hosted_tarball, parsed_package_json.binary[checkAgainst] + '/' + opts.package_name);
+    });
+  });
+  cmds.forEach((cmd) => {
+    hosts.forEach((host) => {
+      parsed_package_json.binary = {
+        module_name: 'binary-module-name',
+        module_path: 'binary-module-path',
+        host: { endpoint: 's3-production-path' },
+        development_host: { endpoint: 's3-development-path' },
+        staging_host: { endpoint: 's3-staging-path' }
+      };
+
+      const checkAgainst = host !== 'production' ? `${host}_host` : 'host';
+      const cloned = JSON.parse(JSON.stringify(parsed_package_json));
+      const opts = versioning.evaluate(cloned, makeOoptions(cmd, host));
+
+      t.equal(opts.host, parsed_package_json.binary[checkAgainst].endpoint + '/');
+      t.equal(opts.hosted_path, parsed_package_json.binary[checkAgainst].endpoint + '/');
+      t.equal(opts.hosted_tarball, parsed_package_json.binary[checkAgainst].endpoint + '/' + opts.package_name);
+    });
+  });
+  t.end();
+});
+
+test('should use host specified by the --s3_host option (production_host used)', (t) => {
+  const makeOoptions = (cmd, host) => {
+    return {
+      s3_host: host,
+      argv: {
+        remain: [cmd],
+        cooked: [cmd, '--s3_host', host],
+        original: [cmd, `--s3_host=${host}`]
+      }
+    };
+  };
+
   const parsed_package_json = {
     name: 'test',
     main: 'test.js',