Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add pnpm as optional package manager #1020

Merged
merged 1 commit into from
Nov 12, 2023
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
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ This is for adding a module to be included in the default `citgm-all` runs.
- Module source code must be on Github.
- Published versions must include a tag on Github
- The test process must be executable with only the commands
`npm install && npm test` or (`yarn install && yarn test`) using the tarball
`npm install && npm test` or (`yarn install && yarn test`
or `pnpm install && pnpm test`) using the tarball
downloaded from the Github tag mentioned above
- The tests pass on supported major release lines
- The maintainers of the module remain responsive when there are problems
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Options:
--excludeTags tag1 tag2 Specify which tags to skip from the lookup (takes priority over includeTags)
Module names are automatically added as tags.
-y, --yarn Install and test the project using yarn instead of npm
--pnpm Install and test the project using pnpm instead of npm
```

When using a JSON config file, the properties need to be the same as the
Expand All @@ -134,13 +135,14 @@ For syntax, see [lookup.json](./lib/lookup.json), the available attributes are:
"stripAnsi": true Strip ansi data from output stream of npm
"sha": "<git-commit-sha>" Test against a specific commit
"envVar" Pass an environment variable before running
"install": ["install", "--param1", "--param2"] - Array of command line parameters passed to 'npm' or 'yarn' as install arguments
"install": ["install", "--param1", "--param2"] - Array of command line parameters passed to `npm` or `yarn` or `pnpm` as install arguments
"maintainers": ["user1", "user2"] - List of module maintainers to be contacted with issues
"scripts": ["script1", "script2"] - List of scripts from package.json to run instead of 'test'
"tags": ["tag1", "tag2"] Specify which tags apply to the module
"useGitClone": true Use a shallow git clone instead of downloading the module
"ignoreGitHead": Ignore the gitHead field if it exists and fallback to using github tags
"yarn": Install and test the project using yarn instead of npm
"pnpm": Install and test the project using pnpm instead of npm
"timeout": Number of milliseconds before timeout. Applies separately to `install` and `test`
```

Expand Down
1 change: 1 addition & 0 deletions lib/bin/citgm-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const options = {
tmpDir: app.tmpDir,
customTest: app.customTest,
yarn: app.yarn,
pnpm: app.pnpm,
includeTags: app.includeTags || [],
excludeTags: app.excludeTags || []
};
Expand Down
3 changes: 2 additions & 1 deletion lib/bin/citgm.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ const options = {
sha: app.sha,
tmpDir: app.tmpDir,
customTest: app.customTest,
yarn: app.yarn
yarn: app.yarn,
pnpm: app.pnpm
};

