fix(doctor): allow for missing local bin and node_modules

npm · Mar 3, 2022 · da39e80 · da39e80
1 parent 506be10
commit da39e80
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 17 deletions.
diff --git a/lib/commands/doctor.js b/lib/commands/doctor.js
@@ -58,29 +58,33 @@ class Doctor extends BaseCommand {
       ...(isWindows
         ? []
         : [
-          ['Perms check on cached files', 'checkFilesPermission', [this.npm.cache, true, R_OK]],
           [
+            'Perms check on cached files',
+            'checkFilesPermission',
+            [this.npm.cache, true, R_OK]
+          ], [
             'Perms check on local node_modules',
             'checkFilesPermission',
-            [this.npm.localDir, true],
-          ],
-          [
+            [this.npm.localDir, true, R_OK | W_OK, true],
+          ], [
             'Perms check on global node_modules',
             'checkFilesPermission',
-            [this.npm.globalDir, false],
-          ],
-          [
+            [this.npm.globalDir, false, R_OK],
+          ], [
             'Perms check on local bin folder',
             'checkFilesPermission',
-            [this.npm.localBin, false, R_OK | W_OK | X_OK],
-          ],
-          [
+            [this.npm.localBin, false, R_OK | W_OK | X_OK, true],
+          ], [
             'Perms check on global bin folder',
             'checkFilesPermission',
             [this.npm.globalBin, false, X_OK],
           ],
         ]),
-      ['Verify cache contents', 'verifyCachedFiles', [this.npm.flatOptions.cache]],
+      [
+        'Verify cache contents',
+        'verifyCachedFiles',
+        [this.npm.flatOptions.cache]
+      ],
       // TODO:
       // - ensure arborist.loadActual() runs without errors and no invalid edges
       // - ensure package-lock.json matches loadActual()
@@ -202,10 +206,7 @@ class Doctor extends BaseCommand {
     }
   }
 
-  async checkFilesPermission (root, shouldOwn, mask = null) {
-    if (mask === null) {
-      mask = shouldOwn ? R_OK | W_OK : R_OK
-    }
+  async checkFilesPermission (root, shouldOwn, mask, missingOk) {
 
     let ok = true
 
@@ -218,8 +219,10 @@ class Doctor extends BaseCommand {
       for (const f of files) {
         tracker.silly('checkFilesPermission', f.substr(root.length + 1))
         const st = await lstat(f).catch(er => {
-          ok = false
-          tracker.warn('checkFilesPermission', 'error getting info for ' + f)
+          if (!missingOk || err.code === 'ENOENT') {
+            ok = false
+            tracker.warn('checkFilesPermission', 'error getting info for ' + f)
+          }
         })
 
         tracker.completeWork(1)

diff --git a/tap-snapshots/test/lib/commands/doctor.js.test.cjs b/tap-snapshots/test/lib/commands/doctor.js.test.cjs
@@ -826,6 +826,64 @@ Perms check on global bin folder    not ok  Check the permissions of files in {C
 Verify cache contents               ok      verified 0 tarballs
 `
 
+exports[`test/lib/commands/doctor.js TAP missing local node_modules > logs 1`] = `
+Object {
+  "error": Array [],
+  "info": Array [
+    Array [
+      "Running checkup",
+    ],
+    Array [
+      "checkPing",
+      "Pinging registry",
+    ],
+    Array [
+      "getLatestNpmVersion",
+      "Getting npm package information",
+    ],
+    Array [
+      "getLatestNodejsVersion",
+      "Getting Node.js release information",
+    ],
+    Array [
+      "getGitPath",
+      "Finding git in your PATH",
+    ],
+    Array [
+      "verifyCachedFiles",
+      "Verifying the npm cache",
+    ],
+    Array [
+      "verifyCachedFiles",
+      String(
+        Verification complete. Stats: {
+          "badContentCount": 0,
+          "reclaimedCount": 0,
+          "missingContent": 0,
+          "verifiedContent": 0
+        }
+      ),
+    ],
+  ],
+  "warn": Array [],
+}
+`
+
+exports[`test/lib/commands/doctor.js TAP missing local node_modules > missing local node_modules 1`] = `
+Check                               Value  Recommendation/Notes
+npm ping                            ok
+npm -v                              ok     current: v1.0.0, latest: v1.0.0
+node -v                             ok     current: v1.0.0, recommended: v1.0.0
+npm config get registry             ok     using default registry (https://registry.npmjs.org/)
+which git                           ok     /path/to/git
+Perms check on cached files         ok
+Perms check on local node_modules   ok
+Perms check on global node_modules  ok
+Perms check on local bin folder     ok
+Perms check on global bin folder    ok
+Verify cache contents               ok     verified 0 tarballs
+`
+
 exports[`test/lib/commands/doctor.js TAP node out of date - current > logs 1`] = `
 Object {
   "error": Array [],

diff --git a/test/lib/commands/doctor.js b/test/lib/commands/doctor.js
@@ -324,6 +324,21 @@ t.test('missing global directories', async t => {
   t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
 })
 
+t.test('missing local node_modules', async t => {
+  const { joinedOutput, logs, npm } = await loadMockNpm(t, {
+    mocks,
+    globalPrefixDir: dirs.globalPrefixDir,
+  })
+  tnock(t, npm.config.get('registry'))
+    .get('/-/ping?write=true').reply(200, '{}')
+    .get('/npm').reply(200, npmManifest(npm.version))
+  tnock(t, 'https://nodejs.org')
+    .get('/dist/index.json').reply(200, nodeVersions)
+  await npm.exec('doctor', [])
+  t.matchSnapshot(joinedOutput(), 'missing local node_modules')
+  t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
+})
+
 t.test('incorrect owner', async t => {
   const { joinedOutput, logs, npm } = await loadMockNpm(t, {
     mocks: {