Skip to content

Commit 999febf

Browse files
CLOUDP-352308 add logging for spawn processes in link tools (#3310)
* feat: enable logging for spawn processes * docs: add changelog * refactor: as co-pilot suggested test: add unit test
1 parent 587f14e commit 999febf

File tree

10 files changed

+412
-93
lines changed

10 files changed

+412
-93
lines changed

.changeset/tidy-dragons-wait.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@lg-tools/link': minor
3+
'@lg-tools/cli': minor
4+
---
5+
6+
developer experience slightly improves by adding more detailed logging to spawn processes in the link script. Previously, running link script would lead to a group of processes being spawned in parallel all piping their stdout and stderr to the console in verbose mode which made it difficult to distinguish what each line of output was from which process. The command and working directory are also now logged for each process along with their exit code.

tools/link/src/utils/createLinkFrom.spec.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import { ChildProcess } from 'child_process';
2-
import xSpawn from 'cross-spawn';
31
import fsx from 'fs-extra';
42
import path from 'path';
53

64
import { createLinkFrom } from './createLinkFrom';
7-
import { MockChildProcess } from './mocks.testutils';
5+
import * as spawnLoggedModule from './spawnLogged';
86

97
describe('tools/link/createLinkFrom', () => {
10-
let spawnSpy: jest.SpyInstance<ChildProcess>;
8+
let spawnLoggedSpy: jest.SpyInstance;
119

1210
beforeAll(() => {
1311
fsx.emptyDirSync('./tmp');
@@ -18,30 +16,32 @@ describe('tools/link/createLinkFrom', () => {
1816
});
1917

2018
beforeEach(() => {
21-
spawnSpy = jest.spyOn(xSpawn, 'spawn');
22-
spawnSpy.mockImplementation((..._args) => new MockChildProcess());
19+
spawnLoggedSpy = jest
20+
.spyOn(spawnLoggedModule, 'spawnLogged')
21+
.mockResolvedValue(undefined);
2322
});
2423

2524
afterEach(() => {
26-
spawnSpy.mockRestore();
25+
spawnLoggedSpy.mockRestore();
2726
fsx.emptyDirSync('./tmp');
2827
});
2928

3029
afterAll(() => {
31-
fsx.rmdirSync('./tmp/');
30+
fsx.removeSync('./tmp/');
3231
});
3332

34-
test('calls `npm link` command from package directory', () => {
35-
createLinkFrom(path.resolve('./tmp/'), {
33+
test('calls `npm link` command from package directory', async () => {
34+
await createLinkFrom(path.resolve('./tmp/'), {
3635
scopeName: '@example',
3736
scopePath: 'scope',
3837
packageName: 'test-package',
3938
});
4039

41-
expect(spawnSpy).toHaveBeenCalledWith(
40+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
4241
'npm',
4342
['link'],
4443
expect.objectContaining({
44+
name: 'link_src:test-package',
4545
cwd: expect.stringContaining('tmp/scope/test-package'),
4646
}),
4747
);
Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/* eslint-disable no-console */
22
import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta';
33
import chalk from 'chalk';
4-
import { spawn } from 'cross-spawn';
54
import path from 'path';
65

76
import { findDirectory } from './findDirectory';
7+
import { spawnLogged } from './spawnLogged';
88
import { PackageDetails } from './types';
99

1010
interface CreateLinkOptions extends PackageDetails {
@@ -16,7 +16,7 @@ interface CreateLinkOptions extends PackageDetails {
1616
* Runs the pnpm link command in a leafygreen-ui package directory
1717
* @returns Promise that resolves when the pnpm link command has finished
1818
*/
19-
export function createLinkFrom(
19+
export async function createLinkFrom(
2020
source: string = process.cwd(),
2121
{
2222
scopeName,
@@ -27,29 +27,30 @@ export function createLinkFrom(
2727
}: CreateLinkOptions,
2828
): Promise<void> {
2929
const scopeSrc = scopePath;
30-
return new Promise<void>(resolve => {
31-
const packagesDirectory = findDirectory(source, scopeSrc);
32-
packageManager = packageManager ?? getPackageManager(source);
30+
const packagesDirectory = findDirectory(source, scopeSrc);
3331

34-
if (packagesDirectory) {
35-
verbose &&
36-
console.log(
37-
'Creating link for:',
38-
chalk.green(`${scopeName}/${packageName}`),
39-
);
32+
if (packagesDirectory) {
33+
verbose &&
34+
console.log(
35+
'Creating link for:',
36+
chalk.green(`${scopeName}/${packageName}`),
37+
);
38+
39+
const resolvedPackageManager =
40+
packageManager ?? getPackageManager(packagesDirectory);
4041

41-
spawn(packageManager, ['link'], {
42+
try {
43+
await spawnLogged(resolvedPackageManager, ['link'], {
44+
name: `link_src:${packageName}`,
4245
cwd: path.join(packagesDirectory, packageName),
43-
stdio: verbose ? 'inherit' : 'ignore',
44-
})
45-
.on('close', resolve)
46-
.on('error', () => {
47-
throw new Error(`Couldn't create link for package: ${packageName}`);
48-
});
49-
} else {
50-
throw new Error(
51-
`Can't find a ${scopeSrc} directory in ${process.cwd()} or any of its parent directories.`,
52-
);
46+
verbose,
47+
});
48+
} catch (_) {
49+
throw new Error(`Couldn't create link for package: ${packageName}`);
5350
}
54-
});
51+
} else {
52+
throw new Error(
53+
`Can't find a ${scopeSrc} directory in ${process.cwd()} or any of its parent directories.`,
54+
);
55+
}
5556
}
Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,23 @@
1-
import { ChildProcess } from 'child_process';
2-
import xSpawn from 'cross-spawn';
31
import fsx from 'fs-extra';
42

53
import { installPackages } from './install';
6-
import { MockChildProcess } from './mocks.testutils';
4+
import * as spawnLoggedModule from './spawnLogged';
75

86
describe('tools/link/utils/install', () => {
9-
let spawnSpy: jest.SpyInstance<ChildProcess>;
7+
let spawnLoggedSpy: jest.SpyInstance;
108

119
beforeAll(() => {
1210
fsx.ensureDirSync('./tmp/');
1311
});
1412

1513
beforeEach(() => {
16-
spawnSpy = jest.spyOn(xSpawn, 'spawn');
17-
spawnSpy.mockImplementation((..._args) => new MockChildProcess());
14+
spawnLoggedSpy = jest
15+
.spyOn(spawnLoggedModule, 'spawnLogged')
16+
.mockResolvedValue(undefined);
1817
});
1918

2019
afterEach(() => {
21-
spawnSpy.mockRestore();
20+
spawnLoggedSpy.mockRestore();
2221
fsx.emptyDirSync('./tmp');
2322
});
2423

@@ -28,20 +27,24 @@ describe('tools/link/utils/install', () => {
2827

2928
test('runs `npm install` command', async () => {
3029
await installPackages('./tmp');
31-
expect(spawnSpy).toHaveBeenCalledWith(
30+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
3231
'npm',
3332
['install'],
34-
expect.objectContaining({}),
33+
expect.objectContaining({
34+
cwd: './tmp',
35+
}),
3536
);
3637
});
3738

3839
test('runs install command using local package manager', async () => {
3940
fsx.createFileSync('./tmp/pnpm-lock.yaml');
4041
await installPackages('./tmp');
41-
expect(spawnSpy).toHaveBeenCalledWith(
42+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
4243
'pnpm',
4344
['install'],
44-
expect.objectContaining({}),
45+
expect.objectContaining({
46+
cwd: './tmp',
47+
}),
4548
);
4649
});
4750
});

tools/link/src/utils/install.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,28 @@
11
import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta';
2-
import { spawn } from 'cross-spawn';
32
import fsx from 'fs-extra';
43

4+
import { spawnLogged } from './spawnLogged';
5+
56
export async function installPackages(
67
path: string,
78
options?: {
89
packageManager?: SupportedPackageManager;
910
verbose?: boolean;
1011
},
1112
): Promise<void> {
12-
return new Promise<void>((resolve, reject) => {
13-
if (fsx.existsSync(path)) {
14-
const pkgMgr = options?.packageManager ?? getPackageManager(path);
13+
if (fsx.existsSync(path)) {
14+
const pkgMgr = options?.packageManager ?? getPackageManager(path);
1515

16-
spawn(pkgMgr, ['install'], {
16+
try {
17+
await spawnLogged(pkgMgr, ['install'], {
18+
name: 'install',
1719
cwd: path,
18-
stdio: options?.verbose ? 'inherit' : 'ignore',
19-
})
20-
.on('close', resolve)
21-
.on('error', err => {
22-
throw new Error(`Error installing packages\n` + err);
23-
});
24-
} else {
25-
console.error(`Path ${path} does not exist`);
26-
reject();
20+
verbose: options?.verbose,
21+
});
22+
} catch (err) {
23+
throw new Error(`Error installing packages\n` + err);
2724
}
28-
});
25+
} else {
26+
throw new Error(`Path ${path} does not exist`);
27+
}
2928
}

tools/link/src/utils/linkPackageTo.spec.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
1-
import { ChildProcess } from 'child_process';
2-
import xSpawn from 'cross-spawn';
31
import fsx from 'fs-extra';
42
import path from 'path';
53

64
import { linkPackageTo } from './linkPackageTo';
7-
import { MockChildProcess } from './mocks.testutils';
5+
import * as spawnLoggedModule from './spawnLogged';
86

97
describe('tools/link/linkPackageTo', () => {
10-
let spawnSpy: jest.SpyInstance<ChildProcess>;
8+
let spawnLoggedSpy: jest.SpyInstance;
119

1210
beforeAll(() => {
1311
fsx.ensureDirSync('./tmp/app');
1412
fsx.emptyDirSync('./tmp/app');
1513
});
1614

1715
beforeEach(() => {
18-
spawnSpy = jest.spyOn(xSpawn, 'spawn');
19-
spawnSpy.mockImplementation((..._args) => new MockChildProcess());
16+
spawnLoggedSpy = jest
17+
.spyOn(spawnLoggedModule, 'spawnLogged')
18+
.mockResolvedValue(undefined);
2019
});
2120

2221
afterEach(() => {
23-
spawnSpy.mockRestore();
22+
spawnLoggedSpy.mockRestore();
2423
fsx.emptyDirSync('./tmp');
2524
});
2625

2726
afterAll(() => {
2827
fsx.rmdirSync('./tmp/');
2928
});
3029

31-
test('calls `npm link <package>` from the destination directory', () => {
32-
linkPackageTo(path.resolve('./tmp/app'), {
30+
test('calls `npm link <package>` from the destination directory', async () => {
31+
await linkPackageTo(path.resolve('./tmp/app'), {
3332
scopeName: '@example',
3433
packageName: 'test-package',
3534
});
3635

37-
expect(spawnSpy).toHaveBeenCalledWith(
36+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
3837
'npm',
3938
['link', '@example/test-package'],
4039
expect.objectContaining({
40+
name: 'link_dst:test-package',
4141
cwd: expect.stringContaining('tmp/app'),
4242
}),
4343
);
Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getPackageManager, SupportedPackageManager } from '@lg-tools/meta';
22
import chalk from 'chalk';
3-
import { spawn } from 'cross-spawn';
43

4+
import { spawnLogged } from './spawnLogged';
55
import { PackageDetails } from './types';
66

77
interface LinkPackagesToOptions
@@ -14,23 +14,24 @@ interface LinkPackagesToOptions
1414
* Runs the pnpm link <packageName> command in the destination directory
1515
* @returns Promise that resolves when the pnpm link <packageName> command has finished
1616
*/
17-
export function linkPackageTo(
17+
export async function linkPackageTo(
1818
destination: string,
1919
{ scopeName, packageName, verbose, packageManager }: LinkPackagesToOptions,
2020
): Promise<void> {
2121
const fullPackageName = `${scopeName}/${packageName}`;
22-
return new Promise<void>(resolve => {
23-
// eslint-disable-next-line no-console
24-
verbose && console.log('Linking package:', chalk.blue(fullPackageName));
25-
packageManager = packageManager ?? getPackageManager(destination);
22+
// eslint-disable-next-line no-console
23+
verbose && console.log('Linking package:', chalk.blue(fullPackageName));
2624

27-
spawn(packageManager, ['link', fullPackageName], {
25+
const resolvedPackageManager =
26+
packageManager ?? getPackageManager(destination);
27+
28+
try {
29+
await spawnLogged(resolvedPackageManager, ['link', fullPackageName], {
30+
name: `link_dst:${packageName}`,
2831
cwd: destination,
29-
stdio: verbose ? 'inherit' : 'ignore',
30-
})
31-
.on('close', resolve)
32-
.on('error', () => {
33-
throw new Error(`Couldn't link package: ${fullPackageName}`);
34-
});
35-
});
32+
verbose,
33+
});
34+
} catch (_) {
35+
throw new Error(`Couldn't link package: ${fullPackageName}`);
36+
}
3637
}

tools/link/src/utils/linkPackagesForScope.spec.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import { ChildProcess } from 'child_process';
2-
import xSpawn from 'cross-spawn';
31
import fsx from 'fs-extra';
42
import path from 'path';
53

64
import { linkPackagesForScope } from './linkPackagesForScope';
7-
import { MockChildProcess } from './mocks.testutils';
5+
import * as spawnLoggedModule from './spawnLogged';
86

97
describe('tools/link/linkPackagesForScope', () => {
10-
let spawnSpy: jest.SpyInstance<ChildProcess>;
8+
let spawnLoggedSpy: jest.SpyInstance;
119

1210
beforeAll(() => {
1311
fsx.emptyDirSync('./tmp');
@@ -16,12 +14,13 @@ describe('tools/link/linkPackagesForScope', () => {
1614
});
1715

1816
beforeEach(() => {
19-
spawnSpy = jest.spyOn(xSpawn, 'spawn');
20-
spawnSpy.mockImplementation((..._args) => new MockChildProcess());
17+
spawnLoggedSpy = jest
18+
.spyOn(spawnLoggedModule, 'spawnLogged')
19+
.mockResolvedValue(undefined);
2120
});
2221

2322
afterEach(() => {
24-
spawnSpy.mockRestore();
23+
spawnLoggedSpy.mockRestore();
2524
fsx.emptyDirSync('./tmp');
2625
});
2726

@@ -48,19 +47,21 @@ describe('tools/link/linkPackagesForScope', () => {
4847
);
4948

5049
// Creates links
51-
expect(spawnSpy).toHaveBeenCalledWith(
50+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
5251
'npm',
5352
['link'],
5453
expect.objectContaining({
54+
name: 'link_src:test-package',
5555
cwd: expect.stringContaining('tmp/packages/scope/test-package'),
5656
}),
5757
);
5858

5959
// Consumes links
60-
expect(spawnSpy).toHaveBeenCalledWith(
60+
expect(spawnLoggedSpy).toHaveBeenCalledWith(
6161
'npm',
6262
['link', '@example/test-package'],
6363
expect.objectContaining({
64+
name: 'link_dst:test-package',
6465
cwd: expect.stringContaining('tmp/app'),
6566
}),
6667
);

0 commit comments

Comments
 (0)