Skip to content

Commit 4561bd1

Browse files
committed
test: test cases for circular symlinked deps and peer deps
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.
1 parent 8b634e8 commit 4561bd1

2 files changed

Lines changed: 146 additions & 0 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/* eslint no-irregular-whitespace: 0 */
2+
'use strict';
3+
// Refs: https://github.com/nodejs/node/pull/5950
4+
5+
// This test illustrates the problem that symlinked modules are unable
6+
// to find their peer dependencies. This was fixed in #5950 but that is
7+
// reverted because that particular way of fixing it causes too much
8+
// breakage (breakage that was not caught by either CI or CITGM on multiple
9+
// runs.
10+
11+
// This test passes in v6 with https://github.com/nodejs/node/pull/5950 but
12+
// fails with https://github.com/nodejs/node/pull/5950 reverted. This test
13+
// will fail in Node.js v4 and v5.
14+
15+
const common = require('../common');
16+
const fs = require('fs');
17+
const path = require('path');
18+
const assert = require('assert');
19+
20+
common.refreshTmpDir();
21+
22+
const tmpDir = common.tmpDir;
23+
24+
// Creates the following structure
25+
// {tmpDir}
26+
// ├── app
27+
// │   ├── index.js
28+
// │   └── node_modules
29+
// │   ├── moduleA -> {tmpDir}/moduleA
30+
// │   └── moduleB
31+
// │   ├── index.js
32+
// │   └── package.json
33+
// └── moduleA
34+
// ├── index.js
35+
// └── package.json
36+
37+
const moduleA = path.join(tmpDir, 'moduleA');
38+
const app = path.join(tmpDir, 'app');
39+
const moduleB = path.join(app, 'node_modules', 'moduleB');
40+
const moduleA_link = path.join(app, 'node_modules', 'moduleA');
41+
fs.mkdirSync(moduleA);
42+
fs.mkdirSync(app);
43+
fs.mkdirSync(path.join(app, 'node_modules'));
44+
fs.mkdirSync(moduleB);
45+
46+
// Attempt to make the symlink. If this fails due to lack of sufficient
47+
// permissions, the test will bail out and be skipped.
48+
try {
49+
fs.symlinkSync(moduleA, moduleA_link);
50+
} catch (err) {
51+
if (err.code !== 'EPERM') throw err;
52+
console.log('1..0 # Skipped: insufficient privileges for symlinks');
53+
return;
54+
}
55+
56+
fs.writeFileSync(path.join(moduleA, 'package.json'),
57+
JSON.stringify({name: 'moduleA', main: 'index.js'}), 'utf8');
58+
fs.writeFileSync(path.join(moduleA, 'index.js'),
59+
'module.exports = require(\'moduleB\');', 'utf8');
60+
fs.writeFileSync(path.join(app, 'index.js'),
61+
'\'use strict\'; require(\'moduleA\');', 'utf8');
62+
fs.writeFileSync(path.join(moduleB, 'package.json'),
63+
JSON.stringify({name: 'moduleB', main: 'index.js'}), 'utf8');
64+
fs.writeFileSync(path.join(moduleB, 'index.js'),
65+
'module.exports = 1;', 'utf8');
66+
67+
// TODO(@jasnell): Update this comment with this issue is fixed and
68+
// this test is moved out of the known_issues directory.
69+
//
70+
// Ideally, this should not throw but it does because moduleA is not
71+
// able to find it's peer dependency moduleB. The reason it cannot find
72+
// it's peer is because moduleA is cached at it's realpath and when it's
73+
// require goes to find moduleA, it can't (because moduleB does not exist
74+
// within it's lookup path).
75+
assert.doesNotThrow(() => {
76+
require(path.join(app, 'index'));
77+
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/* eslint no-irregular-whitespace: 0 */
2+
'use strict';
3+
4+
// This tests to make sure that modules with symlinked circular dependencies
5+
// do not blow out the module cache and recurse forever. See issue
6+
// https://github.com/nodejs/node/pull/5950 for context. PR #5950 attempted
7+
// to solve a problem with symlinked peer dependencies by caching using the
8+
// symlink path. Unfortunately, that breaks the case tested in this module
9+
// because each symlinked module, despite pointing to the same code on disk,
10+
// is loaded and cached as a separate module instance, which blows up the
11+
// cache and leads to a recursion bug.
12+
13+
// This test should pass in Node.js v4 and v5. It should pass in Node.js v6
14+
// after https://github.com/nodejs/node/pull/5950 has been reverted.
15+
16+
const common = require('../common');
17+
const assert = require('assert');
18+
const path = require('path');
19+
const fs = require('fs');
20+
21+
// {tmpDir}
22+
// ├── index.js
23+
// └── node_modules
24+
// ├── moduleA
25+
// │   ├── index.js
26+
// │   └── node_modules
27+
// │   └── moduleB -> {tmpDir}/node_modules/moduleB
28+
// └── moduleB
29+
// ├── index.js
30+
// └── node_modules
31+
// └── moduleA -> {tmpDir}/node_modules/moduleA
32+
33+
common.refreshTmpDir();
34+
const tmpDir = common.tmpDir;
35+
36+
const node_modules = path.join(tmpDir, 'node_modules');
37+
const moduleA = path.join(node_modules, 'moduleA');
38+
const moduleB = path.join(node_modules, 'moduleB');
39+
const moduleA_link = path.join(moduleB, 'node_modules', 'moduleA');
40+
const moduleB_link = path.join(moduleA, 'node_modules', 'moduleB');
41+
42+
fs.mkdirSync(node_modules);
43+
fs.mkdirSync(moduleA);
44+
fs.mkdirSync(moduleB);
45+
fs.mkdirSync(path.join(moduleA, 'node_modules'));
46+
fs.mkdirSync(path.join(moduleB, 'node_modules'));
47+
48+
try {
49+
fs.symlinkSync(moduleA, moduleA_link);
50+
fs.symlinkSync(moduleB, moduleB_link);
51+
} catch (err) {
52+
if (err.code !== 'EPERM') throw err;
53+
console.log('1..0 # Skipped: insufficient privileges for symlinks');
54+
return;
55+
}
56+
57+
fs.writeFileSync(path.join(tmpDir, 'index.js'),
58+
'module.exports = require(\'moduleA\');', 'utf8');
59+
fs.writeFileSync(path.join(moduleA, 'index.js'),
60+
'module.exports = {b: require(\'moduleB\')};', 'utf8');
61+
fs.writeFileSync(path.join(moduleB, 'index.js'),
62+
'module.exports = {a: require(\'moduleA\')};', 'utf8');
63+
64+
// Ensure that the symlinks are not followed forever...
65+
const obj = require(path.join(tmpDir, 'index'));
66+
assert.ok(obj);
67+
assert.ok(obj.b);
68+
assert.ok(obj.b.a);
69+
assert.ok(!obj.b.a.b);

0 commit comments

Comments
 (0)