renovate/renovate
1
0
Fork 0
mirror of https://github.com/renovatebot/renovate.git synced 2025-01-10 14:06:30 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Merge add6ca5720 into 60754ce088

Browse source
This commit is contained in:
Felipe Santos 2025-01-02 18:34:12 +01:00 committed by GitHub
commit 3e24b3ff7f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 123 additions and 137 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

114
lib/modules/platform/gerrit/client.spec.ts
View file

@ -399,10 +399,20 @@ describe('modules/platform/gerrit/client', () => {
describe('approveChange()', () => {
it('already approved - do nothing', async () => {
const change = partial<GerritChange>({});
const change = partial<GerritChange>({
labels: {
'Code-Review': {
all: [{ value: 2 }],
},
},
});
httpMock
.scope(gerritEndpointUrl)
.get((url) => url.includes('/a/changes/123456?o='))
.get(
(url) =>
url.includes('/a/changes/123456?o=') &&
url.includes('o=DETAILED_LABELS'),
)
.reply(200, gerritRestResponse(change), jsonResultHeader);
await expect(client.approveChange(123456)).toResolve();
});
@ -411,17 +421,54 @@ describe('modules/platform/gerrit/client', () => {
const change = partial<GerritChange>({ labels: {} });
httpMock
.scope(gerritEndpointUrl)
.get((url) => url.includes('/a/changes/123456?o='))
.get(
(url) =>
url.includes('/a/changes/123456?o=') &&
url.includes('o=DETAILED_LABELS'),
)
.reply(200, gerritRestResponse(change), jsonResultHeader);
await expect(client.approveChange(123456)).toResolve();
});
it('not already approved - approve now', async () => {
const change = partial<GerritChange>({ labels: { 'Code-Review': {} } });
const change = partial<GerritChange>({
labels: { 'Code-Review': { all: [] } },
});
httpMock
.scope(gerritEndpointUrl)
.get((url) => url.includes('/a/changes/123456?o='))
.get(
(url) =>
url.includes('/a/changes/123456?o=') &&
url.includes('o=DETAILED_LABELS'),
)
.reply(200, gerritRestResponse(change), jsonResultHeader);
const approveMock = httpMock
.scope(gerritEndpointUrl)
.post('/a/changes/123456/revisions/current/review', {
labels: { 'Code-Review': +2 },
notify: 'NONE',
})
.reply(200, gerritRestResponse(''), jsonResultHeader);
await expect(client.approveChange(123456)).toResolve();
expect(approveMock.isDone()).toBeTrue();
});
it('not already approved because of +1 - approve now', async () => {
const change = partial<GerritChange>({
labels: {
'Code-Review': {
all: [{ value: 1 }],
},
},
});
httpMock
.scope(gerritEndpointUrl)
.get(
(url) =>
url.includes('/a/changes/123456?o=') &&
url.includes('o=DETAILED_LABELS'),
)
.reply(200, gerritRestResponse(change), jsonResultHeader);
const approveMock = httpMock
.scope(gerritEndpointUrl)
@ -434,63 +481,6 @@ describe('modules/platform/gerrit/client', () => {
expect(approveMock.isDone()).toBeTrue();
});
});
describe('wasApprovedBy()', () => {
it('label not exists', () => {
expect(
client.wasApprovedBy(partial<GerritChange>({}), 'user'),
).toBeUndefined();
});
it('not approved by anyone', () => {
expect(
client.wasApprovedBy(
partial<GerritChange>({
labels: {
'Code-Review': {},
},
}),
'user',
),
).toBeUndefined();
});
it('approved by given user', () => {
expect(
client.wasApprovedBy(
partial<GerritChange>({
labels: {
'Code-Review': {
approved: {
_account_id: 1,
username: 'user',
},
},
},
}),
'user',
),
).toBeTrue();
});
it('approved by given other', () => {
expect(
client.wasApprovedBy(
partial<GerritChange>({
labels: {
'Code-Review': {
approved: {
_account_id: 1,
username: 'other',
},
},
},
}),
'user',
),
).toBeFalse();
});
});
});
function gerritRestResponse(body: any): any {

23
lib/modules/platform/gerrit/client.ts
View file

@ -2,6 +2,7 @@ import { REPOSITORY_ARCHIVED } from '../../../constants/error-messages';
import { logger } from '../../../logger';
import { GerritHttp } from '../../../util/http/gerrit';
import { regEx } from '../../../util/regex';
import { getQueryString } from '../../../util/url';
import type {
GerritAccountInfo,
GerritBranchInfo,
@ -72,10 +73,14 @@ class GerritClient {
return changes.body;
}
async getChange(changeNumber: number): Promise<GerritChange> {
async getChange(
changeNumber: number,
extraOptions?: string[],
): Promise<GerritChange> {
const options = [...this.requestDetails, ...(extraOptions ?? [])];
const queryString = getQueryString({ o: options });
const changes = await this.gerritHttp.getJson<GerritChange>(
`a/changes/${changeNumber}?` +
this.requestDetails.map((det) => `o=${det}`).join('&'),
`a/changes/${changeNumber}?${queryString}`,
);
return changes.body;
}
@ -196,15 +201,11 @@ class GerritClient {
}
async checkIfApproved(changeId: number): Promise<boolean> {
const change = await client.getChange(changeId);
const reviewLabels = change?.labels?.['Code-Review'];
return reviewLabels === undefined || reviewLabels.approved !== undefined;
}
wasApprovedBy(change: GerritChange, username: string): boolean | undefined {
const change = await client.getChange(changeId, ['DETAILED_LABELS']);
const reviewLabel = change?.labels?.['Code-Review'];
return (
change.labels?.['Code-Review'].approved &&
change.labels['Code-Review'].approved.username === username
reviewLabel === undefined ||
reviewLabel.all?.some((label) => label.value === 2) === true
);
}

51
lib/modules/platform/gerrit/index.spec.ts
View file

@ -187,28 +187,6 @@ describe('modules/platform/gerrit/index', () => {
gerrit.writeToConfig({ labels: {} });
});
it('updatePr() - auto approve enabled', async () => {
const change = partial<GerritChange>({
current_revision: 'some-revision',
revisions: {
'some-revision': partial<GerritRevisionInfo>({
commit: {
message: 'some message',
},
}),
},
});
clientMock.getChange.mockResolvedValueOnce(change);
await gerrit.updatePr({
number: 123456,
prTitle: 'subject',
platformPrOptions: {
autoApprove: true,
},
});
expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
});
it('updatePr() - closed => abandon the change', async () => {
const change = partial<GerritChange>({});
clientMock.getChange.mockResolvedValueOnce(change);
@ -309,7 +287,7 @@ describe('modules/platform/gerrit/index', () => {
]);
});
it('createPr() - update body WITHOUT approve', async () => {
it('createPr() - update body', async () => {
const pr = await gerrit.createPr({
sourceBranch: 'source',
targetBranch: 'target',
@ -325,26 +303,6 @@ describe('modules/platform/gerrit/index', () => {
'body',
TAG_PULL_REQUEST_BODY,
);
expect(clientMock.approveChange).not.toHaveBeenCalled();
});
it('createPr() - update body and approve', async () => {
const pr = await gerrit.createPr({
sourceBranch: 'source',
targetBranch: 'target',
prTitle: change.subject,
prBody: 'body',
platformPrOptions: {
autoApprove: true,
},
});
expect(pr).toHaveProperty('number', 123456);
expect(clientMock.addMessageIfNotAlreadyExists).toHaveBeenCalledWith(
123456,
'body',
TAG_PULL_REQUEST_BODY,
);
expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
});
});
@ -750,6 +708,13 @@ describe('modules/platform/gerrit/index', () => {
//TODO: add some tests for Gerrit-specific replacements..
});
describe('approvePrForAutomerge()', () => {
it('approvePrForAutomerge() - calls approveChange', async () => {
await expect(gerrit.approvePrForAutomerge(123456)).toResolve();
expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
});
});
describe('currently unused/not-implemented functions', () => {
it('deleteLabel()', async () => {
await expect(

17
lib/modules/platform/gerrit/index.ts
View file

@ -160,9 +160,6 @@ export async function updatePr(prConfig: UpdatePrConfig): Promise<void> {
TAG_PULL_REQUEST_BODY,
);
}
if (prConfig.platformPrOptions?.autoApprove) {
await client.approveChange(prConfig.number);
}
if (prConfig.state && prConfig.state === 'closed') {
await client.abandonChange(prConfig.number);
}
@ -195,9 +192,6 @@ export async function createPr(prConfig: CreatePRConfig): Promise<Pr | null> {
prConfig.prBody,
TAG_PULL_REQUEST_BODY,
);
if (prConfig.platformPrOptions?.autoApprove) {
await client.approveChange(pr._number);
}
return getPr(pr._number);
}
@ -442,3 +436,14 @@ export function findIssue(title: string): Promise<Issue | null> {
export function getIssueList(): Promise<Issue[]> {
return Promise.resolve([]);
}
/**
* The Code-Review +2 vote of when the change was created or updated in Gerrit
* may have been downgraded by a CI check utilizing the same account as
* Renovate (e.g. SonarQube which posts Code-Review +1). This function will
* vote with +2 again on the change, if needed, before Renovate attempt to
* automerge it.
*/
export async function approvePrForAutomerge(number: number): Promise<void> {
await client.approveChange(number);
}

12
lib/modules/platform/gerrit/scm.spec.ts
View file

@ -369,7 +369,6 @@ describe('modules/platform/gerrit/scm', () => {
},
});
clientMock.findChanges.mockResolvedValueOnce([existingChange]);
clientMock.wasApprovedBy.mockReturnValueOnce(true);
git.prepareCommit.mockResolvedValueOnce({
commitSha: 'commitSha' as LongCommitSha,
parentCommitSha: 'parentSha' as LongCommitSha,
@ -385,6 +384,7 @@ describe('modules/platform/gerrit/scm', () => {
message: 'commit msg',
files: [],
prTitle: 'pr title',
autoApprove: true,
}),
).toBe('commitSha');
expect(git.prepareCommit).toHaveBeenCalledWith({
@ -396,19 +396,15 @@ describe('modules/platform/gerrit/scm', () => {
'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
],
prTitle: 'pr title',
autoApprove: true,
force: true,
});
expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
expect(git.pushCommit).toHaveBeenCalledWith({
files: [],
sourceRef: 'renovate/dependency-1.x',
targetRef: 'refs/for/main%notify=NONE',
});
expect(clientMock.wasApprovedBy).toHaveBeenCalledWith(
existingChange,
'user',
);
expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
targetRef: 'refs/for/main%notify=NONE,l=Code-Review+2',
});
});
});
});

13
lib/modules/platform/gerrit/scm.ts
View file

@ -129,19 +129,16 @@ export class GerritScm extends DefaultGitScm {
hasChanges = await git.hasDiff('HEAD', 'FETCH_HEAD'); //avoid empty patchsets
}
if (hasChanges || commit.force) {
const pushOptions = ['notify=NONE'];
if (commit.autoApprove) {
pushOptions.push('l=Code-Review+2');
}
const pushResult = await git.pushCommit({
sourceRef: commit.branchName,
targetRef: `refs/for/${commit.baseBranch!}%notify=NONE`,
targetRef: `refs/for/${commit.baseBranch!}%${pushOptions.join(',')}`,
files: commit.files,
});
if (pushResult) {
//existingChange was the old change before commit/push. we need to approve again, if it was previously approved from renovate
if (
existingChange &&
client.wasApprovedBy(existingChange, username)
) {
await client.approveChange(existingChange._number);
}
return commitSha;
}
}

7
lib/modules/platform/gerrit/types.ts
View file

@ -79,6 +79,8 @@ export interface GerritLabelInfo {
rejected?: GerritAccountInfo;
/** If true, the label blocks submit operation. If not set, the default is false. */
blocking?: boolean;
/** List of votes. Only set when o=DETAILED_LABELS. */
all?: GerritApprovalInfo[];
}
export interface GerritActionInfo {
@ -101,3 +103,8 @@ export interface GerritMergeableInfo {
| 'CHERRY_PICK';
mergeable: boolean;
}
export interface GerritApprovalInfo {
value?: number;
username?: string;
}

5
lib/modules/platform/types.ts
View file

@ -283,6 +283,11 @@ export interface Platform {
maxBodyLength(): number;
labelCharLimit?(): number;
/**
* Platforms that support `autoApprove` can implement this function. It will be
* invoked during automerge before the PR branch status is checked.
*/
approvePrForAutomerge?(number: number): Promise<void>;
}
export interface PlatformScm {

2
lib/util/git/types.ts
View file

@ -86,6 +86,8 @@ export interface CommitFilesConfig {
platformCommit?: PlatformCommitOptions;
/** Only needed by Gerrit platform */
prTitle?: string;
/** Only needed by Gerrit platform */
autoApprove?: boolean;
}
export interface PushFilesConfig {

2
lib/workers/repository/update/branch/commit.ts
View file

@ -62,5 +62,7 @@ export function commitFilesToBranch(
platformCommit: config.platformCommit,
// Only needed by Gerrit platform
prTitle: config.prTitle,
// Only needed by Gerrit platform
autoApprove: config.autoApprove,
});
}

12
lib/workers/repository/update/pr/automerge.spec.ts
View file

@ -124,6 +124,18 @@ describe('workers/repository/update/pr/automerge', () => {
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should attempt to auto-approve if supported', async () => {
config.autoApprove = true;
config.automerge = true;
config.pruneBranchAfterAutomerge = true;
platform.getBranchStatus.mockResolvedValueOnce('green');
platform.mergePr.mockResolvedValueOnce(true);
const res = await prAutomerge.checkAutoMerge(pr, config);
expect(res).toEqual({ automerged: true, branchRemoved: true });
expect(platform.approvePrForAutomerge).toHaveBeenCalledTimes(1);
expect(platform.mergePr).toHaveBeenCalledTimes(1);
});
it('should not automerge if enabled and pr is unmergeable', async () => {
config.automerge = true;
scm.isBranchConflicted.mockResolvedValueOnce(true);

4
lib/workers/repository/update/pr/automerge.ts
View file

@ -69,6 +69,10 @@ export async function checkAutoMerge(
prAutomergeBlockReason: 'PlatformNotReady',
};
}
if (config.autoApprove && platform.approvePrForAutomerge) {
logger.debug('Auto-approving PR for automerge');
await platform.approvePrForAutomerge(pr.number);
}
const branchStatus = await resolveBranchStatus(
branchName,
!!config.internalChecksAsSuccess,