Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ permissions:
contents: read

jobs:
pnpm-commands:
name: 'pnpm command execution (${{ matrix.os }})'
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
steps:
- uses: actions/checkout@v7
- uses: ./
with:
version: '12.0.0'
runtime: node@24
- name: Test command execution and reject deprecation warnings
run: node --throw-deprecation --test src/pnpm-commands.test.mjs

smoke:
# Direct binary download + pnpm on PATH across OSes and architectures.
name: 'Smoke (${{ matrix.os }} / pnpm ${{ matrix.version }})'
Expand Down Expand Up @@ -1051,4 +1067,3 @@ jobs:
echo "Expected a plain install to write a lockfile"; exit 1
fi
shell: bash

226 changes: 113 additions & 113 deletions dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"build:bundle": "esbuild src/index.ts --bundle --platform=node --target=node24 --format=cjs --minify --outfile=dist/index.js",
"build": "pnpm run build:bundle",
"start": "pnpm run build && sh ./run.sh",
"test": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON --experimental-strip-types --test src/cache-restore/*.test.mjs src/install-runtime/*.test.mjs"
"test": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON --experimental-strip-types --test src/cache-restore/*.test.mjs src/install-runtime/*.test.mjs src/pnpm-commands.test.mjs"
},
"dependencies": {
"@actions/cache": "^6.2.0",
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ async function runPost() {
// installed, the log was already saved then. Runs before the prune because
// pnpm versions before pnpm/pnpm#13893 delete the log during one.
await saveVerificationCache()
pruneStore(inputs)
await pruneStore(inputs)
await saveCache(inputs)
}

Expand Down
4 changes: 1 addition & 3 deletions src/inputs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const options: InputOptions = {
required: true,
}

const parseInputPath = (name: string) => expandTilde(getInput(name, options))

function parseRuntime(): RuntimeInput | undefined {
const raw = getInput('runtime').trim()
if (!raw) return undefined
Expand Down Expand Up @@ -129,7 +127,7 @@ function isSupportedRuntime(name: string): name is RuntimeName {

export const getInputs = (): Inputs => ({
version: getInput('version'),
dest: parseInputPath('dest'),
dest: path.resolve(expandTilde(getInput('dest', options))),
cache: getBooleanInput('cache'),
...resolveProjectPaths(),
runtime: parseRuntime(),
Expand Down
123 changes: 123 additions & 0 deletions src/pnpm-commands.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import assert from 'node:assert/strict'
import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { createRequire } from 'node:module'
import path from 'node:path'
import { after, beforeEach, test } from 'node:test'
import { fileURLToPath } from 'node:url'
import { build } from 'esbuild'

const { outputFiles } = await build({
stdin: {
contents: `
export { getInputs } from './inputs/index.ts'
export { runPnpmInstall } from './pnpm-install/index.ts'
export { pruneStore } from './pnpm-store-prune/index.ts'
`,
resolveDir: fileURLToPath(new URL('.', import.meta.url)),
},
bundle: true,
platform: 'node',
format: 'cjs',
write: false,
})
const bundledModule = { exports: {} }
new Function('require', 'module', 'exports', outputFiles[0].text)(
createRequire(import.meta.url), bundledModule, bundledModule.exports,
)
const { getInputs, runPnpmInstall, pruneStore } = bundledModule.exports

const root = mkdtempSync(path.join(process.cwd(), '.pnpm-commands-test-'))
const dest = path.join(root, 'pnpm home & tools')
const project = path.join(root, 'project')
const record = path.join(root, 'command.json')
mkdirSync(path.join(dest, 'bin'), { recursive: true })
mkdirSync(project)
writeFileSync(path.join(project, 'package.json'), '{}')
writeFileSync(path.join(project, 'pnpm-lock.yaml'), '')

// A native executable named pnpm proves direct spawning works even when the
// destination contains spaces or shell metacharacters. Node consumes the
// fixture named "install" as its script, then passes through the install flags.
Comment on lines +37 to +40

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Narrating comments violate policy

This comment restates how the native executable fixture and its arguments work instead of letting the test structure express that intent. The same pattern appears in the PATH setup at line 84 and in src/pnpm-store-prune/index.ts at lines 13–15. This violates the repository directive that comments must not narrate code, so the narration must be removed or the surrounding names and structure made self-explanatory before merging.

Context Used: Comments and docs in code are suspicious. Is test coverage not sufficient the reason why a comment was added? Comments should not replace tests. Comments should also not narrate code. Is the code hard to understand? Then it should be refactored to ma... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

copyFileSync(process.execPath, path.join(dest, process.platform === 'win32' ? 'pnpm.exe' : 'pnpm'))
const recorder = `
const fs = require('node:fs')
setTimeout(() => {
fs.writeFileSync(process.env.PNPM_TEST_RECORD, JSON.stringify({
args: process.argv.slice(2), cwd: process.cwd(), executable: process.execPath,
}))
process.exitCode = Number(process.env.PNPM_TEST_EXIT_CODE || 0)
}, 25)
`
writeFileSync(path.join(project, 'install'), recorder)
const shimScript = path.join(dest, 'record.cjs')
writeFileSync(shimScript, recorder)
const quoteShell = value => `'${value.replaceAll("'", "'\\''")}'`
const shim = process.platform === 'win32'
? `@"${process.execPath}" "${shimScript}" %*\r\n`
: `#!/bin/sh\nexec ${quoteShell(process.execPath)} ${quoteShell(shimScript)} "$@"\n`
writeFileSync(path.join(dest, 'bin', process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm'), shim, { mode: 0o755 })
after(() => rmSync(root, { recursive: true, force: true }))

beforeEach(t => {
const previousEnv = { ...process.env }
const previousExitCode = process.exitCode
t.after(() => {
for (const name of Object.keys(process.env)) {
if (!(name in previousEnv)) delete process.env[name]
}
Object.assign(process.env, previousEnv)
process.exitCode = previousExitCode
})
Object.assign(process.env, {
GITHUB_WORKSPACE: root,
INPUT_DEST: path.relative(process.cwd(), dest),
INPUT_CACHE: 'true',
INPUT_INSTALL: 'true',
'INPUT_REQUIRE-LOCKFILE': 'true',
'INPUT_WORKING-DIRECTORY': 'project',
'INPUT_PACKAGE-JSON-FILE': '',
'INPUT_NODE-VERSION-FILE': '',
INPUT_RUNTIME: '',
PNPM_TEST_RECORD: record,
PNPM_TEST_EXIT_CODE: '0',
})
// Simulate the PATH prepared by setup, with self-update's shim first.
const pathKey = Object.keys(process.env).find(key => key.toUpperCase() === 'PATH') ?? 'PATH'
process.env[pathKey] = [path.join(dest, 'bin'), dest, process.env[pathKey]].join(path.delimiter)
rmSync(record, { force: true })
})

test('install uses the native binary with a relative dest and a separate project directory', () => {
const inputs = getInputs()
assert.equal(inputs.dest, dest)
runPnpmInstall(inputs, true)
const actual = JSON.parse(readFileSync(record, 'utf8'))
assert.deepEqual(actual.args, ['--frozen-lockfile', '--no-runtime'])
assert.equal(actual.cwd, project)
assert.equal(path.dirname(actual.executable), dest)
})

test('pruning awaits the self-updated shim ahead of the original executable on PATH', async () => {
await pruneStore(getInputs())
assert.deepEqual(JSON.parse(readFileSync(record, 'utf8')).args, ['store', 'prune'])
})

test('pruning is skipped when caching is disabled', async () => {
await pruneStore({ ...getInputs(), cache: false })
assert.throws(() => readFileSync(record), { code: 'ENOENT' })
})

test('pruning failure warns without failing the action', async t => {
process.env.PNPM_TEST_EXIT_CODE = '7'
const output = []
const write = process.stdout.write.bind(process.stdout)
t.mock.method(process.stdout, 'write', (chunk, ...args) => {
output.push(String(chunk))
return write(chunk, ...args)
})
const previousExitCode = process.exitCode
await pruneStore(getInputs())
assert.match(output.join(''), /::warning::.*exit code 7/)
assert.equal(process.exitCode, previousExitCode)
assert.deepEqual(JSON.parse(readFileSync(record, 'utf8')).args, ['store', 'prune'])
})
7 changes: 2 additions & 5 deletions src/pnpm-install/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,11 @@ export function runPnpmInstall(inputs: Inputs, runtimeInstalled = Boolean(inputs
return
}

// spawnSync inherits process.env, which already has $PNPM_HOME/bin and
// $PNPM_HOME prepended via addPath() in install-pnpm — so the pnpm this
// action installed (or a self-updated one) is the one that resolves.
const pnpmBin = path.join(inputs.dest, process.platform === 'win32' ? 'pnpm.exe' : 'pnpm')
startGroup(`Running ${command}...`)
const { error, status, signal } = spawnSync('pnpm', args, {
const { error, status, signal } = spawnSync(pnpmBin, args, {
stdio: 'inherit',
cwd: workingDirectory,
shell: true,
})
endGroup()

Expand Down
27 changes: 11 additions & 16 deletions src/pnpm-store-prune/index.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,23 @@
import { warning, startGroup, endGroup } from '@actions/core'
import { spawnSync } from 'child_process'
import { exec } from '@actions/exec'
import { Inputs } from '../inputs'

export function pruneStore(inputs: Inputs) {
export async function pruneStore(inputs: Inputs) {
if (!inputs.cache) {
// Without caching, the store is ephemeral with the runner — no need to prune.
return
}

startGroup('Running pnpm store prune...')
const { error, status } = spawnSync('pnpm', ['store', 'prune'], {
stdio: 'inherit',
shell: true,
})
endGroup()

if (error) {
warning(error)
return
}

if (status) {
warning(`command pnpm store prune exits with code ${status}`)
return
try {
// A later `pnpm self-update` puts its shim ahead of the original binary
// on PATH. The toolkit preserves that order and supports Windows .cmd
// shims without Node's deprecated `shell: true` + args combination.
await exec('pnpm', ['store', 'prune'])
} catch (error) {
warning(error instanceof Error ? error : String(error))
} finally {
endGroup()
}
}

Expand Down
Loading