test: remove common.getServiceName()

Trott · evanlucas · commit ccbc78cfc6a9 · 2016-05-17T08:52:44.000-05:00
Replace lightly-used services file parsing in favor of confirming one of a small number of allowable values in service name lookup tests. In nodejs/node-v0.x-archive#8047, it was decided that this sort of service file parsing was superior to hardcoding acceptable values, but I'm not convinced: * No guarantee that the host uses /etc/services before, e.g., nscd. * Increases complexity of tests without guaranteeing robustness. I think that simply checking against a small set of expected values may be a better solution. Ideally, there would also be a unit test that used a test double for the appropriate `cares` function and confirms that it is called with the correct parameters, but now we're getting way ahead of ourselves. PR-URL: #6709 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
diff --git a/test/common.js b/test/common.js
@@ -160,7 +160,7 @@ Object.defineProperty(exports, 'opensslCli', {get: function() {
     opensslCli = false;
   }
   return opensslCli;
-}, enumerable: true });
+}, enumerable: true});
 
 Object.defineProperty(exports, 'hasCrypto', {
   get: function() {
@@ -395,55 +395,6 @@ exports.mustCall = function(fn, expected) {
   };
 };
 
-var etcServicesFileName = path.join('/etc', 'services');
-if (exports.isWindows) {
-  etcServicesFileName = path.join(process.env.SystemRoot, 'System32', 'drivers',
-    'etc', 'services');
-}
-
-/*
- * Returns a string that represents the service name associated
- * to the service bound to port "port" and using protocol "protocol".
- *
- * If the service is not defined in the services file, it returns
- * the port number as a string.
- *
- * Returns undefined if /etc/services (or its equivalent on non-UNIX
- * platforms) can't be read.
- */
-exports.getServiceName = function getServiceName(port, protocol) {
-  if (port == null) {
-    throw new Error('Missing port number');
-  }
-
-  if (typeof protocol !== 'string') {
-    throw new Error('Protocol must be a string');
-  }
-
-  /*
-   * By default, if a service can't be found in /etc/services,
-   * its name is considered to be its port number.
-   */
-  var serviceName = port.toString();
-
-  try {
-    var servicesContent = fs.readFileSync(etcServicesFileName,
-      { encoding: 'utf8'});
-    var regexp = `^(\\w+)\\s+\\s${port}/${protocol}\\s`;
-    var re = new RegExp(regexp, 'm');
-
-    var matches = re.exec(servicesContent);
-    if (matches && matches.length > 1) {
-      serviceName = matches[1];
-    }
-  } catch (e) {
-    console.error('Cannot read file: ', etcServicesFileName);
-    return undefined;
-  }
-
-  return serviceName;
-};
-
 exports.hasMultiLocalhost = function hasMultiLocalhost() {
   var TCP = process.binding('tcp_wrap').TCP;
   var t = new TCP();
diff --git a/test/internet/test-dns-ipv4.js b/test/internet/test-dns-ipv4.js
@@ -1,5 +1,5 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const dns = require('dns');
 const net = require('net');
@@ -173,24 +173,7 @@ TEST(function test_lookupservice_ip_ipv4(done) {
     if (err) throw err;
     assert.equal(typeof host, 'string');
     assert(host);
-
-    /*
-     * Retrieve the actual HTTP service name as setup on the host currently
-     * running the test by reading it from /etc/services. This is not ideal,
-     * as the service name lookup could use another mechanism (e.g nscd), but
-     * it's already better than hardcoding it.
-     */
-    var httpServiceName = common.getServiceName(80, 'tcp');
-    if (!httpServiceName) {
-      /*
-       * Couldn't find service name, reverting to the most sensible default
-       * for port 80.
-       */
-      httpServiceName = 'http';
-    }
-
-    assert.strictEqual(service, httpServiceName);
-
+    assert(['http', 'www', '80'].includes(service));
     done();
   });
 
diff --git a/test/internet/test-dns-ipv6.js b/test/internet/test-dns-ipv6.js
@@ -182,24 +182,7 @@ TEST(function test_lookupservice_ip_ipv6(done) {
     if (err) throw err;
     assert.equal(typeof host, 'string');
     assert(host);
-
-    /*
-     * Retrieve the actual HTTP service name as setup on the host currently
-     * running the test by reading it from /etc/services. This is not ideal,
-     * as the service name lookup could use another mechanism (e.g nscd), but
-     * it's already better than hardcoding it.
-     */
-    var httpServiceName = common.getServiceName(80, 'tcp');
-    if (!httpServiceName) {
-      /*
-       * Couldn't find service name, reverting to the most sensible default
-       * for port 80.
-       */
-      httpServiceName = 'http';
-    }
-
-    assert.strictEqual(service, httpServiceName);
-
+    assert(['http', 'www', '80'].includes(service));
     done();
   });
 
