Skip to content

Commit

Permalink
✅ All tests that run the CLI should set the COREPACK_HOME environme…
Browse files Browse the repository at this point in the history 
…nt variable to use the test-specific Corepack installation directory (#233)

All tests in `test/index.ts` except the "CLI should add Git tag with customized tag prefix" test did not set the environment variable `COREPACK_HOME` when running the CLI.
As a result, when the CLI tries to run the `npm config get tag-version-prefix` command internally, Corepack downloads npm to the default Corepack installation directory.
This caused a segmentation fault and the `npm config get tag-version-prefix` command was aborted by the `SIGSEGV` signal.
To fix this, set the environment variable `COREPACK_HOME` in all tests within `test/index.ts` and use the test-specific Corepack installation directory.

---------

* ✅ Set the `COREPACK_HOME` environment variable when running the CLI in tests ( 12ca0c1 )

* ✅ All `exec()` functions returned by the `initGit()` function should use the `COREPACK_HOME` environment variable ( f479280 )

    All CLI and package manager commands executed within `test/index.ts` should use the `COREPACK_HOME` environment variable.
    To keep this in mind, we modified our test code as follows:

    + Add the `execDefaultEnv` option to the `initGit()` function.
    + Add the `COREPACK_HOME` environment variable to the `execDefaultEnv` option of the `initGit()` function called in `test/index.ts`.

* ✅ Fix again problem with "pnpm add" commands failing on Windows ( 333c03c )

    I inadvertently caused this problem to reappear in the work on commit f479280.
    This is a problem that was previously fixed in commit 2613c17.

    Is it back again? I am probably an idiot.
    So I added a comment.

* 📝 Update CHANGELOG ( c11a224 )
  • Loading branch information
@sounisi5011
sounisi5011 committed Jul 3, 2023
1 parent 8dbca9f commit 5848d67
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ Because it is the minimum version available for Vitest.
* [#185] - Rename `ava.config.js` file to `ava.config.cjs`
* [#200] - Migrate from AVA to Vitest
* [#214] - Refactoring test code
* [#233] - All tests that run the CLI should set the `COREPACK_HOME` environment variable to use the test-specific Corepack installation directory

### Others

Expand Down Expand Up @@ -219,6 +220,7 @@ Because it is the minimum version available for Vitest.
[#227]: https://github.com/sounisi5011/package-version-git-tag/pull/227
[#228]: https://github.com/sounisi5011/package-version-git-tag/pull/228
[#230]: https://github.com/sounisi5011/package-version-git-tag/pull/230
[#233]: https://github.com/sounisi5011/package-version-git-tag/pull/233

## [3.0.0] (2020-06-02 UTC)

Expand Down
19 changes: 14 additions & 5 deletions test/helpers/git.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@ export type ExecFunc = (
) => ExecaChildProcess;
export type GitRemote = PromiseValue<ReturnType<typeof initGitServer>>;

interface InitGitOptions {
execDefaultEnv?: NodeJS.ProcessEnv | undefined;
}

/* eslint-disable import/export */
export async function initGit(
dirpath: string,
options: {
options: InitGitOptions & {
useRemoteRepo: true;
},
): Promise<{
Expand All @@ -28,7 +32,7 @@ export async function initGit(
}>;
export async function initGit(
dirpath: string,
options?: {
options?: InitGitOptions & {
useRemoteRepo?: false | undefined;
},
): Promise<{
Expand All @@ -39,7 +43,7 @@ export async function initGit(
}>;
export async function initGit(
dirpath: string,
options?: {
options?: InitGitOptions & {
useRemoteRepo?: boolean | undefined;
},
): Promise<{
Expand All @@ -49,14 +53,19 @@ export async function initGit(
remote: null | GitRemote;
}> {
const gitDirpath = path.resolve(dirpath);
const execDefaultEnv = options?.execDefaultEnv;
const exec: ExecFunc = ([command, ...args], options) =>
execa(command, args, {
cwd: gitDirpath,
extendEnv: false,
...options,
// By default, only the PATH environment variable is inherited.
// By default, only the PATH environment variable and execDefaultEnv are inherited.
// This is because some tests are broken by inheriting environment variables.
env: { PATH: process.env['PATH'], ...options?.env },
env: {
PATH: process.env['PATH'],
...execDefaultEnv,
...options?.env,
},
});
const version = [
getRandomInt(0, 99),
Expand Down
57 changes: 35 additions & 22 deletions test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ describe.concurrent('CLI should add Git tag', () => {
)('%s', async (testName, { cliArgs, expected }) => {
const { exec, version } = await initGit(
tmpDir('CLI should add Git tag', testName),
{ execDefaultEnv: { COREPACK_HOME } },
);

await expect(
Expand Down Expand Up @@ -119,6 +120,7 @@ describe.concurrent('CLI should not add Git tag with dry-run', () => {
test.each(['-n', '--dry-run'])('%s', async (option) => {
const { exec, version } = await initGit(
tmpDir('CLI should not add Git tag with dry-run', option),
{ execDefaultEnv: { COREPACK_HOME } },
);

const gitTagResult = exec(['git', 'tag', '-l']).then(
Expand Down Expand Up @@ -195,6 +197,7 @@ describe.concurrent(
'CLI should complete successfully if Git tag has been added',
testName,
),
{ execDefaultEnv: { COREPACK_HOME } },
);
await exec(['git', 'tag', `v${version}`]);

Expand Down Expand Up @@ -240,6 +243,7 @@ test.concurrent(
async () => {
const { exec, version } = await initGit(
tmpDir('CLI should fail if Git tag exists on different commits'),
{ execDefaultEnv: { COREPACK_HOME } },
);

await exec(['git', 'tag', `v${version}`]);
Expand Down Expand Up @@ -270,6 +274,7 @@ test.concurrent(
tmpDir(
'CLI push flag should fail if there is no remote repository',
),
{ execDefaultEnv: { COREPACK_HOME } },
);

await expect(
Expand Down Expand Up @@ -336,6 +341,7 @@ describe.concurrent('CLI should add and push Git tag', () => {
version,
remote: { tagList },
} = await initGit(tmpDir('CLI should add and push Git tag', testName), {
execDefaultEnv: { COREPACK_HOME },
useRemoteRepo: true,
});

Expand Down Expand Up @@ -381,7 +387,10 @@ test.concurrent(
remote: { tagList },
} = await initGit(
tmpDir('CLI should not add and not push Git tag with dry-run'),
{ useRemoteRepo: true },
{
execDefaultEnv: { COREPACK_HOME },
useRemoteRepo: true,
},
);

const gitTagResult = exec(['git', 'tag', '-l']).then(
Expand Down Expand Up @@ -425,6 +434,7 @@ test.concurrent('CLI should add and push single Git tag', async () => {
version,
remote: { tagList },
} = await initGit(tmpDir('CLI should add and push single Git tag'), {
execDefaultEnv: { COREPACK_HOME },
useRemoteRepo: true,
});

Expand Down Expand Up @@ -495,6 +505,7 @@ describe.concurrent.each(
test.each(optionList)('%s', async (option) => {
const { exec } = await initGit(
tmpDir(`CLI should to display`, testName, option),
{ execDefaultEnv: { COREPACK_HOME } },
);

const gitTagResult = exec(['git', 'tag', '-l']).then(
Expand Down Expand Up @@ -523,6 +534,7 @@ describe.concurrent.each(
test.concurrent('CLI should not work with unknown options', async () => {
const { exec } = await initGit(
tmpDir('CLI should not work with unknown options'),
{ execDefaultEnv: { COREPACK_HOME } },
);

const gitTagResult = exec(['git', 'tag', '-l']).then(
Expand Down Expand Up @@ -699,13 +711,6 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
testName: string,
{ pkgJson, configFile, commad }: Case,
): Promise<void> => {
const { exec, gitDirpath, version } = await initGit(
tmpDir(
'CLI should add Git tag with customized tag prefix',
...uniqueNameList,
testName,
),
);
const configValue: Record<typeof configFile, string> | undefined =
typeof customPrefix === 'string'
? {
Expand All @@ -732,10 +737,19 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
process.platform === 'win32'
? { APPDATA: process.env['APPDATA'] }
: {};
const env: NodeJS.ProcessEnv = {
...(packageManager?.type === 'pnpm' ? pnpmEnv : {}),
COREPACK_HOME,
};
const { exec, gitDirpath, version } = await initGit(
tmpDir(
'CLI should add Git tag with customized tag prefix',
...uniqueNameList,
testName,
),
{
execDefaultEnv: {
...(packageManager?.type === 'pnpm' ? pnpmEnv : {}),
COREPACK_HOME,
},
},
);
const pnpmConfig =
packageManager?.type !== 'pnpm'
? undefined
Expand Down Expand Up @@ -769,7 +783,6 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
['pnpm', 'config', 'get', 'node-version'],
{
env: {
...env,
COREPACK_ENABLE_STRICT: '0',
},
},
Expand All @@ -782,6 +795,11 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
if (commad.execCli[0] !== CLI_PATH) {
// Always use "pnpm add <folder>", even if the package manager is not pnpm.
// This is because the "yarn add /path/to/local/folder" command may fail on GitHub Actions.
// Note: This pnpm command is always executed independent of the value of "packageManager.type".
// Do not omit to specify "pnpmEnv"!
// The "APPDATA" environment variable is required,
// but the default "env" option specified in "execDefaultEnv" has the "APPDATA" environment variable
// only if "packageManager.type === 'pnpm'".
await exec(['pnpm', 'add', PROJECT_ROOT], { env: pnpmEnv });
}
await Promise.all([
Expand Down Expand Up @@ -811,9 +829,7 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
]);

if (packageManager) {
await expect(
exec(commad.version, { env }),
).resolves.toMatchObject({
await expect(exec(commad.version)).resolves.toMatchObject({
stdout: packageManager.semver.version,
stderr: '',
});
Expand All @@ -822,9 +838,8 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
await expect(
exec(commad.getPrefix, {
env: pnpmConfig?.isNewPnpmConfig
? env
? {}
: {
...env,
// The old pnpm "pnpm config" command executes the "npm config" command internally.
// see https://github.com/pnpm/pnpm/blob/v7.19.0/pnpm/src/pnpm.ts#L27-L64
// Thus, we will set this environment variable so that npm can be used.
Expand All @@ -840,9 +855,8 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
await expect(
exec(commad.getPrefix, {
env: pnpmConfig?.isNewPnpmConfig
? env
? {}
: {
...env,
// The old pnpm "pnpm config" command executes the "npm config" command internally.
// see https://github.com/pnpm/pnpm/blob/v7.19.0/pnpm/src/pnpm.ts#L27-L64
// Thus, we will set this environment variable so that npm can be used.
Expand All @@ -866,7 +880,7 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
).resolves.toMatchObject({ stdout: '', stderr: '' });

await expect(
exec(commad.execCli, { env }),
exec(commad.execCli),
'CLI should exits successfully',
).resolves.toSatisfy(() => true);

Expand All @@ -889,7 +903,6 @@ describe.concurrent('CLI should add Git tag with customized tag prefix', () => {
await exec(['git', 'commit', '-m', 'Second commit']);
await exec(commad.setNewVersion(newVersion), {
env: {
...env,
// The "pnpm version" command executes the "npm version" command internally.
// see https://github.com/pnpm/pnpm/blob/v7.30.0/pnpm/src/pnpm.ts#L27-L61
// Thus, we will set this environment variable so that npm can be used.
Expand Down

0 comments on commit 5848d67

Please sign in to comment.