From 7fbac5cfac505ad7ca8ffd8bceb262d02d871dcb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 29 Nov 2025 15:53:03 +0100 Subject: [PATCH] cli: update to use new run utils from cli-common Signed-off-by: Patrik Oldsberg --- .changeset/tender-dancers-hunt.md | 5 + packages/cli/src/lib/run.ts | 121 ------------------ .../cli/src/lib/versioning/packages.test.ts | 50 ++++---- packages/cli/src/lib/versioning/packages.ts | 15 ++- packages/cli/src/lib/versioning/yarn.ts | 11 +- .../src/modules/build/lib/bundler/config.ts | 6 +- .../build/lib/packager/createDistWorkspace.ts | 10 +- .../cli/src/modules/info/commands/info.ts | 4 +- .../maintenance/commands/repo/clean.ts | 12 +- .../migrate/commands/packageLintConfigs.ts | 4 +- .../migrate/commands/versions/bump.test.ts | 116 ++++++++++------- .../modules/migrate/commands/versions/bump.ts | 4 +- .../migrate/commands/versions/migrate.test.ts | 69 +++++----- packages/cli/src/modules/migrate/lib/utils.ts | 6 +- packages/cli/src/modules/new/lib/tasks.ts | 14 +- .../src/modules/test/commands/package/test.ts | 6 +- .../src/modules/test/commands/repo/test.ts | 12 +- 17 files changed, 178 insertions(+), 287 deletions(-) create mode 100644 .changeset/tender-dancers-hunt.md delete mode 100644 packages/cli/src/lib/run.ts diff --git a/.changeset/tender-dancers-hunt.md b/.changeset/tender-dancers-hunt.md new file mode 100644 index 0000000000..e20ec43ae0 --- /dev/null +++ b/.changeset/tender-dancers-hunt.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Updated to use new utilities from `@backstage/cli-common`. diff --git a/packages/cli/src/lib/run.ts b/packages/cli/src/lib/run.ts deleted file mode 100644 index e8602fcb18..0000000000 --- a/packages/cli/src/lib/run.ts +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - SpawnOptions, - spawn, - ChildProcess, - execFile as execFileCb, -} from 'child_process'; -import { ExitCodeError } from './errors'; -import { promisify } from 'util'; -import { assertError, ForwardedError } from '@backstage/errors'; - -export const execFile = promisify(execFileCb); - -type LogFunc = (data: Buffer) => void; - -type SpawnOptionsPartialEnv = Omit & { - env?: Partial; - // Pipe stdout to this log function - stdoutLogFunc?: LogFunc; - // Pipe stderr to this log function - stderrLogFunc?: LogFunc; -}; - -// Runs a child command, returning a promise that is only resolved if the child exits with code 0. -export async function run( - name: string, - args: string[] = [], - options: SpawnOptionsPartialEnv = {}, -) { - const { stdoutLogFunc, stderrLogFunc } = options; - const env: NodeJS.ProcessEnv = { - ...process.env, - FORCE_COLOR: 'true', - ...(options.env ?? {}), - }; - - const stdio = [ - 'inherit', - stdoutLogFunc ? 'pipe' : 'inherit', - stderrLogFunc ? 'pipe' : 'inherit', - ] as ('inherit' | 'pipe')[]; - - const child = spawn(name, args, { - stdio, - shell: true, - ...options, - env, - }); - - if (stdoutLogFunc && child.stdout) { - child.stdout.on('data', stdoutLogFunc); - } - if (stderrLogFunc && child.stderr) { - child.stderr.on('data', stderrLogFunc); - } - - await waitForExit(child, name); -} - -export async function runPlain(cmd: string, ...args: string[]) { - try { - const { stdout } = await execFile(cmd, args, { shell: true }); - return stdout.trim(); - } catch (error) { - assertError(error); - if ('stderr' in error) { - process.stderr.write(error.stderr as Buffer); - } - if (typeof error.code === 'number') { - throw new ExitCodeError(error.code, [cmd, ...args].join(' ')); - } - throw new ForwardedError('Unknown execution error', error); - } -} - -export async function runCheck(cmd: string, ...args: string[]) { - try { - await execFile(cmd, args, { shell: true }); - return true; - } catch (error) { - return false; - } -} - -export async function waitForExit( - child: ChildProcess & { exitCode: number | null }, - name?: string, -): Promise { - if (typeof child.exitCode === 'number') { - if (child.exitCode) { - throw new ExitCodeError(child.exitCode, name); - } - return; - } - - await new Promise((resolve, reject) => { - child.once('error', error => reject(error)); - child.once('exit', code => { - if (code) { - reject(new ExitCodeError(code, name)); - } else { - resolve(); - } - }); - }); -} diff --git a/packages/cli/src/lib/versioning/packages.test.ts b/packages/cli/src/lib/versioning/packages.test.ts index de6e7e1ed2..e823ce8769 100644 --- a/packages/cli/src/lib/versioning/packages.test.ts +++ b/packages/cli/src/lib/versioning/packages.test.ts @@ -14,16 +14,17 @@ * limitations under the License. */ -import * as runObj from '../run'; +import * as runObj from '@backstage/cli-common'; import * as yarn from './yarn'; import { fetchPackageInfo, mapDependencies } from './packages'; import { createMockDirectory } from '@backstage/backend-test-utils'; import { NotFoundError } from '@backstage/errors'; -jest.mock('../run', () => { +jest.mock('@backstage/cli-common', () => { + const actual = jest.requireActual('@backstage/cli-common'); return { - run: jest.fn(), - execFile: jest.fn(), + ...actual, + runOutput: jest.fn(), }; }); @@ -39,42 +40,40 @@ describe('fetchPackageInfo', () => { }); it('should forward info for yarn classic', async () => { - jest.spyOn(runObj, 'execFile').mockResolvedValue({ - stdout: `{"type":"inspect","data":{"the":"data"}}`, - stderr: '', - }); + jest + .spyOn(runObj, 'runOutput') + .mockResolvedValue(`{"type":"inspect","data":{"the":"data"}}`); jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('classic'); await expect(fetchPackageInfo('my-package')).resolves.toEqual({ the: 'data', }); - expect(runObj.execFile).toHaveBeenCalledWith( + expect(runObj.runOutput).toHaveBeenCalledWith([ 'yarn', - ['info', '--json', 'my-package'], - { shell: true }, - ); + 'info', + '--json', + 'my-package', + ]); }); it('should forward info for yarn berry', async () => { - jest - .spyOn(runObj, 'execFile') - .mockResolvedValue({ stdout: `{"the":"data"}`, stderr: '' }); + jest.spyOn(runObj, 'runOutput').mockResolvedValue(`{"the":"data"}`); jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('berry'); await expect(fetchPackageInfo('my-package')).resolves.toEqual({ the: 'data', }); - expect(runObj.execFile).toHaveBeenCalledWith( + expect(runObj.runOutput).toHaveBeenCalledWith([ 'yarn', - ['npm', 'info', '--json', 'my-package'], - { shell: true }, - ); + 'npm', + 'info', + '--json', + 'my-package', + ]); }); it('should throw if no info with yarn classic', async () => { - jest - .spyOn(runObj, 'execFile') - .mockResolvedValue({ stdout: '', stderr: '' }); + jest.spyOn(runObj, 'runOutput').mockResolvedValue(''); jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('classic'); await expect(fetchPackageInfo('my-package')).rejects.toThrow( @@ -83,9 +82,10 @@ describe('fetchPackageInfo', () => { }); it('should throw if no info with yarn berry', async () => { - jest - .spyOn(runObj, 'execFile') - .mockRejectedValue({ stdout: 'bla bla bla Response Code: 404 bla bla' }); + const error = new Error('Command failed'); + (error as Error & { stdout?: string }).stdout = + 'bla bla bla Response Code: 404 bla bla'; + jest.spyOn(runObj, 'runOutput').mockRejectedValue(error); jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('berry'); await expect(fetchPackageInfo('my-package')).rejects.toThrow( diff --git a/packages/cli/src/lib/versioning/packages.ts b/packages/cli/src/lib/versioning/packages.ts index 223f6b8aab..11f1ba3041 100644 --- a/packages/cli/src/lib/versioning/packages.ts +++ b/packages/cli/src/lib/versioning/packages.ts @@ -17,7 +17,7 @@ import { minimatch } from 'minimatch'; import { getPackages } from '@manypkg/get-packages'; import { detectYarnVersion } from './yarn'; -import { execFile } from '../run'; +import { runOutput } from '@backstage/cli-common'; import { NotFoundError } from '@backstage/errors'; const DEP_TYPES = [ @@ -54,11 +54,7 @@ export async function fetchPackageInfo( const cmd = yarnVersion === 'classic' ? ['info'] : ['npm', 'info']; try { - const { stdout: output } = await execFile( - 'yarn', - [...cmd, '--json', name], - { shell: true }, - ); + const output = await runOutput(['yarn', ...cmd, '--json', name]); if (!output) { throw new NotFoundError( @@ -81,7 +77,12 @@ export async function fetchPackageInfo( throw error; } - if (error?.stdout.includes('Response Code: 404')) { + if ( + error instanceof Error && + 'stdout' in error && + typeof error.stdout === 'string' && + error.stdout.includes('Response Code: 404') + ) { throw new NotFoundError( `No package information found for package ${name}`, ); diff --git a/packages/cli/src/lib/versioning/yarn.ts b/packages/cli/src/lib/versioning/yarn.ts index b6c0383bc2..908ddca949 100644 --- a/packages/cli/src/lib/versioning/yarn.ts +++ b/packages/cli/src/lib/versioning/yarn.ts @@ -15,10 +15,7 @@ */ import { assertError, ForwardedError } from '@backstage/errors'; -import { execFile as execFileCb } from 'child_process'; -import { promisify } from 'util'; - -const execFile = promisify(execFileCb); +import { runOutput } from '@backstage/cli-common'; const versions = new Map>(); @@ -30,16 +27,12 @@ export function detectYarnVersion(dir?: string): Promise<'classic' | 'berry'> { const promise = Promise.resolve().then(async () => { try { - const { stdout } = await execFile('yarn', ['--version'], { - shell: true, + const stdout = await runOutput(['yarn', '--version'], { cwd, }); return stdout.trim().startsWith('1.') ? 'classic' : 'berry'; } catch (error) { assertError(error); - if ('stderr' in error) { - process.stderr.write(error.stderr as Buffer); - } throw new ForwardedError('Failed to determine yarn version', error); } }); diff --git a/packages/cli/src/modules/build/lib/bundler/config.ts b/packages/cli/src/modules/build/lib/bundler/config.ts index da444dff15..284d49d10a 100644 --- a/packages/cli/src/modules/build/lib/bundler/config.ts +++ b/packages/cli/src/modules/build/lib/bundler/config.ts @@ -29,7 +29,7 @@ import { paths as cliPaths } from '../../../../lib/paths'; import fs from 'fs-extra'; import { optimization as optimizationConfig } from './optimization'; import pickBy from 'lodash/pickBy'; -import { runPlain } from '../../../../lib/run'; +import { runOutput } from '@backstage/cli-common'; import { transforms } from './transforms'; import { version } from '../../../../lib/version'; import yn from 'yn'; @@ -78,14 +78,14 @@ async function readBuildInfo() { let commit: string | undefined; try { - commit = await runPlain('git', 'rev-parse', 'HEAD'); + commit = await runOutput(['git', 'rev-parse', 'HEAD']); } catch (error) { // ignore, see below } let gitVersion: string | undefined; try { - gitVersion = await runPlain('git', 'describe', '--always'); + gitVersion = await runOutput(['git', 'describe', '--always']); } catch (error) { // ignore, see below } diff --git a/packages/cli/src/modules/build/lib/packager/createDistWorkspace.ts b/packages/cli/src/modules/build/lib/packager/createDistWorkspace.ts index 971409cd93..cde4d666d0 100644 --- a/packages/cli/src/modules/build/lib/packager/createDistWorkspace.ts +++ b/packages/cli/src/modules/build/lib/packager/createDistWorkspace.ts @@ -25,7 +25,7 @@ import { tmpdir } from 'os'; import tar, { CreateOptions, FileOptions } from 'tar'; import partition from 'lodash/partition'; import { paths } from '../../../../lib/paths'; -import { run } from '../../../../lib/run'; +import { run } from '@backstage/cli-common'; import { dependencies as cliDependencies, devDependencies as cliDevDependencies, @@ -228,11 +228,11 @@ export async function createDistWorkspace( await runParallelWorkers({ items: customBuild, worker: async ({ name, dir, args }) => { - await run('yarn', ['run', 'build', ...(args || [])], { + await run(['yarn', 'run', 'build', ...(args || [])], { cwd: dir, stdoutLogFunc: prefixLogFunc(`${name}: `, 'stdout'), stderrLogFunc: prefixLogFunc(`${name}: `, 'stderr'), - }); + }).waitForExit(); }, }); } @@ -321,9 +321,9 @@ async function moveToDistWorkspace( console.log(`Repacking ${target.name} into dist workspace`); const archivePath = resolvePath(workspaceDir, archive); - await run('yarn', ['pack', '--filename', archivePath], { + await run(['yarn', 'pack', '--filename', archivePath], { cwd: target.dir, - }); + }).waitForExit(); const outputDir = relativePath(paths.targetRoot, target.dir); const absoluteOutputPath = resolvePath(workspaceDir, outputDir); diff --git a/packages/cli/src/modules/info/commands/info.ts b/packages/cli/src/modules/info/commands/info.ts index 416c995541..97090d8b9a 100644 --- a/packages/cli/src/modules/info/commands/info.ts +++ b/packages/cli/src/modules/info/commands/info.ts @@ -16,14 +16,14 @@ import { version as cliVersion } from '../../../../package.json'; import os from 'os'; -import { runPlain } from '../../../lib/run'; +import { runOutput } from '@backstage/cli-common'; import { paths } from '../../../lib/paths'; import { Lockfile } from '../../../lib/versioning'; import fs from 'fs-extra'; export default async () => { await new Promise(async () => { - const yarnVersion = await runPlain('yarn --version'); + const yarnVersion = await runOutput(['yarn', '--version']); const isLocal = fs.existsSync(paths.resolveOwn('./src')); const backstageFile = paths.resolveTargetRoot('backstage.json'); diff --git a/packages/cli/src/modules/maintenance/commands/repo/clean.ts b/packages/cli/src/modules/maintenance/commands/repo/clean.ts index 51c31129ea..c76076ad55 100644 --- a/packages/cli/src/modules/maintenance/commands/repo/clean.ts +++ b/packages/cli/src/modules/maintenance/commands/repo/clean.ts @@ -14,14 +14,11 @@ * limitations under the License. */ -import { execFile as execFileCb } from 'child_process'; import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; -import { promisify } from 'util'; import { PackageGraph } from '@backstage/cli-node'; import { paths } from '../../../../lib/paths'; - -const execFile = promisify(execFileCb); +import { run } from '@backstage/cli-common'; export async function command(): Promise { const packages = await PackageGraph.listTargetPackages(); @@ -44,12 +41,9 @@ export async function command(): Promise { await fs.remove(resolvePath(pkg.dir, 'dist-types')); await fs.remove(resolvePath(pkg.dir, 'coverage')); } else if (cleanScript) { - const result = await execFile('yarn', ['run', 'clean'], { + await run(['yarn', 'run', 'clean'], { cwd: pkg.dir, - shell: true, - }); - process.stdout.write(result.stdout); - process.stderr.write(result.stderr); + }).waitForExit(); } } }), diff --git a/packages/cli/src/modules/migrate/commands/packageLintConfigs.ts b/packages/cli/src/modules/migrate/commands/packageLintConfigs.ts index c8e8ef3a42..8f5e395b98 100644 --- a/packages/cli/src/modules/migrate/commands/packageLintConfigs.ts +++ b/packages/cli/src/modules/migrate/commands/packageLintConfigs.ts @@ -17,7 +17,7 @@ import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; import { PackageGraph } from '@backstage/cli-node'; -import { runPlain } from '../../../lib/run'; +import { runOutput } from '@backstage/cli-common'; const PREFIX = `module.exports = require('@backstage/cli/config/eslint-factory')`; @@ -84,6 +84,6 @@ export async function command() { } if (hasPrettier) { - await runPlain('prettier', '--write', ...configPaths); + await runOutput(['prettier', '--write', ...configPaths]); } } diff --git a/packages/cli/src/modules/migrate/commands/versions/bump.test.ts b/packages/cli/src/modules/migrate/commands/versions/bump.test.ts index 738610d09c..202704e030 100644 --- a/packages/cli/src/modules/migrate/commands/versions/bump.test.ts +++ b/packages/cli/src/modules/migrate/commands/versions/bump.test.ts @@ -15,7 +15,7 @@ */ import fs from 'fs-extra'; import { Command } from 'commander'; -import * as runObj from '../../../../lib/run'; +import * as runObj from '@backstage/cli-common'; import bump, { bumpBackstageJsonVersion, createVersionFinder } from './bump'; import { registerMswTestHooks, withLogCollector } from '@backstage/test-utils'; import { YarnInfoInspectData } from '../../../../lib/versioning/packages'; @@ -60,21 +60,22 @@ jest.mock('ora', () => ({ })); let mockDir: MockDirectory; -jest.mock('@backstage/cli-common', () => ({ - ...jest.requireActual('@backstage/cli-common'), - findPaths: () => ({ - resolveTargetRoot(filename: string) { - return mockDir.resolve(filename); - }, - get targetDir() { - return mockDir.path; - }, - }), -})); - -jest.mock('../../../../lib/run', () => { +jest.mock('@backstage/cli-common', () => { + const actual = jest.requireActual('@backstage/cli-common'); return { - run: jest.fn(), + ...actual, + findPaths: () => ({ + resolveTargetRoot(filename: string) { + return mockDir.resolve(filename); + }, + get targetDir() { + return mockDir.path; + }, + }), + run: jest.fn().mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + }), }; }); @@ -184,7 +185,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -222,8 +226,7 @@ describe('bump', () => { expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -277,7 +280,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -318,8 +324,7 @@ describe('bump', () => { expect(mockFetchPackageInfo).toHaveBeenCalledWith('@backstage/theme'); expect(runObj.run).not.toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -373,7 +378,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -421,8 +429,7 @@ describe('bump', () => { expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -477,7 +484,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -526,14 +536,14 @@ describe('bump', () => { expect(mockFetchPackageInfo).toHaveBeenCalledWith('@backstage/core'); expect(runObj.run).toHaveBeenCalledTimes(2); - expect(runObj.run).toHaveBeenCalledWith('yarn', [ + expect(runObj.run).toHaveBeenCalledWith([ + 'yarn', 'plugin', 'import', 'https://versions.backstage.io/v1/releases/0.0.1/yarn-plugin', ]); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -587,7 +597,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/releases/999.0.1/manifest.json', @@ -656,7 +669,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -763,7 +779,10 @@ describe('bump', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -813,8 +832,7 @@ describe('bump', () => { expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -873,7 +891,10 @@ describe('bump', () => { }); mockFetchPackageInfo.mockRejectedValue(new NotFoundError('Nope')); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://versions.backstage.io/v1/tags/main/manifest.json', @@ -1094,7 +1115,10 @@ describe('environment variables', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://custom.example.com/v1/tags/main/manifest.json', @@ -1131,8 +1155,7 @@ describe('environment variables', () => { expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -1186,7 +1209,10 @@ describe('environment variables', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); const { log: logs } = await withLogCollector(['log', 'warn'], async () => { await bump({ pattern: null, release: 'main' } as unknown as Command); @@ -1215,8 +1241,7 @@ describe('environment variables', () => { expect(runObj.run).toHaveBeenCalledTimes(1); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); @@ -1253,7 +1278,10 @@ describe('environment variables', () => { }, }); - jest.spyOn(runObj, 'run').mockResolvedValue(undefined); + jest.spyOn(runObj, 'run').mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + } as any); worker.use( rest.get( 'https://custom.example.com/v1/tags/main/manifest.json', @@ -1291,14 +1319,14 @@ describe('environment variables', () => { ]); expect(runObj.run).toHaveBeenCalledTimes(2); - expect(runObj.run).toHaveBeenCalledWith('yarn', [ + expect(runObj.run).toHaveBeenCalledWith([ + 'yarn', 'plugin', 'import', 'https://custom.example.com/v1/releases/1.5.0/yarn-plugin', ]); expect(runObj.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + ['yarn', 'install'], expect.any(Object), ); }); diff --git a/packages/cli/src/modules/migrate/commands/versions/bump.ts b/packages/cli/src/modules/migrate/commands/versions/bump.ts index 19dcaae28f..cad477188b 100644 --- a/packages/cli/src/modules/migrate/commands/versions/bump.ts +++ b/packages/cli/src/modules/migrate/commands/versions/bump.ts @@ -41,7 +41,7 @@ import { } from '@backstage/release-manifests'; import { migrateMovedPackages } from './migrate'; import { runYarnInstall } from '../../lib/utils'; -import { run } from '../../../../lib/run'; +import { run } from '@backstage/cli-common'; const DEP_TYPES = [ 'dependencies', @@ -135,7 +135,7 @@ export default async (opts: OptionValues) => { ? `${env.BACKSTAGE_VERSIONS_BASE_URL}/v1/releases/${releaseManifest.releaseVersion}/yarn-plugin` : `https://versions.backstage.io/v1/releases/${releaseManifest.releaseVersion}/yarn-plugin`; - await run('yarn', ['plugin', 'import', yarnPluginUrl]); + await run(['yarn', 'plugin', 'import', yarnPluginUrl]).waitForExit(); console.log(); } diff --git a/packages/cli/src/modules/migrate/commands/versions/migrate.test.ts b/packages/cli/src/modules/migrate/commands/versions/migrate.test.ts index 9ff00fd285..936d10e24c 100644 --- a/packages/cli/src/modules/migrate/commands/versions/migrate.test.ts +++ b/packages/cli/src/modules/migrate/commands/versions/migrate.test.ts @@ -17,7 +17,7 @@ import { MockDirectory, createMockDirectory, } from '@backstage/backend-test-utils'; -import * as run from '../../../../lib/run'; +import * as runObj from '@backstage/cli-common'; import migrate from './migrate'; import { withLogCollector } from '@backstage/test-utils'; import fs from 'fs-extra'; @@ -33,21 +33,22 @@ jest.mock('chalk', () => ({ })); let mockDir: MockDirectory; -jest.mock('@backstage/cli-common', () => ({ - ...jest.requireActual('@backstage/cli-common'), - findPaths: () => ({ - resolveTargetRoot(filename: string) { - return mockDir.resolve(filename); - }, - get targetDir() { - return mockDir.path; - }, - }), -})); - -jest.mock('../../../../lib/run', () => { +jest.mock('@backstage/cli-common', () => { + const actual = jest.requireActual('@backstage/cli-common'); return { - run: jest.fn(), + ...actual, + findPaths: () => ({ + resolveTargetRoot(filename: string) { + return mockDir.resolve(filename); + }, + get targetDir() { + return mockDir.path; + }, + }), + run: jest.fn().mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + }), }; }); @@ -58,8 +59,15 @@ function expectLogsToMatch(receivedLogs: String[], expected: String[]): void { describe('versions:migrate', () => { mockDir = createMockDirectory(); + beforeEach(() => { + (runObj.run as jest.Mock).mockReturnValue({ + exitCode: null, + waitForExit: jest.fn().mockResolvedValue(undefined), + }); + }); + afterEach(() => { - jest.resetAllMocks(); + (runObj.run as jest.Mock).mockClear(); }); it('should bump to the moved version when the package is moved', async () => { @@ -116,8 +124,6 @@ describe('versions:migrate', () => { }, }); - jest.spyOn(run, 'run').mockResolvedValue(undefined); - const { warn, log: logs } = await withLogCollector(async () => { await migrate({}); }); @@ -136,10 +142,9 @@ describe('versions:migrate', () => { 'Could not find package.json for @backstage/theme@^1.0.0 in b (dependencies)', ]); - expect(run.run).toHaveBeenCalledTimes(1); - expect(run.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + expect(runObj.run).toHaveBeenCalledTimes(1); + expect(runObj.run).toHaveBeenCalledWith( + ['yarn', 'install'], expect.any(Object), ); @@ -227,16 +232,13 @@ describe('versions:migrate', () => { }, }); - jest.spyOn(run, 'run').mockResolvedValue(undefined); - await withLogCollector(async () => { await migrate({}); }); - expect(run.run).toHaveBeenCalledTimes(1); - expect(run.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + expect(runObj.run).toHaveBeenCalledTimes(1); + expect(runObj.run).toHaveBeenCalledWith( + ['yarn', 'install'], expect.any(Object), ); @@ -259,7 +261,7 @@ describe('versions:migrate', () => { ); }); - it('should replaces the occurrences of changed packages, and is careful', async () => { + it('should replace occurrences of changed packages, and is careful', async () => { mockDir.setContent({ 'package.json': JSON.stringify({ workspaces: { @@ -314,16 +316,13 @@ describe('versions:migrate', () => { }, }); - jest.spyOn(run, 'run').mockResolvedValue(undefined); - await withLogCollector(async () => { await migrate({}); }); - expect(run.run).toHaveBeenCalledTimes(1); - expect(run.run).toHaveBeenCalledWith( - 'yarn', - ['install'], + expect(runObj.run).toHaveBeenCalledTimes(1); + expect(runObj.run).toHaveBeenCalledWith( + ['yarn', 'install'], expect.any(Object), ); diff --git a/packages/cli/src/modules/migrate/lib/utils.ts b/packages/cli/src/modules/migrate/lib/utils.ts index 5b7b544c87..955cefde1d 100644 --- a/packages/cli/src/modules/migrate/lib/utils.ts +++ b/packages/cli/src/modules/migrate/lib/utils.ts @@ -16,7 +16,7 @@ import ora from 'ora'; import chalk from 'chalk'; -import { run } from '../../../lib/run'; +import { run } from '@backstage/cli-common'; export async function runYarnInstall() { const spinner = ora({ @@ -27,7 +27,7 @@ export async function runYarnInstall() { const installOutput = new Array(); try { - await run('yarn', ['install'], { + await run(['yarn', 'install'], { env: { FORCE_COLOR: 'true', // We filter out all of the npm_* environment variables that are added when @@ -41,7 +41,7 @@ export async function runYarnInstall() { }, stdoutLogFunc: data => installOutput.push(data), stderrLogFunc: data => installOutput.push(data), - }); + }).waitForExit(); spinner.succeed(); } catch (error) { spinner.fail(); diff --git a/packages/cli/src/modules/new/lib/tasks.ts b/packages/cli/src/modules/new/lib/tasks.ts index 2922f34f35..b0003e1053 100644 --- a/packages/cli/src/modules/new/lib/tasks.ts +++ b/packages/cli/src/modules/new/lib/tasks.ts @@ -16,11 +16,8 @@ import chalk from 'chalk'; import ora from 'ora'; -import { promisify } from 'util'; -import { exec as execCb } from 'child_process'; import { assertError } from '@backstage/errors'; - -const exec = promisify(execCb); +import { run } from '@backstage/cli-common'; const TASK_NAME_MAX_LENGTH = 14; @@ -71,16 +68,11 @@ export class Task { ) { try { await Task.forItem('executing', command, async () => { - await exec(command, { cwd: options?.cwd }); + const parts = command.trim().split(/\s+/); + await run(parts, { cwd: options?.cwd }).waitForExit(); }); } catch (error) { assertError(error); - if (error.stderr) { - process.stderr.write(error.stderr as Buffer); - } - if (error.stdout) { - process.stdout.write(error.stdout as Buffer); - } if (options?.optional) { Task.error(`Warning: Failed to execute command ${chalk.cyan(command)}`); } else { diff --git a/packages/cli/src/modules/test/commands/package/test.ts b/packages/cli/src/modules/test/commands/package/test.ts index e7ef0af302..1ecf5b395e 100644 --- a/packages/cli/src/modules/test/commands/package/test.ts +++ b/packages/cli/src/modules/test/commands/package/test.ts @@ -16,7 +16,7 @@ import { Command, OptionValues } from 'commander'; import { paths } from '../../../../lib/paths'; -import { runCheck } from '../../../../lib/run'; +import { runCheck } from '@backstage/cli-common'; function includesAnyOf(hayStack: string[], ...needles: string[]) { for (const needle of needles) { @@ -55,8 +55,8 @@ export default async (_opts: OptionValues, cmd: Command) => { !includesAnyOf(args, '--watch', '--watchAll') ) { const isGitRepo = () => - runCheck('git', 'rev-parse', '--is-inside-work-tree'); - const isMercurialRepo = () => runCheck('hg', '--cwd', '.', 'root'); + runCheck(['git', 'rev-parse', '--is-inside-work-tree']); + const isMercurialRepo = () => runCheck(['hg', '--cwd', '.', 'root']); if ((await isGitRepo()) || (await isMercurialRepo())) { args.push('--watch'); diff --git a/packages/cli/src/modules/test/commands/repo/test.ts b/packages/cli/src/modules/test/commands/repo/test.ts index f0520d29e2..fd18ba09fe 100644 --- a/packages/cli/src/modules/test/commands/repo/test.ts +++ b/packages/cli/src/modules/test/commands/repo/test.ts @@ -22,7 +22,7 @@ import { relative as relativePath } from 'path'; import { Command, OptionValues } from 'commander'; import { Lockfile, PackageGraph } from '@backstage/cli-node'; import { paths } from '../../../../lib/paths'; -import { runCheck, runPlain } from '../../../../lib/run'; +import { runCheck, runOutput } from '@backstage/cli-common'; import { isChildPath } from '@backstage/cli-common'; import { SuccessCache } from '../../../../lib/cache/SuccessCache'; @@ -63,14 +63,14 @@ async function readPackageTreeHashes(graph: PackageGraph) { ...pkg, path: relativePath(paths.targetRoot, pkg.dir), })); - const output = await runPlain( + const output = await runOutput([ 'git', 'ls-tree', - '--format="%(objectname)=%(path)"', + '--format=%(objectname)=%(path)', 'HEAD', '--', ...pkgs.map(pkg => pkg.path), - ); + ]); const map = new Map( output @@ -175,8 +175,8 @@ export async function command(opts: OptionValues, cmd: Command): Promise { !hasFlags('--coverage', '--watch', '--watchAll') ) { const isGitRepo = () => - runCheck('git', 'rev-parse', '--is-inside-work-tree'); - const isMercurialRepo = () => runCheck('hg', '--cwd', '.', 'root'); + runCheck(['git', 'rev-parse', '--is-inside-work-tree']); + const isMercurialRepo = () => runCheck(['hg', '--cwd', '.', 'root']); if ((await isGitRepo()) || (await isMercurialRepo())) { isSingleWatchMode = true;