fix: updated ebadplatform messaging to be generated based on the error (

npm#6277)
ertw · Mar 21, 2023 · 2def359 · 2def359
1 parent 2e3b11a
commit 2def359
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 26 deletions.
diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js
@@ -247,16 +247,34 @@ const errorMessage = (er, npm) => {
       break
 
     case 'EBADPLATFORM': {
-      const validOs =
-        er.required && er.required.os && er.required.os.join
-          ? er.required.os.join(',')
-          : er.required.os
-      const validArch =
-        er.required && er.required.cpu && er.required.cpu.join
-          ? er.required.cpu.join(',')
-          : er.required.cpu
-      const expected = { os: validOs, arch: validArch }
-      const actual = { os: process.platform, arch: process.arch }
+      const actual = er.current
+      const expected = { ...er.required }
+      const checkedKeys = []
+      for (const key in expected) {
+        if (Array.isArray(expected[key]) && expected[key].length > 0) {
+          expected[key] = expected[key].join(',')
+          checkedKeys.push(key)
+        } else if (expected[key] === undefined ||
+            Array.isArray(expected[key]) && expected[key].length === 0) {
+          delete expected[key]
+          delete actual[key]
+        } else {
+          checkedKeys.push(key)
+        }
+      }
+
+      const longestKey = Math.max(...checkedKeys.map((key) => key.length))
+      const detailEntry = []
+      for (const key of checkedKeys) {
+        const padding = key.length === longestKey
+          ? 1
+          : 1 + (longestKey - key.length)
+
+        // padding + 1 because 'actual' is longer than 'valid'
+        detailEntry.push(`Valid ${key}:${' '.repeat(padding + 1)}${expected[key]}`)
+        detailEntry.push(`Actual ${key}:${' '.repeat(padding)}${actual[key]}`)
+      }
+
       short.push([
         'notsup',
         [
@@ -270,12 +288,7 @@ const errorMessage = (er, npm) => {
       ])
       detail.push([
         'notsup',
-        [
-          'Valid OS:    ' + validOs,
-          'Valid Arch:  ' + validArch,
-          'Actual OS:   ' + process.platform,
-          'Actual Arch: ' + process.arch,
-        ].join('\n'),
+        detailEntry.join('\n'),
       ])
       break
     }

diff --git a/tap-snapshots/test/lib/utils/error-message.js.test.cjs b/tap-snapshots/test/lib/utils/error-message.js.test.cjs
@@ -241,17 +241,37 @@ Object {
     Array [
       "notsup",
       String(
-        Valid OS:    !yours,mine
-        Valid Arch:  x867,x5309
-        Actual OS:   posix
-        Actual Arch: x64
+        Valid os:   !yours,mine
+        Actual os:  posix
+        Valid cpu:  x867,x5309
+        Actual cpu: x64
       ),
     ],
   ],
   "summary": Array [
     Array [
       "notsup",
-      "Unsupported platform for [email protected]: wanted {/"os/":/"!yours,mine/",/"arch/":/"x867,x5309/"} (current: {/"os/":/"posix/",/"arch/":/"x64/"})",
+      "Unsupported platform for [email protected]: wanted {/"os/":/"!yours,mine/",/"cpu/":/"x867,x5309/"} (current: {/"os/":/"posix/",/"cpu/":/"x64/"})",
+    ],
+  ],
+}
+`
+
+exports[`test/lib/utils/error-message.js TAP bad platform omits keys with no required value > must match snapshot 1`] = `
+Object {
+  "detail": Array [
+    Array [
+      "notsup",
+      String(
+        Valid os:  !yours,mine
+        Actual os: posix
+      ),
+    ],
+  ],
+  "summary": Array [
+    Array [
+      "notsup",
+      "Unsupported platform for [email protected]: wanted {/"os/":/"!yours,mine/"} (current: {/"os/":/"posix/"})",
     ],
   ],
 }
@@ -263,17 +283,17 @@ Object {
     Array [
       "notsup",
       String(
-        Valid OS:    !yours
-        Valid Arch:  x420
-        Actual OS:   posix
-        Actual Arch: x64
+        Valid os:   !yours
+        Actual os:  posix
+        Valid cpu:  x420
+        Actual cpu: x64
       ),
     ],
   ],
   "summary": Array [
     Array [
       "notsup",
-      "Unsupported platform for [email protected]: wanted {/"os/":/"!yours/",/"arch/":/"x420/"} (current: {/"os/":/"posix/",/"arch/":/"x64/"})",
+      "Unsupported platform for [email protected]: wanted {/"os/":/"!yours/",/"cpu/":/"x420/"} (current: {/"os/":/"posix/",/"cpu/":/"x64/"})",
     ],
   ],
 }

diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js
@@ -384,6 +384,27 @@ t.test('bad platform', async t => {
     t.matchSnapshot(errorMessage(er))
     t.end()
   })
+  t.test('omits keys with no required value', t => {
+    const er = Object.assign(new Error('a bad plat'), {
+      pkgid: '[email protected]',
+      current: {
+        os: 'posix',
+        cpu: 'x64',
+        libc: 'musl',
+      },
+      required: {
+        os: ['!yours', 'mine'],
+        libc: [], // empty arrays should also lead to a key being removed
+        cpu: undefined, // XXX npm-install-checks sets unused keys to undefined
+      },
+      code: 'EBADPLATFORM',
+    })
+    const msg = errorMessage(er)
+    t.matchSnapshot(msg)
+    t.notMatch(msg, /Valid cpu/, 'omits cpu from message')
+    t.notMatch(msg, /Valid libc/, 'omits libc from message')
+    t.end()
+  })
 })
 
 t.test('explain ERESOLVE errors', async t => {