Skip to content

Commit

Permalink
Update execa (#121)
Browse files Browse the repository at this point in the history
This PR updates `execa` to `^8.0.1`. Since `execa@>=6.0.0` is ESM-only and `jest` only has experimental ESM support (jestjs/jest#10976), this required switching from `ts-jest` to `babel-jest`. To minimize dependency transpilation, the ESM packages that are necessary to transpile are enumerated in `jest.config.js`.

This version of `execa` includes [automatic escaping of shell arguments](https://github.com/sindresorhus/execa/tree/v8.0.1#execafile-arguments-options), which was the entire point of #112, #113, and this PR.

The state of ESM support in the Node.js ecosystem is absolutely horrible, and I would not recommend further migrations for the time being. We should continue to dual-release our packages and avoid ESM-only dependencies until the ecosystem has matured. For details see the above `jest` issue and nodejs/node#37648.
  • Loading branch information
rekmarks committed Dec 8, 2023
1 parent 858cb26 commit f969b0e
Show file tree
Hide file tree
Showing 10 changed files with 2,220 additions and 482 deletions.
10 changes: 10 additions & 0 deletions babel.config.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// We use Babel to transpile down ESM dependencies to CommonJS for our tests
// using babel-jest.
module.exports = {
env: {
test: {
presets: ['@babel/preset-env', '@babel/preset-typescript'],
plugins: ['@babel/plugin-transform-modules-commonjs']
}
}
};
40 changes: 33 additions & 7 deletions jest.config.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,32 @@
* https://jestjs.io/docs/configuration
*/

/**
* Dependencies that are ESM-only, and need to be transpiled by Babel.
* This list is used in the `transformIgnorePatterns` option below.
*
* You probably need to add a dependency to this list if the tests fail with something like:
* - `SyntaxError: Cannot use import statement outside a module`
* - `SyntaxError: Unexpected token 'export'`
* If so, identify the dependency that's causing the error via the stack trace, and add it
* to this list.
*
* No, we do not live in the best of all possible worlds. Why do you ask?
*
* For details on Jest's currently experimental ESM support see: https://github.com/jestjs/jest/issues/9430
*/
const ESM_DEPENDENCIES = [
'execa',
'strip-final-newline',
'npm-run-path',
'path-key',
'onetime',
'mimic-fn',
'human-signals',
'is-stream',
'get-stream',
];

// This file needs to be .cjs for compatibility with jest-it-up.
module.exports = {
// All imported modules in your tests should be mocked automatically
Expand Down Expand Up @@ -103,8 +129,9 @@ module.exports = {
// An enum that specifies notification mode. Requires { notify: true }
// notifyMode: "failure-change",

// A preset that is used as a base for Jest's configuration
preset: 'ts-jest',
// Disabled in favor of babel-jest, configured via "transform" below.
// // A preset that is used as a base for Jest's configuration
// preset: 'ts-jest',

// Run tests from one or more projects
// projects: undefined,
Expand Down Expand Up @@ -189,13 +216,12 @@ module.exports = {
// timers: "real",

// A map from regular expressions to paths to transformers
// transform: undefined,
transform: {
"\\.[jt]sx?$": "babel-jest"
},

// An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation
// transformIgnorePatterns: [
// "/node_modules/",
// "\\.pnp\\.[^\\/]+$"
// ],
transformIgnorePatterns: [`node_modules/(?!(${ESM_DEPENDENCIES.join('|')}))`],

// An array of regexp pattern strings that are matched against all modules before the module loader will automatically return a mock for them
// unmockedModulePathPatterns: undefined,
Expand Down
12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,33 @@
"@metamask/auto-changelog": "~3.3.0",
"@metamask/utils": "^8.2.1",
"debug": "^4.3.4",
"execa": "^5.1.1",
"execa": "^8.0.1",
"pony-cause": "^2.1.9",
"semver": "^7.5.4",
"which": "^3.0.0",
"yaml": "^2.2.2",
"yargs": "^17.7.1"
},
"devDependencies": {
"@babel/core": "^7.23.5",
"@babel/plugin-transform-modules-commonjs": "^7.23.3",
"@babel/preset-env": "^7.23.5",
"@babel/preset-typescript": "^7.23.3",
"@lavamoat/allow-scripts": "^2.3.1",
"@metamask/eslint-config": "^10.0.0",
"@metamask/eslint-config-jest": "^10.0.0",
"@metamask/eslint-config-nodejs": "^10.0.0",
"@metamask/eslint-config-typescript": "^10.0.0",
"@types/debug": "^4.1.7",
"@types/jest": "^29.5.1",
"@types/jest": "^29.5.10",
"@types/jest-when": "^3.5.2",
"@types/node": "^17.0.23",
"@types/rimraf": "^4.0.5",
"@types/which": "^3.0.0",
"@types/yargs": "^17.0.10",
"@typescript-eslint/eslint-plugin": "^5.62.0",
"@typescript-eslint/parser": "^5.62.0",
"babel-jest": "^29.7.0",
"deepmerge": "^4.2.2",
"eslint": "^8.27.0",
"eslint-config-prettier": "^8.5.0",
Expand All @@ -59,15 +64,14 @@
"eslint-plugin-jsdoc": "^39.6.2",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.2.1",
"jest": "^29.5.0",
"jest": "^29.7.0",
"jest-it-up": "^3.0.0",
"jest-when": "^3.5.2",
"nanoid": "^3.3.4",
"prettier": "^2.2.1",
"prettier-plugin-packagejson": "^2.3.0",
"rimraf": "^4.0.5",
"stdio-mock": "^1.2.0",
"ts-jest": "^29.1.0",
"tsx": "^4.6.1",
"typescript": "~5.1.6"
},
Expand Down
8 changes: 4 additions & 4 deletions src/misc-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('misc-utils', () => {
describe('runCommand', () => {
it('runs the command, discarding its output', async () => {
const execaSpy = jest
.spyOn(execaModule, 'default')
.spyOn(execaModule, 'execa')
// Typecast: It's difficult to provide a full return value for execa
.mockResolvedValue({ stdout: ' some output ' } as any);

Expand All @@ -155,7 +155,7 @@ describe('misc-utils', () => {
describe('getStdoutFromCommand', () => {
it('executes the given command and returns a version of the standard out from the command with whitespace trimmed', async () => {
const execaSpy = jest
.spyOn(execaModule, 'default')
.spyOn(execaModule, 'execa')
// Typecast: It's difficult to provide a full return value for execa
.mockResolvedValue({ stdout: ' some output ' } as any);

Expand All @@ -175,7 +175,7 @@ describe('misc-utils', () => {
describe('getLinesFromCommand', () => {
it('executes the given command and returns the standard out from the command split into lines', async () => {
const execaSpy = jest
.spyOn(execaModule, 'default')
.spyOn(execaModule, 'execa')
// Typecast: It's difficult to provide a full return value for execa
.mockResolvedValue({ stdout: 'line 1\nline 2\nline 3' } as any);

Expand All @@ -193,7 +193,7 @@ describe('misc-utils', () => {

it('does not strip leading and trailing whitespace from the output, but does remove empty lines', async () => {
const execaSpy = jest
.spyOn(execaModule, 'default')
.spyOn(execaModule, 'execa')
// Typecast: It's difficult to provide a full return value for execa
.mockResolvedValue({
stdout: ' line 1\nline 2\n\n line 3 \n',
Expand Down
8 changes: 4 additions & 4 deletions src/misc-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import which from 'which';
import execa from 'execa';
import { execa, Options } from 'execa';
import createDebug from 'debug';
import { ErrorWithCause } from 'pony-cause';
import { isObject } from '@metamask/utils';
Expand Down Expand Up @@ -130,7 +130,7 @@ export async function resolveExecutable(
export async function runCommand(
command: string,
args?: readonly string[] | undefined,
options?: execa.Options<string> | undefined,
options?: Options | undefined,
): Promise<void> {
await execa(command, args, options);
}
Expand All @@ -149,7 +149,7 @@ export async function runCommand(
export async function getStdoutFromCommand(
command: string,
args?: readonly string[] | undefined,
options?: execa.Options<string> | undefined,
options?: Options | undefined,
): Promise<string> {
return (await execa(command, args, options)).stdout.trim();
}
Expand All @@ -167,7 +167,7 @@ export async function getStdoutFromCommand(
export async function getLinesFromCommand(
command: string,
args?: readonly string[] | undefined,
options?: execa.Options<string> | undefined,
options?: Options | undefined,
): Promise<string[]> {
const { stdout } = await execa(command, args, options);
return stdout.split('\n').filter((value) => value !== '');
Expand Down
82 changes: 82 additions & 0 deletions src/monorepo-workflow-operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,88 @@ describe('monorepo-workflow-operations', () => {
});
});

it('follows the workflow correctly when executed twice', async () => {
await withSandbox(async (sandbox) => {
const releaseVersion = '1.1.0';
const {
project,
stdout,
stderr,
createReleaseBranchSpy,
commitAllChangesSpy,
projectDirectoryPath,
} = await setupFollowMonorepoWorkflow({
sandbox,
releaseVersion,
doesReleaseSpecFileExist: false,
isEditorAvailable: true,
});

createReleaseBranchSpy.mockResolvedValueOnce({
version: releaseVersion,
firstRun: true,
});

await followMonorepoWorkflow({
project,
tempDirectoryPath: sandbox.directoryPath,
firstRemovingExistingReleaseSpecification: false,
releaseType: 'ordinary',
defaultBranch: 'main',
stdout,
stderr,
});

expect(createReleaseBranchSpy).toHaveBeenCalledTimes(1);
expect(createReleaseBranchSpy).toHaveBeenLastCalledWith({
project,
releaseType: 'ordinary',
});

expect(commitAllChangesSpy).toHaveBeenCalledTimes(2);
expect(commitAllChangesSpy).toHaveBeenNthCalledWith(
1,
projectDirectoryPath,
`Initialize Release ${releaseVersion}`,
);
expect(commitAllChangesSpy).toHaveBeenNthCalledWith(
2,
projectDirectoryPath,
`Update Release ${releaseVersion}`,
);

// Second call of followMonorepoWorkflow

createReleaseBranchSpy.mockResolvedValueOnce({
version: releaseVersion,
firstRun: false, // It's no longer the first run
});

await followMonorepoWorkflow({
project,
tempDirectoryPath: sandbox.directoryPath,
firstRemovingExistingReleaseSpecification: false,
releaseType: 'ordinary',
defaultBranch: 'main',
stdout,
stderr,
});

expect(createReleaseBranchSpy).toHaveBeenCalledTimes(2);
expect(createReleaseBranchSpy).toHaveBeenLastCalledWith({
project,
releaseType: 'ordinary',
});

expect(commitAllChangesSpy).toHaveBeenCalledTimes(3);
expect(commitAllChangesSpy).toHaveBeenNthCalledWith(
3,
projectDirectoryPath,
`Update Release ${releaseVersion}`,
);
});
});

it('attempts to execute the release spec if it was successfully edited', async () => {
await withSandbox(async (sandbox) => {
const {
Expand Down
1 change: 1 addition & 0 deletions src/repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ export async function hasChangesInDirectorySinceGitTag(
tagName,
);

/* istanbul ignore else */
if (!(tagName in CHANGED_FILE_PATHS_BY_TAG_NAME)) {
CHANGED_FILE_PATHS_BY_TAG_NAME[tagName] = changedFilePaths;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/helpers/monorepo-environment.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'fs';
import path from 'path';
import { ExecaReturnValue } from 'execa';
import type { ExecaReturnValue } from 'execa';
import YAML from 'yaml';
import { TOOL_EXECUTABLE_PATH, TSX_PATH } from './constants.js';
import Environment, {
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/helpers/repo.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'fs';
import path from 'path';
import execa, { ExecaChildProcess, Options as ExecaOptions } from 'execa';
import { execa, ExecaChildProcess, Options as ExecaOptions } from 'execa';
import deepmerge from 'deepmerge';
import { isErrorWithCode } from '../../helpers.js';
import { debug, sleepFor } from './utils.js';
Expand Down Expand Up @@ -178,7 +178,7 @@ export default abstract class Repo {
async runCommand(
executableName: string,
args?: readonly string[] | undefined,
options?: ExecaOptions<string> | undefined,
options?: ExecaOptions | undefined,
): Promise<ExecaChildProcess<string>> {
const { env, ...remainingOptions } =
options === undefined ? { env: {} } : options;
Expand Down
Loading

0 comments on commit f969b0e

Please sign in to comment.