Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix npm ci with file: dependencies #216

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/shrinkwrap.js
Expand Up @@ -111,7 +111,6 @@ function shrinkwrapDeps (deps, top, tree, seen) {
var pkginfo = deps[moduleName(child)] = {}
var requested = getRequested(child) || child.package._requested || {}
var linked = child.isLink || child.isInLink
var linkedFromHere = linked && path.relative(top.realpath, child.realpath)[0] !== '.'
pkginfo.version = childVersion(top, child, requested)
if (requested.type === 'git' && child.package._from) {
pkginfo.from = child.package._from
Expand Down Expand Up @@ -142,7 +141,7 @@ function shrinkwrapDeps (deps, top, tree, seen) {
})
}
// iterate into children on non-links and links contained within the top level package
if (child.children.length && (!child.isLink || linkedFromHere)) {
if (child.children.length) {
pkginfo.dependencies = {}
shrinkwrapDeps(pkginfo.dependencies, top, child, seen)
}
Expand Down
83 changes: 83 additions & 0 deletions test/tap/ci-with-local-dependency.js
@@ -0,0 +1,83 @@
var fs = require('graceful-fs')
var path = require('path')

var mkdirp = require('mkdirp')
var osenv = require('osenv')
var rimraf = require('rimraf')
var test = require('tap').test

var common = require('../common-tap.js')

var pkg = common.pkg

var EXEC_OPTS = { cwd: pkg, stdio: [0, 1, 2] }

var localDependencyJson = {
name: 'local-dependency',
version: '0.0.0',
dependencies: {
underscore: '1.5.1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I just realized, this is going to hit the public registry during a test. I'll switch it to use the mock registry, but for future reference, that's usually not a great idea. With as many tests as npm has, if it had to look further than localhost, it's kind of a nightmare.

}
}

var dependentJson = {
name: 'dependent',
version: '0.0.0',
dependencies: {
'local-dependency': '../local-dependency'
}
}

var target = path.resolve(pkg, '../local-dependency')

test('setup', function (t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to make or remove the common.pkg folder any more. (In fact, doing so can throw off the root ownership testing.) Both the common.cache and common.pkg are managed by the common-tap.js module.

Just write the package.json file. The rest is taken care of :)

cleanup()
t.end()
})

test('\'npm install\' should install local pkg from sub path', function (t) {
setup()
common.npm(['install', '--loglevel=silent'], EXEC_OPTS, function (err, code) {
if (err) throw err
t.equal(code, 0, 'npm install exited with code')
t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/package.json')).isFile(), 'local dependency package.json exists')
t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/node_modules/underscore')).isDirectory(), 'transitive dependency installed')
t.end()
})
})

test('\'npm ci\' should work', function (t) {
common.npm(['ci', '--loglevel=silent'], EXEC_OPTS, function (err, code) {
if (err) throw err
t.equal(code, 0, 'npm install exited with code')
t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/package.json')).isFile(), 'local dependency package.json exists')
t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/node_modules/underscore')).isDirectory(), 'transitive dependency installed')
t.end()
})
})

test('cleanup', function (t) {
cleanup()
t.end()
})

function cleanup () {
process.chdir(osenv.tmpdir())
rimraf.sync(pkg)
rimraf.sync(target)
}

function setup () {
cleanup()
mkdirp.sync(target)
fs.writeFileSync(
path.join(target, 'package.json'),
JSON.stringify(localDependencyJson, null, 2)
)
mkdirp.sync(pkg)
fs.writeFileSync(
path.join(pkg, 'package.json'),
JSON.stringify(dependentJson, null, 2)
)
process.chdir(pkg)
}
4 changes: 3 additions & 1 deletion test/tap/spec-local-specifiers.js
Expand Up @@ -585,7 +585,9 @@ test('save behavior', function (t) {
var deps = pjson.dependencies || {}
t.is(deps['sb-transitive'], 'file:../sb-transitive', 'package.json')
var sdep = shrinkwrap.dependencies['sb-transitive'] || {}
t.like(sdep, {bundled: null, dependencies: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep')
var tdep = sdep.dependencies.sbta
t.like(tdep, {bundled: null, version: 'file:../sb-transitive/sbta'}, 'npm-shrinkwrap.json transitive dep')
t.like(sdep, {bundled: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep')
t.done()
})
})
Expand Down