Skip to content

Commit 462881f

Browse files
author
copilot-swe-agent[bot]
authored
Fix rush change to enforce git email policy
1 parent dc885ee commit 462881f

4 files changed

Lines changed: 123 additions & 18 deletions

File tree

libraries/rush-lib/src/cli/actions/ChangeAction.ts

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import * as path from 'node:path';
55
import * as child_process from 'node:child_process';
66

7-
87
import type {
98
CommandLineFlagParameter,
109
CommandLineStringParameter,
@@ -30,6 +29,7 @@ import {
3029
import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer';
3130
import { Git } from '../../logic/Git';
3231
import { RushConstants } from '../../logic/RushConstants';
32+
import * as PolicyValidator from '../../logic/policy/PolicyValidator';
3333

3434
const BULK_LONG_NAME: string = '--bulk';
3535
const BULK_MESSAGE_LONG_NAME: string = '--message';
@@ -171,6 +171,15 @@ export class ChangeAction extends BaseRushAction {
171171
}
172172

173173
public async runAsync(): Promise<void> {
174+
await PolicyValidator.validatePolicyAsync(
175+
this.rushConfiguration,
176+
this.rushConfiguration.defaultSubspace,
177+
undefined,
178+
{
179+
allowShrinkwrapUpdates: true
180+
}
181+
);
182+
174183
if (this._verifyAllParameter.value) {
175184
const incompatibleParameters: (
176185
| CommandLineFlagParameter
@@ -319,10 +328,7 @@ export class ChangeAction extends BaseRushAction {
319328
this.terminal,
320329
await this._getChangeFilesSinceBaseBranchAsync()
321330
);
322-
changeFileData = await this._promptForChangeFileDataAsync(
323-
sortedProjectList,
324-
existingChangeComments
325-
);
331+
changeFileData = await this._promptForChangeFileDataAsync(sortedProjectList, existingChangeComments);
326332

327333
if (this._isEmailRequired(changeFileData)) {
328334
const email: string = this._changeEmailParameter.value
@@ -592,9 +598,7 @@ export class ChangeAction extends BaseRushAction {
592598
}
593599
}
594600

595-
private async _promptForCommentsAsync(
596-
packageName: string
597-
): Promise<IChangeInfo | undefined> {
601+
private async _promptForCommentsAsync(packageName: string): Promise<IChangeInfo | undefined> {
598602
const bumpOptions: { [type: string]: string } = this._getBumpOptions(packageName);
599603
const { default: input } = await import('@inquirer/input');
600604
const comment: string = await input({ message: `Describe changes, or ENTER if no changes:` });
@@ -680,10 +684,7 @@ export class ChangeAction extends BaseRushAction {
680684
* or will ask for it if it is not found or the Git config is wrong.
681685
*/
682686
private async _detectOrAskForEmailAsync(): Promise<string> {
683-
return (
684-
(await this._detectAndConfirmEmailAsync()) ||
685-
(await this._promptForEmailAsync())
686-
);
687+
return (await this._detectAndConfirmEmailAsync()) || (await this._promptForEmailAsync());
687688
}
688689

689690
private _detectEmail(): string | undefined {
@@ -780,9 +781,7 @@ export class ChangeAction extends BaseRushAction {
780781

781782
const fileExists: boolean = FileSystem.exists(filePath);
782783
const shouldWrite: boolean =
783-
!fileExists ||
784-
overwrite ||
785-
(interactiveMode ? await this._promptForOverwriteAsync(filePath) : false);
784+
!fileExists || overwrite || (interactiveMode ? await this._promptForOverwriteAsync(filePath) : false);
786785

787786
if (!interactiveMode && fileExists && !overwrite) {
788787
throw new Error(`Changefile ${filePath} already exists`);
@@ -794,9 +793,7 @@ export class ChangeAction extends BaseRushAction {
794793
}
795794
}
796795

797-
private async _promptForOverwriteAsync(
798-
filePath: string
799-
): Promise<boolean> {
796+
private async _promptForOverwriteAsync(filePath: string): Promise<boolean> {
800797
const { default: confirm } = await import('@inquirer/confirm');
801798
const overwrite: boolean = await confirm({
802799
message: `Overwrite ${filePath}?`
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import '../../test/mockRushCommandLineParser';
5+
6+
import { AlreadyReportedError, LockFile } from '@rushstack/node-core-library';
7+
8+
import { EnvironmentConfiguration } from '../../../api/EnvironmentConfiguration';
9+
import * as PolicyValidator from '../../../logic/policy/PolicyValidator';
10+
import { RushCommandLineParser } from '../../RushCommandLineParser';
11+
import { ChangeAction } from '../ChangeAction';
12+
13+
describe(ChangeAction.name, () => {
14+
let oldExitCode: number | string | undefined;
15+
let oldArgs: string[];
16+
17+
beforeEach(() => {
18+
jest.spyOn(process, 'exit').mockImplementation();
19+
20+
// Suppress "Another Rush command is already running" error
21+
jest.spyOn(LockFile, 'tryAcquire').mockImplementation(() => ({}) as LockFile);
22+
23+
oldExitCode = process.exitCode;
24+
oldArgs = process.argv;
25+
});
26+
27+
afterEach(() => {
28+
jest.clearAllMocks();
29+
process.exitCode = oldExitCode;
30+
process.argv = oldArgs;
31+
EnvironmentConfiguration.reset();
32+
});
33+
34+
it('runs policy validation before verifying change files', async () => {
35+
const startPath: string = `${__dirname}/changeRepo`;
36+
const parser: RushCommandLineParser = new RushCommandLineParser({ cwd: startPath });
37+
38+
const validatePolicySpy: jest.SpyInstance = jest
39+
.spyOn(PolicyValidator, 'validatePolicyAsync')
40+
.mockResolvedValue();
41+
const verifySpy: jest.SpyInstance = jest
42+
.spyOn(ChangeAction.prototype as unknown as { _verifyAsync: () => Promise<void> }, '_verifyAsync')
43+
.mockResolvedValue();
44+
45+
process.argv = [
46+
'pretend-this-is-node.exe',
47+
'pretend-this-is-rush',
48+
'change',
49+
'--verify',
50+
'--target-branch',
51+
'origin/main'
52+
];
53+
54+
await expect(parser.executeAsync()).resolves.toEqual(true);
55+
expect(validatePolicySpy).toHaveBeenCalledTimes(1);
56+
expect(validatePolicySpy).toHaveBeenCalledWith(
57+
parser.rushConfiguration,
58+
parser.rushConfiguration.defaultSubspace,
59+
undefined,
60+
{
61+
allowShrinkwrapUpdates: true
62+
}
63+
);
64+
expect(verifySpy).toHaveBeenCalledTimes(1);
65+
});
66+
67+
it('aborts rush change when policy validation fails', async () => {
68+
const startPath: string = `${__dirname}/changeRepo`;
69+
const parser: RushCommandLineParser = new RushCommandLineParser({ cwd: startPath });
70+
71+
const verifySpy: jest.SpyInstance = jest
72+
.spyOn(ChangeAction.prototype as unknown as { _verifyAsync: () => Promise<void> }, '_verifyAsync')
73+
.mockResolvedValue();
74+
jest.spyOn(PolicyValidator, 'validatePolicyAsync').mockRejectedValue(new AlreadyReportedError());
75+
76+
process.argv = [
77+
'pretend-this-is-node.exe',
78+
'pretend-this-is-rush',
79+
'change',
80+
'--verify',
81+
'--target-branch',
82+
'origin/main'
83+
];
84+
85+
await expect(parser.executeAsync()).resolves.toEqual(false);
86+
expect(verifySpy).not.toHaveBeenCalled();
87+
});
88+
});
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "a",
3+
"version": "1.0.0"
4+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"npmVersion": "6.4.1",
3+
"rushVersion": "5.5.2",
4+
"projectFolderMinDepth": 1,
5+
"projectFolderMaxDepth": 99,
6+
"gitPolicy": {
7+
"allowedEmailRegExps": ["[^@]+@users\\.noreply\\.github\\.com"],
8+
"sampleEmail": "example@users.noreply.github.com"
9+
},
10+
"projects": [
11+
{
12+
"packageName": "a",
13+
"projectFolder": "a"
14+
}
15+
]
16+
}

0 commit comments

Comments
 (0)