test: npm ls errors

Author: dominykas

README.md

@@ -17,7 +17,7 @@ Only the explicitly allowed `[pre|post]install` scripts will be executed.
 $ npx allow-scripts [--dry-run]
 ```
 
-Running the command will scan the list of installed dependencies (from the first source available: `npm-shrinkwrap.json`, `package-lock.json`, `npm ls --json`). It will then execute the scripts for allowed dependencies that have them in the following order:
+Running the command will scan the list of installed dependencies using `npm ls --json`. It will then execute the scripts for allowed dependencies that have them in the following order:
 
 - `preinstall` in the main package
 - `preinstall` in dependencies

lib/index.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const Cp = require('child_process');
-const Fs = require('fs');
 const Npm = require('libnpm');
 const Path = require('path');
 const Semver = require('semver');
@@ -74,30 +73,21 @@ internals.runScript = (stage, { pkg, path, cwd, unsafePerm }, options) => {
     });
 };
 
-internals.getLockFile = (cwd) => {
-
-    if (Fs.existsSync(Path.join(cwd, 'npm-shrinkwrap.json'))) {
-        return require(Path.join(cwd, 'npm-shrinkwrap.json'));
-    }
-
-    if (Fs.existsSync(Path.join(cwd, 'package-lock.json'))) {
-        return require(Path.join(cwd, 'package-lock.json'));
-    }
+internals.getInstalledContents = (cwd) => {
 
     let output;
     try {
         output = Cp.execSync('npm ls --json', { cwd });
     }
     catch (err) {
-        output = err.output[1]; // npm will exist with an error when e.g. there's peer deps missing - attempt to ignore that
+        output = err.output[1]; // npm will exit with an error when e.g. there's peer deps missing - attempt to ignore that
     }
 
     try {
         return JSON.parse(output.toString());
     }
     catch (err) {
-        console.error(err);
-        throw new Error('Failed to read the contents of node_modules');
+        throw new Error(`Failed to read the contents of node_modules. \`npm ls --json\` returned: ${output}`);
     }
 };
 
@@ -108,7 +98,7 @@ exports.run = async (options) => {
 
     pkg._id = `${pkg.name}@${pkg.version}`; // @todo: find an official way to do this for top level package
 
-    const tree = Npm.logicalTree(pkg, internals.getLockFile(cwd));
+    const tree = Npm.logicalTree(pkg, internals.getInstalledContents(cwd));
     const queue = internals.queue(tree);
 
     const allowScripts = pkg.allowScripts || {};

package.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/dominykas/allow-scripts.git"
   },
   "scripts": {
-    "test": "lab -L -t 86 -m 5000"
+    "test": "lab -L -t 93 -m 5000"
   },
   "bin": {
     "allow-scripts": "./bin/allow-scripts.js"

test/fixtures/basic.txt renamed to test/fixtures/basic.full.txt

@@ -0,0 +1,6 @@
+preinstall from basic
+install from with-install-script
+install from basic
+postinstall from basic
+prepublish from basic
+prepare from basic

test/fixtures/basic.incomplete.txt

@@ -53,12 +53,9 @@ exports.setup = (main, deps) => {
     Sinon.stub(console, 'info').callsFake(appendLog);
     Sinon.stub(console, 'log').callsFake(appendLog);
     Sinon.stub(console, 'warn').callsFake(appendLog);
+    Sinon.stub(process.stderr, 'write').callsFake((data) => appendLog(data.toString()));
 
     return {
-        getExpectedResult: () => {
-
-            return Fs.readFileSync(Path.join(__dirname, `${main}.txt`)).toString().trim();
-        },
         getActualResult: () => {
 
             return Fs.readFileSync(output).toString().trim();

test/fixtures/index.js

@@ -1,7 +1,10 @@
 'use strict';
 
+const Cp = require('child_process');
+const Fs = require('fs');
 const Fixtures = require('./fixtures');
-
+const Path = require('path');
+const Sinon = require('sinon');
 
 const Allow = require('..');
 
@@ -37,7 +40,7 @@ describe('allow-scripts', () => {
 
             await Allow.run({});
 
-            expect(fixture.getActualResult()).to.equal(fixture.getExpectedResult());
+            expect(fixture.getActualResult()).to.equal(Fs.readFileSync(Path.join(__dirname, 'fixtures', 'basic.full.txt')).toString().trim());
             expect(fixture.getLog()).not.to.contain('without-scripts');
             expect(fixture.getLog()).not.to.contain('without-install-scripts');
         });
@@ -113,5 +116,29 @@ describe('allow-scripts', () => {
             expect(fixture.getActualResult()).to.equal('');
             expect(fixture.getLog()).to.equal('');
         });
+
+        it('deals with incomplete installed tree', async () => {
+
+            const fixture = Fixtures.setup('basic', [
+                // 'with-preinstall-script',
+                'with-install-script',
+                // 'with-postinstall-script',
+                'without-scripts',
+                'without-install-scripts'
+            ]);
+
+            await Allow.run({});
+
+            expect(fixture.getActualResult()).to.equal(Fs.readFileSync(Path.join(__dirname, 'fixtures', 'basic.incomplete.txt')).toString().trim());
+        });
+
+        it('deals with unparseable tree', async () => {
+
+            Sinon.stub(Cp, 'execSync').returns('not-json');
+
+            Fixtures.setup('basic', []);
+
+            await expect(Allow.run({})).to.reject('Failed to read the contents of node_modules. `npm ls --json` returned: not-json');
+        });
     });
 });