Skip to content

Commit

Permalink
test: remove common.isOSXMojave
Browse files Browse the repository at this point in the history
common.isOSXMojave was added because it was believed that there was a
bug in macOS Mojave that allowed unprivileged users to bind to
privileged ports. As it turns out, that was a feature not a bug. It is
likely to be in all future versions of macOS. Remove isOSXMojave and
skip appropriate tests based on isOSX.

Refs: https://news.ycombinator.com/item?id=18302380
PR-URL: #25658
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
  • Loading branch information
Trott authored and addaleax committed Jan 28, 2019
1 parent c93e5e1 commit 1905f8e
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 7 deletions.
3 changes: 0 additions & 3 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ const isOpenBSD = process.platform === 'openbsd';
const isLinux = process.platform === 'linux';
const isOSX = process.platform === 'darwin';

const isOSXMojave = isOSX && (os.release().startsWith('18'));

const enoughTestMem = os.totalmem() > 0x70000000; /* 1.75 Gb */
const cpus = os.cpus();
const enoughTestCpu = Array.isArray(cpus) &&
Expand Down Expand Up @@ -750,7 +748,6 @@ module.exports = {
isMainThread,
isOpenBSD,
isOSX,
isOSXMojave,
isSunOS,
isWindows,
localIPv6Hosts,
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-cluster-bind-privileged-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
const common = require('../common');

// Skip on OS X Mojave. https://github.com/nodejs/node/issues/21679
if (common.isOSXMojave)
common.skip('bypass test for Mojave due to OSX issue');
if (common.isOSX)
common.skip('macOS may allow ordinary processes to use any port');

if (common.isWindows)
common.skip('not reliable on Windows.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
const common = require('../common');

// Skip on OS X Mojave. https://github.com/nodejs/node/issues/21679
if (common.isOSXMojave)
common.skip('bypass test for Mojave due to OSX issue');
if (common.isOSX)
common.skip('macOS may allow ordinary processes to use any port');

if (common.isWindows)
common.skip('not reliable on Windows');
Expand Down

0 comments on commit 1905f8e

Please sign in to comment.