if (!windows) {
Expand Down
3 changes: 2 additions & 1 deletion lib/citgm.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ export class Tester extends EventEmitter {
init(this);
await findNode(this);

const { npm, yarn } = await getPackageManagers();
const { npm, yarn, pnpm } = await getPackageManagers();
this.npmPath = npm;
this.yarnPath = yarn;
this.pnpmPath = pnpm;

await tempDirectory.create(this);
await grabModuleData(this);
Expand Down
5 changes: 5 additions & 0 deletions lib/common-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ export function commonArgs() {
description: 'Install and test the project using yarn instead of npm',
default: false
})
.option('pnpm', {
type: 'boolean',
description: 'Install and test the project using pnpm instead of npm',
default: false
})
.example(
'citgm-all --customTest /path/to/customTest.js',
'Runs a custom node test script instead of "npm test"'
Expand Down
8 changes: 6 additions & 2 deletions lib/grab-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ export async function grabProject(context) {
}

return new Promise((resolve, reject) => {
const packageManager =
context.options.yarn || context.module.useYarn ? 'yarn' : 'npm';
let packageManager = 'npm';
if (context.options.yarn || context.module.useYarn) {
packageManager = 'yarn';
} else if (context.options.pnpm || context.module.usePnpm) {
packageManager = 'pnpm';
}
let packageName = context.module.raw;
if (context.module.type === 'directory') {
context.module.raw = context.module.name = path.basename(packageName);
Expand Down
3 changes: 3 additions & 0 deletions lib/lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ export function lookup(context) {
if (rep.yarn) {
context.module.useYarn = true;
}
if (rep.pnpm) {
context.module.usePnpm = true;
}
if (rep.timeout) {
context.module.timeout = rep.timeout;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/package-manager/get-executable.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const npmWhich = promisify(
);

export function getExecutable(binaryName) {
if (binaryName === 'yarn') {
// Use `npm-which` for yarn to get the local version
if (binaryName === 'yarn' || binaryName === 'pnpm') {
// Use `npm-which` for yarn or pnpm to get the local version
return npmWhich(binaryName);
} else {
return which(binaryName);
Expand Down
11 changes: 8 additions & 3 deletions lib/package-manager/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { getExecutable } from './get-executable.js';
export function pkgInstall(context) {
if (context.options.yarn || context.module.useYarn) {
return install('yarn', context);
} else if (context.options.pnpm || context.module.usePnpm) {
return install('pnpm', context);
} else {
return install('npm', context);
}
Expand All @@ -13,15 +15,18 @@ export function pkgInstall(context) {
export function pkgTest(context) {
if (context.options.yarn || context.module.useYarn) {
return test('yarn', context);
} else if (context.options.pnpm || context.module.usePnpm) {
return test('pnpm', context);
} else {
return test('npm', context);
}
}

export async function getPackageManagers() {
const [npm, yarn] = await Promise.all([
const [npm, yarn, pnpm] = await Promise.all([
getExecutable('npm'),
getExecutable('yarn')
getExecutable('yarn'),
getExecutable('pnpm')
]);
return { npm, yarn };
return { npm, yarn, pnpm };
}
22 changes: 18 additions & 4 deletions lib/package-manager/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ function getVersion(packageManager, context) {
path.join(context.path, context.module.name),
context
);
const packageManagerBin =
packageManager === 'npm' ? context.npmPath : context.yarnPath;
let packageManagerBin = context.npmPath;
if (packageManager === 'yarn') {
packageManagerBin = context.yarnPath;
} else if (packageManager === 'pnpm') {
packageManagerBin = context.pnpmPath;
}

const binDirectory = path.dirname(packageManagerBin);
options.env.PATH = `${binDirectory}${envSeparator}${process.env.PATH}`;
Expand Down Expand Up @@ -74,14 +78,20 @@ export default function install(packageManager, context) {
if (version && semverLt(version, '2.0.0', { includePrerelease: true })) {
options.env['YARN_IGNORE_ENGINES'] = 'true';
}
} else if (packageManager === 'pnpm') {
// No pnpm-specific options yet
}

if (context.module.install) {
args = context.module.install;
}

const packageManagerBin =
packageManager === 'npm' ? context.npmPath : context.yarnPath;
let packageManagerBin = context.npmPath;
if (packageManager === 'yarn') {
packageManagerBin = context.yarnPath;
} else if (packageManager === 'pnpm') {
packageManagerBin = context.pnpmPath;
}

const binDirectory = path.dirname(packageManagerBin);
options.env.PATH = `${binDirectory}${envSeparator}${process.env.PATH}`;
Expand All @@ -93,6 +103,10 @@ export default function install(packageManager, context) {
proc,
(err) => {
if (err) {
if (context.testError.length === 0) {
// Because pnpm prints errors to stdout
context.testError = context.testOutput;
}
return reject(err);
}
resolve();
Expand Down
20 changes: 12 additions & 8 deletions lib/package-manager/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,20 @@ export async function test(packageManager, context) {
bin = await which('node', { path: options.env.PATH });
}

const packageManagerBin =
packageManager === 'npm' ? context.npmPath : context.yarnPath;
let packageManagerBin = context.npmPath;
if (packageManager === 'yarn') {
packageManagerBin = context.yarnPath;
} else if (packageManager === 'pnpm') {
packageManagerBin = context.pnpmPath;
}

const binDirectory = dirname(packageManagerBin);
options.env.PATH = `${binDirectory}${envSeparator}${
options.env.PATH || process.env.PATH
}`;

/* Run `npm/yarn test`, or `/path/to/customTest.js` if the customTest option
was passed */
/* Run `npm/yarn/pnpm test`, or `/path/to/customTest.js` if the customTest
option was passed */
let scripts;
if (context.options.customTest) {
scripts = [[context.options.customTest]];
Expand All @@ -84,7 +88,7 @@ export async function test(packageManager, context) {
context.emit(
'data',
'silly',
`${context.module.name} npm-test:`,
`${context.module.name} ${packageManager}-test:`,
`Scripts to execute: ${JSON.stringify(scripts)}.`
);
context.scripts = scripts;
Expand All @@ -103,7 +107,7 @@ export async function test(packageManager, context) {
context.emit(
'data',
'silly',
`${context.module.name} npm-test:`,
`${context.module.name} ${packageManager}-test:`,
`Running script ${JSON.stringify(args)}.`
);

Expand All @@ -126,7 +130,7 @@ export async function test(packageManager, context) {
context.emit(
'data',
'verbose',
`${context.module.name} npm-test:`,
`${context.module.name} ${packageManager}-test:`,
data.toString()
);
});
Expand All @@ -135,7 +139,7 @@ export async function test(packageManager, context) {
context.emit(
'data',
'verbose',
`${context.module.name} npm-test:`,
`${context.module.name} ${packageManager}-test:`,
data.toString()
);
});
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"normalize-git-url": "^3.0.2",
"npm-package-arg": "^11.0.1",
"npm-which": "^3.0.1",
"pnpm": "^8.10.0",
"read-package-json": "^7.0.0",
"root-check": "^2.0.0",
"semver": "^7.5.4",
Expand Down
15 changes: 15 additions & 0 deletions test/bin/test-citgm-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,21 @@ test('citgm-all: install with yarn', (t) => {
});
});

test('citgm-all: install with pnpm', (t) => {
t.plan(1);
const proc = spawn(citgmAllPath, [
'-l',
'test/fixtures/custom-lookup.json',
'--pnpm'
]);
proc.on('error', (err) => {
t.error(err);
});
proc.on('close', (code) => {
t.equal(code, 0, 'citgm-all should only run omg-i-pass');
});
});

test('bin: sigterm', (t) => {
t.plan(1);

Expand Down
3 changes: 2 additions & 1 deletion test/helpers/make-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export function npmContext(mod, packageManagers, path, options) {
meta: {},
options: options,
npmPath: packageManagers.npm,
yarnPath: packageManagers.yarn
yarnPath: packageManagers.yarn,
pnpmPath: packageManagers.pnpm
};
return context;
}
88 changes: 88 additions & 0 deletions test/pnpm/test-pnpm-install.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { tmpdir } from 'os';
import { promises as fs } from 'fs';
import { join, dirname } from 'path';
import { fileURLToPath } from 'url';

import tap from 'tap';

import { getPackageManagers } from '../../lib/package-manager/index.js';
import packageManagerInstall from '../../lib/package-manager/install.js';
import { npmContext } from '../helpers/make-context.js';

const { test } = tap;

const sandbox = join(tmpdir(), `citgm-${Date.now()}-pnpm-install`);
const fixtures = join(
dirname(fileURLToPath(import.meta.url)),
'..',
'fixtures'
);
const moduleFixtures = join(fixtures, 'omg-i-pass');
const moduleTemp = join(sandbox, 'omg-i-pass');
const extraParamFixtures = join(fixtures, 'omg-i-pass-with-install-param');
const extraParamTemp = join(sandbox, 'omg-i-pass-with-install-param');
const badFixtures = join(fixtures, 'omg-bad-tree');
const badTemp = join(sandbox, 'omg-bad-tree');

let packageManagers;

test('pnpm-install: setup', async () => {
packageManagers = await getPackageManagers();
await fs.mkdir(sandbox, { recursive: true });
await Promise.all([
fs.cp(moduleFixtures, moduleTemp, { recursive: true }),
fs.cp(extraParamFixtures, extraParamTemp, { recursive: true }),
fs.cp(badFixtures, badTemp, { recursive: true })
]);
});

test('pnpm-install: basic module', async () => {
const context = npmContext('omg-i-pass', packageManagers, sandbox);
await packageManagerInstall('pnpm', context);
});

test('pnpm-install: no package.json', async (t) => {
t.plan(2);
const context = npmContext('omg-i-fail', packageManagers, sandbox);
try {
await packageManagerInstall('pnpm', context);
} catch (err) {
t.equal(err && err.message, 'Install Failed');
t.notOk(context.module.flaky, 'Module failed but is not flaky');
}
});

test('pnpm-install: timeout', async (t) => {
t.plan(2);
const context = npmContext('omg-i-pass', packageManagers, sandbox, {
timeout: 10
});
try {
await packageManagerInstall('pnpm', context);
} catch (err) {
t.notOk(context.module.flaky, 'Time out should not mark module flaky');
t.equal(err && err.message, 'Install Timed Out');
}
});

test('pnpm-install: failed install', async (t) => {
t.plan(3);
const context = npmContext('omg-bad-tree', packageManagers, sandbox);
const expected = /\/THIS-WILL-FAIL: Not Found/;
try {
await packageManagerInstall('pnpm', context);
} catch (err) {
t.notOk(context.module.flaky, 'Module failed is not flaky');
t.equal(err && err.message, 'Install Failed');
t.match(context.testError.toString(), expected, 'Install error reported');
}
});

tap.teardown(async () => {
await fs.rm(sandbox, {
recursive: true,
force: true,
maxRetries: 10,
retryDelay: 10
});
});
Loading
Loading