Skip to content

Commit

Permalink
tests: parallel tests
Browse files Browse the repository at this point in the history
Several changes to the test suite to support running tests in parallel,
bringing the overall test time down considerably.

- Replace all literal 1337 and 1234 ports with a custom per-process port
  assignment based on the TAP_CHILD_ID environment variable.
- Add common.pkg as a per-test working directory instead of polluting
  __dirname or accidentally reusing the same working directory for
  multiple tests.
- Rework test config handling so that tests don't rely on config setup
  being run in a particular order.
- Remove the npm-registry-couchapp tests, since it (a) relies on
  CouchDB, (b) is no longer a reliable indicator of registry
  compatibility, and (c) is already superceded in most cases by tests
  that use npm-registry-mock.  (A test suite that runs against a
  reference implementation is a thing that should exist, but not here.)
- Remove the fake-registry logging when TAP_CHILD_ID is set, since this
  is extremely hard to make sense of when running multiple tests in
  parallel.

When Node v6 compatibility is dropped in npm v7, we can upgrade to the
latest version of tap for a bit more speed, dropping Domains (and the
associated deprecation warnings), and a fancier test reporter.
  • Loading branch information
isaacs committed Jun 26, 2019
1 parent d4a76a2 commit bec8272
Show file tree
Hide file tree
Showing 266 changed files with 553 additions and 440 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ npm-debug.log
.jshintrc
.eslintrc
.nyc_output
/test/npm_cache*
/node_modules/.cache
16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "6.9.1-next.0",
"version": "6.9.0",
"name": "npm",
"description": "a package manager for JavaScript",
"keywords": [
Expand Down Expand Up @@ -108,7 +108,7 @@
"path-is-inside": "~1.0.2",
"promise-inflight": "~1.0.1",
"qrcode-terminal": "^0.12.0",
"query-string": "^6.4.0",
"query-string": "^6.2.0",
"qw": "~1.0.1",
"read": "~1.0.7",
"read-cmd-shim": "~1.0.1",
Expand All @@ -121,7 +121,7 @@
"rimraf": "^2.6.3",
"safe-buffer": "^5.1.2",
"semver": "^5.6.0",
"sha": "^3.0.0",
"sha": "~2.0.1",
"slide": "~1.1.6",
"sorted-object": "~2.0.1",
"sorted-union-stream": "~2.1.3",
Expand Down Expand Up @@ -276,17 +276,17 @@
"require-inject": "^1.4.4",
"sprintf-js": "^1.1.2",
"standard": "^11.0.1",
"tacks": "^1.3.0",
"tap": "^12.7.0",
"tar-stream": "^2.0.1"
"tacks": "^1.2.7",
"tap": "~12.7.0",
"tar-stream": "^2.0.0"
},
"scripts": {
"dumpconf": "env | grep npm | sort | uniq",
"prepare": "node bin/npm-cli.js rebuild && node bin/npm-cli.js --no-audit --no-timing prune --prefix=. --no-global && rimraf test/*/*/node_modules && make -j4 doc",
"preversion": "bash scripts/update-authors.sh && git add AUTHORS && git commit -m \"update AUTHORS\" || true",
"licenses": "licensee --production --errors-only",
"tap": "tap --reporter=classic --timeout 300",
"tap-cover": "tap --reporter=classic --nyc-arg='--cache' --coverage --timeout 600",
"tap": "tap -J --timeout 300",
"tap-cover": "tap -J --nyc-arg=--cache --coverage --timeout 600",
"test": "standard && npm run test-tap",
"test-coverage": "npm run tap-cover -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",
"test-tap": "npm run tap -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",
Expand Down
90 changes: 90 additions & 0 deletions test/common-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
if (module === require.main) {
console.log('1..1')
console.log('ok 1 setup done')
process.exit(0)
}

var fs = require('graceful-fs')
var path = require('path')
var userconfigSrc = path.resolve(__dirname, 'fixtures', 'config', 'userconfig')
exports.userconfig = userconfigSrc + '-with-gc'
exports.globalconfig = path.resolve(__dirname, 'fixtures', 'config', 'globalconfig')

// if this hasn't been written yet, then do it now.
try {
fs.statSync(exports.userconfig)
} catch (er) {
var uc = fs.readFileSync(userconfigSrc)
var gcini = 'globalconfig = ' + exports.globalconfig + '\n'
// atomic!
fs.writeFileSync(exports.userconfig + '.' + process.pid, gcini + uc)
fs.renameSync(exports.userconfig + '.' + process.pid, exports.userconfig)
}

exports.builtin = path.resolve(__dirname, 'fixtures', 'config', 'builtin')
exports.malformed = path.resolve(__dirname, 'fixtures', 'config', 'malformed')
exports.ucData =
{ globalconfig: exports.globalconfig,
email: 'i@izs.me',
'env-thing': 'asdf',
'init.author.name': 'Isaac Z. Schlueter',
'init.author.email': 'i@izs.me',
'init.author.url': 'http://blog.izs.me/',
'init.version': '1.2.3',
'npm:publishtest': true,
'_npmjs.org:couch': 'https://admin:password@localhost:5984/registry',
'npm-www:nocache': '1',
nodedir: '/Users/isaacs/dev/js/node-v0.8',
'sign-git-tag': true,
message: 'v%s',
'strict-ssl': false,
'tmp': path.normalize(process.env.HOME + '/.tmp'),
_auth: 'dXNlcm5hbWU6cGFzc3dvcmQ=',
_token:
{ AuthSession: 'yabba-dabba-doodle',
version: '1',
expires: '1345001053415',
path: '/',
httponly: true } }

// set the userconfig in the env
// unset anything else that npm might be trying to foist on us
Object.keys(process.env).forEach(function (k) {
if (k.match(/^npm_config_/i)) {
delete process.env[k]
}
})
process.env.npm_config_userconfig = exports.userconfig
process.env.npm_config_other_env_thing = '1000'
process.env.random_env_var = 'asdf'
process.env.npm_config__underbar_env_thing = 'underful'
process.env.NPM_CONFIG_UPPERCASE_ENV_THING = '42'

exports.envData = {
userconfig: exports.userconfig,
'_underbar-env-thing': 'underful',
'uppercase-env-thing': '42',
'other-env-thing': '1000'
}
exports.envDataFix = {
userconfig: exports.userconfig,
'_underbar-env-thing': 'underful',
'uppercase-env-thing': 42,
'other-env-thing': 1000
}

var projectConf = path.resolve(__dirname, '..', '.npmrc')
try {
fs.statSync(projectConf)
} catch (er) {
// project conf not found, probably working with packed npm
fs.writeFileSync(projectConf, '')
}

var projectRc = path.join(__dirname, 'fixtures', 'config', '.npmrc')
try {
fs.statSync(projectRc)
} catch (er) {
// project conf not found, probably working with packed npm
fs.writeFileSync(projectRc, 'just = testing')
}
32 changes: 28 additions & 4 deletions test/common-tap.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'
/* eslint-disable camelcase */

const configCommon = require('./common-config.js')
var fs = require('graceful-fs')
var readCmdShim = require('read-cmd-shim')
var isWindows = require('../lib/utils/is-windows.js')
Expand All @@ -17,9 +18,32 @@ if (!global.setImmediate || !require('timers').setImmediate) {
var spawn = require('child_process').spawn
var path = require('path')

var port = exports.port = 1337
// provide a working dir unique to each test
const main = require.main.filename
exports.pkg = path.resolve(path.dirname(main), path.basename(main, '.js'))
const mkdirp = require('mkdirp')
const rimraf = require('rimraf')
mkdirp.sync(exports.pkg)
require('tap').teardown(() => {
try {
rimraf.sync(exports.pkg)
} catch (e) {
if (process.platform !== 'win32') {
throw e
}
}
})

// space these out to help prevent collisions
const testId = 3 * (+process.env.TAP_CHILD_ID || 0)

var port = exports.port = 15443 + testId
exports.registry = 'http://localhost:' + port

exports.altPort = 7331 + testId

exports.gitPort = 4321 + testId

var fakeRegistry = require('./fake-registry.js')
exports.fakeRegistry = fakeRegistry

Expand All @@ -29,10 +53,10 @@ ourenv.npm_config_progress = 'false'
ourenv.npm_config_metrics = 'false'
ourenv.npm_config_audit = 'false'

var npm_config_cache = path.resolve(__dirname, 'npm_cache')
var npm_config_cache = path.resolve(__dirname, 'npm_cache_' + testId)
ourenv.npm_config_cache = exports.npm_config_cache = npm_config_cache
ourenv.npm_config_userconfig = exports.npm_config_userconfig = path.join(__dirname, 'fixtures', 'config', 'userconfig')
ourenv.npm_config_globalconfig = exports.npm_config_globalconfig = path.join(__dirname, 'fixtures', 'config', 'globalconfig')
ourenv.npm_config_userconfig = exports.npm_config_userconfig = configCommon.userconfig
ourenv.npm_config_globalconfig = exports.npm_config_globalconfig = configCommon.globalconfig
ourenv.npm_config_global_style = 'false'
ourenv.npm_config_legacy_bundling = 'false'
ourenv.npm_config_fetch_retries = '0'
Expand Down
9 changes: 8 additions & 1 deletion test/fake-registry.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
'use strict'
const common = require('./common-tap.js')
const Bluebird = require('bluebird')
const log = require('npmlog')
const silentLogger = {
http: () => {},
silly: () => {}
}

const log = process.env.TAP_CHILD_ID
? silentLogger
: require('npmlog')

const http = require('http')
const EventEmitter = require('events')
Expand Down
6 changes: 3 additions & 3 deletions test/need-npm5-update/ignore-shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var json = {
name: 'ignore-shrinkwrap',
version: '0.0.0',
dependencies: {
'npm-test-ignore-shrinkwrap-file': 'http://localhost:1337/package.js'
'npm-test-ignore-shrinkwrap-file': 'http://localhost:' + common.port + '/package.js'
}
}

Expand All @@ -34,8 +34,8 @@ var shrinkwrap = {
dependencies: {
'npm-test-ignore-shrinkwrap-file': {
version: '1.2.3',
from: 'http://localhost:1337/shrinkwrap.js',
resolved: 'http://localhost:1337/shrinkwrap.js',
from: 'http://localhost:' + common.port + '/shrinkwrap.js',
resolved: 'http://localhost:' + common.port + '/shrinkwrap.js',
dependencies: {
opener: {
version: '1.3.0',
Expand Down
4 changes: 2 additions & 2 deletions test/need-npm5-update/peer-deps-invalid.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ var json = {
name: 'peer-deps-invalid',
version: '0.0.0',
dependencies: {
'npm-test-peer-deps-file': 'http://localhost:1337/ok.js',
'npm-test-peer-deps-file-invalid': 'http://localhost:1337/invalid.js'
'npm-test-peer-deps-file': 'http://localhost:' + common.port + '/ok.js',
'npm-test-peer-deps-file-invalid': 'http://localhost:' + common.port + '/invalid.js'
}
}

Expand Down
5 changes: 4 additions & 1 deletion test/tap/404-parent.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var path = require('path')
var fs = require('fs')
var rimraf = require('rimraf')
var mkdirp = require('mkdirp')
var pkg = path.resolve(__dirname, '404-parent')
const pkg = common.pkg
var mr = require('npm-registry-mock')

test('404-parent: if parent exists, specify parent in error message', function (t) {
Expand Down Expand Up @@ -42,6 +42,9 @@ function setup () {

function performInstall (cb) {
mr({port: common.port}, function (er, s) { // create mock registry.
if (er) {
return cb(er)
}
s.get('/test-npm-404-parent-test')
.many().reply(404, {'error': 'version not found'})
npm.load({
Expand Down
3 changes: 1 addition & 2 deletions test/tap/404-private-registry-scoped.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
var test = require('tap').test
var path = require('path')
var mkdirp = require('mkdirp')
var rimraf = require('rimraf')
var common = require('../common-tap.js')
var mr = common.fakeRegistry.compat
var server

var testdir = path.join(__dirname, path.basename(__filename, '.js'))
var testdir = common.pkg

function setup () {
cleanup()
Expand Down
2 changes: 1 addition & 1 deletion test/tap/404-private-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var mr = common.fakeRegistry.compat
var server

var packageName = path.basename(__filename, '.js')
var testdir = path.join(__dirname, packageName)
var testdir = common.pkg

function setup () {
cleanup()
Expand Down
2 changes: 1 addition & 1 deletion test/tap/404.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const common = require('../common-tap.js')
const e404 = /test-npm-404@latest' is not in the npm registry/
const invalidPackage = /Your package name is not valid, because[\s\S]+1\. name can only contain URL-friendly characters/

const basedir = path.join(__dirname, path.basename(__filename, '.js'))
const basedir = common.pkg
const testdir = path.join(basedir, 'testdir')
const cachedir = path.join(basedir, 'cache')
const globaldir = path.join(basedir, 'global')
Expand Down
9 changes: 5 additions & 4 deletions test/tap/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const mr = require('npm-registry-mock')
const test = require('tap').test
const common = require('../common-tap.js')

const pkg = path.resolve(__dirname, 'access')
const pkg = common.pkg

let server

const scoped = {
Expand Down Expand Up @@ -63,7 +64,7 @@ test('npm access public on current package', function (t) {

test('npm access public when no package passed and no package.json', function (t) {
// need to simulate a missing package.json
var missing = path.join(__dirname, 'access-public-missing-guard')
var missing = path.join(pkg, 'access-public-missing-guard')
mkdirp.sync(path.join(missing, 'node_modules'))

common.npm([
Expand All @@ -83,7 +84,7 @@ test('npm access public when no package passed and no package.json', function (t

test('npm access public when no package passed and invalid package.json', function (t) {
// need to simulate a missing package.json
var invalid = path.join(__dirname, 'access-public-invalid-package')
var invalid = path.join(pkg, 'access-public-invalid-package')
mkdirp.sync(path.join(invalid, 'node_modules'))
// it's hard to force `read-package-json` to break w/o ENOENT, but this will do it
fs.writeFileSync(path.join(invalid, 'package.json'), '{\n')
Expand Down Expand Up @@ -380,7 +381,7 @@ test('npm access ls-packages on user', function (t) {

test('npm access ls-packages with no package specified or package.json', function (t) {
// need to simulate a missing package.json
var missing = path.join(__dirname, 'access-missing-guard')
var missing = path.join(pkg, 'access-missing-guard')
mkdirp.sync(path.join(missing, 'node_modules'))

var serverPackages = {
Expand Down
12 changes: 6 additions & 6 deletions test/tap/add-named-update-protocol-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var fooPkg = {
name: packageName,
version: '0.0.0',
dist: {
tarball: 'https://localhost:1338/registry/' + packageName + '/-/' + packageName + '-0.0.0.tgz',
tarball: 'https://localhost:' + common.altPort + '/registry/' + packageName + '/-/' + packageName + '-0.0.0.tgz',
shasum: '356a192b7913b04c54574d18c28d46e6395428ab'
}
}
Expand All @@ -30,7 +30,7 @@ var fooiPkg = {
name: iPackageName,
version: '0.0.0',
dist: {
tarball: 'http://127.0.0.1:1338/registry/' + iPackageName + '/-/' + iPackageName + '-0.0.0.tgz',
tarball: 'http://127.0.0.1:' + common.altPort + '/registry/' + iPackageName + '/-/' + iPackageName + '-0.0.0.tgz',
shasum: '356a192b7913b04c54574d18c28d46e6395428ab'
}
}
Expand All @@ -39,13 +39,13 @@ var fooiPkg = {

test('setup', function (t) {
mr({
port: 1337,
port: common.port,
throwOnUnmatched: true
}, function (err, s) {
t.ifError(err, 'registry mocked successfully')
server1 = s
mr({
port: 1338,
port: common.altPort,
throwOnUnmatched: true
}, function (err, s) {
t.ifError(err, 'registry mocked successfully')
Expand All @@ -67,7 +67,7 @@ test('tarball paths should update port if updating protocol', function (t) {
'add',
packageName + '@0.0.0',
'--registry',
'http://localhost:1337/registry'
'http://localhost:' + common.port + '/registry'
],
{},
function (er, code, stdout, stderr) {
Expand All @@ -92,7 +92,7 @@ test('tarball paths should NOT update if different hostname', function (t) {
'add',
iPackageName + '@0.0.0',
'--registry',
'http://localhost:1337/registry'
'http://localhost:' + common.port + '/registry'
],
{},
function (er, code, stdout, stderr) {
Expand Down
4 changes: 2 additions & 2 deletions test/tap/add-remote-git-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ var npm = require('../../lib/npm.js')
var fetchPackageMetadata = require('../../lib/fetch-package-metadata.js')
var common = require('../common-tap.js')

var pkg = resolve(__dirname, 'add-remote-git-file')
var repo = resolve(__dirname, 'add-remote-git-file-repo')
var pkg = common.pkg
var repo = common.pkg + '-repo'

var git
var cloneURL = 'git+file://' + resolve(pkg, 'child.git')
Expand Down
Loading

0 comments on commit bec8272

Please sign in to comment.