chore: add coverage tests and istanbul ignores for defensive branches · npm/cli@3bb254c · GitHub
Skip to content

Commit 3bb254c

Browse files
JamieMageeJamieMagee
committed
chore: add coverage tests and istanbul ignores for defensive branches
More tests for the Phase 1 install-script work, plus a handful of /* istanbul ignore next */ markers for defensive fallbacks that can't be hit from realistic inputs (e.g. nodes with neither packageName nor name set, npa() failing on an already-validated string). - reify-output: JSON path with unreviewedScripts and the node.name fallback when packageName is absent - approve-scripts: single-package wording, no-op when already at the right pin, --all with no unreviewed packages - deny-scripts: --all empty, global rejection, --json output, no-op on already-denied - allow-scripts-writer: null node guards, non-hosted git URL, absolute path resolved field, deny entries on file/remote deps, alias-typed policy keys, unparseable existing entries
1 parent 26250bc commit 3bb254c

9 files changed

Lines changed: 358 additions & 100 deletions

File tree

‎docs/lib/content/nav.yml‎

Lines changed: 6 additions & 0 deletions

‎lib/utils/allow-scripts-cmd.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class AllowScriptsCmd extends BaseCommand {
3030
static ignoreImplicitWorkspace = false
3131

3232
// Subclasses set this.
33+
/* istanbul ignore next */
3334
get verb () {
3435
throw new Error('verb must be implemented by subclass')
3536
}
@@ -214,6 +215,7 @@ class AllowScriptsCmd extends BaseCommand {
214215
summary.push({ name, changes: result.changes })
215216
}
216217

218+
/* istanbul ignore else: writePolicyChanges only called when changes are expected */
217219
if (updated !== existing) {
218220
pkg.update({ allowScripts: updated })
219221
await pkg.save()

‎lib/utils/allow-scripts-writer.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ const keyTargetsNode = (key, node) => {
148148
try {
149149
resolvedParsed = node.resolved ? npa(node.resolved) : null
150150
} catch {
151+
/* istanbul ignore next */
151152
return false
152153
}
153154
const keyHost = parsed.hosted?.ssh({ noCommittish: true })
@@ -174,7 +175,7 @@ const keyTargetsNode = (key, node) => {
174175
// - `changes` is a list of `{ key, change }` entries describing edits
175176
// - `warning` is an optional message to surface to the user
176177
const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => {
177-
const allowScripts = { ...(existing || {}) }
178+
const allowScripts = { ...existing }
178179
const changes = []
179180

180181
if (!Array.isArray(nodes) || nodes.length === 0) {
@@ -188,6 +189,8 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => {
188189
for (const node of nodes) {
189190
for (const [key, value] of Object.entries(allowScripts)) {
190191
if (value === false && keyTargetsNode(key, node)) {
192+
/* istanbul ignore next: name fallback covers the empty-name edge case */
193+
const subject = name || 'this package'
191194
return {
192195
allowScripts,
193196
changes,
@@ -211,6 +214,7 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => {
211214
}
212215
}
213216

217+
/* istanbul ignore else: name === null is the no-identity path tested separately */
214218
if (name && allowScripts[name] !== true) {
215219
allowScripts[name] = true
216220
changes.push({ key: name, change: 'added' })
@@ -262,7 +266,7 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => {
262266

263267
// Apply a deny for a single package. Always name-only; ignores `--pin`.
264268
const applyDenyForPackage = (existing, nodes) => {
265-
const allowScripts = { ...(existing || {}) }
269+
const allowScripts = { ...existing }
266270
const changes = []
267271

268272
if (!Array.isArray(nodes) || nodes.length === 0) {

‎tap-snapshots/test/lib/commands/completion.js.test.cjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ exports[`test/lib/commands/completion.js TAP completion multiple command names >
6161
Array [
6262
String(
6363
access
64+
approve-scripts
6465
audit
6566
author
6667
add

‎tap-snapshots/test/lib/docs.js.test.cjs‎

Lines changed: 85 additions & 38 deletions

0 commit comments

Comments
 (0)