Skip to content

Commit

Permalink
test: test cases for circular symlinked deps and peer deps
Browse files Browse the repository at this point in the history
Adds a test case in the known-test cases to catch nodejs#5950
Adds a test to verify that circular symlinked deps do not recurse
forever.
  • Loading branch information
jasnell committed May 3, 2016
1 parent 8b634e8 commit 4561bd1
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 0 deletions.
77 changes: 77 additions & 0 deletions test/known_issues/test-symlinked-peer-modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/* eslint no-irregular-whitespace: 0 */
'use strict';
// Refs: https://github.com/nodejs/node/pull/5950

// This test illustrates the problem that symlinked modules are unable
// to find their peer dependencies. This was fixed in #5950 but that is
// reverted because that particular way of fixing it causes too much
// breakage (breakage that was not caught by either CI or CITGM on multiple
// runs.

// This test passes in v6 with https://github.com/nodejs/node/pull/5950 but
// fails with https://github.com/nodejs/node/pull/5950 reverted. This test
// will fail in Node.js v4 and v5.

const common = require('../common');
const fs = require('fs');
const path = require('path');
const assert = require('assert');

common.refreshTmpDir();

const tmpDir = common.tmpDir;

// Creates the following structure
// {tmpDir}
// ├── app
// │   ├── index.js
// │   └── node_modules
// │   ├── moduleA -> {tmpDir}/moduleA
// │   └── moduleB
// │   ├── index.js
// │   └── package.json
// └── moduleA
// ├── index.js
// └── package.json

const moduleA = path.join(tmpDir, 'moduleA');
const app = path.join(tmpDir, 'app');
const moduleB = path.join(app, 'node_modules', 'moduleB');
const moduleA_link = path.join(app, 'node_modules', 'moduleA');
fs.mkdirSync(moduleA);
fs.mkdirSync(app);
fs.mkdirSync(path.join(app, 'node_modules'));
fs.mkdirSync(moduleB);

// Attempt to make the symlink. If this fails due to lack of sufficient
// permissions, the test will bail out and be skipped.
try {
fs.symlinkSync(moduleA, moduleA_link);
} catch (err) {
if (err.code !== 'EPERM') throw err;
console.log('1..0 # Skipped: insufficient privileges for symlinks');
return;
}

fs.writeFileSync(path.join(moduleA, 'package.json'),
JSON.stringify({name: 'moduleA', main: 'index.js'}), 'utf8');
fs.writeFileSync(path.join(moduleA, 'index.js'),
'module.exports = require(\'moduleB\');', 'utf8');
fs.writeFileSync(path.join(app, 'index.js'),
'\'use strict\'; require(\'moduleA\');', 'utf8');
fs.writeFileSync(path.join(moduleB, 'package.json'),
JSON.stringify({name: 'moduleB', main: 'index.js'}), 'utf8');
fs.writeFileSync(path.join(moduleB, 'index.js'),
'module.exports = 1;', 'utf8');

// TODO(@jasnell): Update this comment with this issue is fixed and
// this test is moved out of the known_issues directory.
//
// Ideally, this should not throw but it does because moduleA is not
// able to find it's peer dependency moduleB. The reason it cannot find
// it's peer is because moduleA is cached at it's realpath and when it's
// require goes to find moduleA, it can't (because moduleB does not exist
// within it's lookup path).
assert.doesNotThrow(() => {
require(path.join(app, 'index'));
});
69 changes: 69 additions & 0 deletions test/sequential/test-module-circular-symlinks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/* eslint no-irregular-whitespace: 0 */
'use strict';

// This tests to make sure that modules with symlinked circular dependencies
// do not blow out the module cache and recurse forever. See issue
// https://github.com/nodejs/node/pull/5950 for context. PR #5950 attempted
// to solve a problem with symlinked peer dependencies by caching using the
// symlink path. Unfortunately, that breaks the case tested in this module
// because each symlinked module, despite pointing to the same code on disk,
// is loaded and cached as a separate module instance, which blows up the
// cache and leads to a recursion bug.

// This test should pass in Node.js v4 and v5. It should pass in Node.js v6
// after https://github.com/nodejs/node/pull/5950 has been reverted.

const common = require('../common');
const assert = require('assert');
const path = require('path');
const fs = require('fs');

// {tmpDir}
// ├── index.js
// └── node_modules
// ├── moduleA
// │   ├── index.js
// │   └── node_modules
// │   └── moduleB -> {tmpDir}/node_modules/moduleB
// └── moduleB
// ├── index.js
// └── node_modules
// └── moduleA -> {tmpDir}/node_modules/moduleA

common.refreshTmpDir();
const tmpDir = common.tmpDir;

const node_modules = path.join(tmpDir, 'node_modules');
const moduleA = path.join(node_modules, 'moduleA');
const moduleB = path.join(node_modules, 'moduleB');
const moduleA_link = path.join(moduleB, 'node_modules', 'moduleA');
const moduleB_link = path.join(moduleA, 'node_modules', 'moduleB');

fs.mkdirSync(node_modules);
fs.mkdirSync(moduleA);
fs.mkdirSync(moduleB);
fs.mkdirSync(path.join(moduleA, 'node_modules'));
fs.mkdirSync(path.join(moduleB, 'node_modules'));

try {
fs.symlinkSync(moduleA, moduleA_link);
fs.symlinkSync(moduleB, moduleB_link);
} catch (err) {
if (err.code !== 'EPERM') throw err;
console.log('1..0 # Skipped: insufficient privileges for symlinks');
return;
}

fs.writeFileSync(path.join(tmpDir, 'index.js'),
'module.exports = require(\'moduleA\');', 'utf8');
fs.writeFileSync(path.join(moduleA, 'index.js'),
'module.exports = {b: require(\'moduleB\')};', 'utf8');
fs.writeFileSync(path.join(moduleB, 'index.js'),
'module.exports = {a: require(\'moduleA\')};', 'utf8');

// Ensure that the symlinks are not followed forever...
const obj = require(path.join(tmpDir, 'index'));
assert.ok(obj);
assert.ok(obj.b);
assert.ok(obj.b.a);
assert.ok(!obj.b.a.b);

0 comments on commit 4561bd1

Please sign in to comment.