From 4561bd1d3eaa160a9fff94b0afd5f104b8e7cd24 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 2 May 2016 16:31:20 -0700 Subject: [PATCH] test: test cases for circular symlinked deps and peer deps Adds a test case in the known-test cases to catch #5950 Adds a test to verify that circular symlinked deps do not recurse forever. --- .../test-symlinked-peer-modules.js | 77 +++++++++++++++++++ .../test-module-circular-symlinks.js | 69 +++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 test/known_issues/test-symlinked-peer-modules.js create mode 100644 test/sequential/test-module-circular-symlinks.js diff --git a/test/known_issues/test-symlinked-peer-modules.js b/test/known_issues/test-symlinked-peer-modules.js new file mode 100644 index 00000000000000..45b1c27c257192 --- /dev/null +++ b/test/known_issues/test-symlinked-peer-modules.js @@ -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')); +}); diff --git a/test/sequential/test-module-circular-symlinks.js b/test/sequential/test-module-circular-symlinks.js new file mode 100644 index 00000000000000..793602bb7fa5fa --- /dev/null +++ b/test/sequential/test-module-circular-symlinks.js @@ -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);