Skip to content

Commit

Permalink
fix(gerrit): commitBody option not being effective (#28774)
Browse files Browse the repository at this point in the history
  • Loading branch information
felipecrs committed May 2, 2024
1 parent 1ee176d commit 9841fa0
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 26 deletions.
17 changes: 12 additions & 5 deletions lib/modules/platform/gerrit/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('modules/platform/gerrit/client', () => {
httpMock
.scope(gerritEndpointUrl)
.get(
'/a/changes/123456?o=SUBMITTABLE&o=CHECK&o=MESSAGES&o=DETAILED_ACCOUNTS&o=LABELS&o=CURRENT_ACTIONS&o=CURRENT_REVISION',
'/a/changes/123456?o=SUBMITTABLE&o=CHECK&o=MESSAGES&o=DETAILED_ACCOUNTS&o=LABELS&o=CURRENT_ACTIONS&o=CURRENT_REVISION&o=CURRENT_COMMIT',
)
.reply(200, gerritRestResponse(change), jsonResultHeader);
await expect(client.getChange(123456)).resolves.toEqual(change);
Expand Down Expand Up @@ -215,17 +215,24 @@ describe('modules/platform/gerrit/client', () => {
});
});

describe('updateCommitMessage', () => {
it('updateCommitMessage - success', async () => {
describe('updateChangeSubject', () => {
it('updateChangeSubject - success', async () => {
const change = partial<GerritChange>({});
const newSubject = 'new subject';
const previousSubject = 'some subject';
const previousBody = `some body\n\nChange-Id: some-change-id\n`;
httpMock
.scope(gerritEndpointUrl)
.put('/a/changes/123456/message', {
message: `new message\n\nChange-Id: changeID\n`,
message: `${newSubject}\n\n${previousBody}`,
})
.reply(200, gerritRestResponse(change), jsonResultHeader);
await expect(
client.updateCommitMessage(123456, 'changeID', 'new message'),
client.updateChangeSubject(
123456,
`${previousSubject}\n\n${previousBody}`,
'new subject',
),
).toResolve();
});
});
Expand Down
15 changes: 9 additions & 6 deletions lib/modules/platform/gerrit/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class GerritClient {
'LABELS',
'CURRENT_ACTIONS', //to check if current_revision can be "rebased"
'CURRENT_REVISION', //get RevisionInfo::ref to fetch
'CURRENT_COMMIT', // to get the commit message
] as const;

private gerritHttp = new GerritHttp();
Expand Down Expand Up @@ -103,15 +104,17 @@ class GerritClient {
});
}

async updateCommitMessage(
async updateChangeSubject(
number: number,
gerritChangeID: string,
prTitle: string,
currentMessage: string,
newSubject: string,
): Promise<void> {
await this.setCommitMessage(
number,
`${prTitle}\n\nChange-Id: ${gerritChangeID}\n`,
// Replace first line of the commit message with the new subject
const newMessage = currentMessage.replace(
new RegExp(`^.*$`, 'm'),
newSubject,
);
await this.setCommitMessage(number, newMessage);
}

async getMessages(changeNumber: number): Promise<GerritChangeMessageInfo[]> {
Expand Down
55 changes: 46 additions & 9 deletions lib/modules/platform/gerrit/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
GerritLabelInfo,
GerritLabelTypeInfo,
GerritProjectInfo,
GerritRevisionInfo,
} from './types';
import { TAG_PULL_REQUEST_BODY, mapGerritChangeToPr } from './utils';
import { writeToConfig } from '.';
Expand Down Expand Up @@ -187,21 +188,39 @@ describe('modules/platform/gerrit/index', () => {
});

it('updatePr() - new prTitle => copy to commit msg', async () => {
const oldSubject = 'old title';
const oldMessage = `${oldSubject}\n\nsome body\n\nChange-Id: ...`;
const change = partial<GerritChange>({
change_id: '...',
subject: 'old title',
subject: oldSubject,
current_revision: 'some-revision',
revisions: {
'some-revision': partial<GerritRevisionInfo>({
commit: {
message: oldMessage,
},
}),
},
});
clientMock.getChange.mockResolvedValueOnce(change);
await gerrit.updatePr({ number: 123456, prTitle: 'new title' });
expect(clientMock.updateCommitMessage).toHaveBeenCalledWith(
expect(clientMock.updateChangeSubject).toHaveBeenCalledWith(
123456,
'...',
oldMessage,
'new title',
);
});

it('updatePr() - auto approve enabled', async () => {
const change = partial<GerritChange>({});
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,
Expand All @@ -225,7 +244,16 @@ describe('modules/platform/gerrit/index', () => {
});

it('updatePr() - existing prBody found in change.messages => nothing todo...', async () => {
const change = partial<GerritChange>({});
const change = partial<GerritChange>({
current_revision: 'some-revision',
revisions: {
'some-revision': partial<GerritRevisionInfo>({
commit: {
message: 'some message',
},
}),
},
});
clientMock.getChange.mockResolvedValueOnce(change);
clientMock.getMessages.mockResolvedValueOnce([
partial<GerritChangeMessageInfo>({
Expand Down Expand Up @@ -279,9 +307,18 @@ describe('modules/platform/gerrit/index', () => {
gerrit.writeToConfig({ labels: {} });
});

const message = 'some subject\n\nsome body\n\nChange-Id: some-change-id';

const change = partial<GerritChange>({
_number: 123456,
change_id: '...',
current_revision: 'some-revision',
revisions: {
'some-revision': partial<GerritRevisionInfo>({
commit: {
message,
},
}),
},
});

beforeEach(() => {
Expand Down Expand Up @@ -312,9 +349,9 @@ describe('modules/platform/gerrit/index', () => {
TAG_PULL_REQUEST_BODY,
);
expect(clientMock.approveChange).not.toHaveBeenCalled();
expect(clientMock.updateCommitMessage).toHaveBeenCalledWith(
expect(clientMock.updateChangeSubject).toHaveBeenCalledWith(
123456,
'...',
message,
'title',
);
});
Expand Down
8 changes: 4 additions & 4 deletions lib/modules/platform/gerrit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ export async function updatePr(prConfig: UpdatePrConfig): Promise<void> {
logger.debug(`updatePr(${prConfig.number}, ${prConfig.prTitle})`);
const change = await client.getChange(prConfig.number);
if (change.subject !== prConfig.prTitle) {
await client.updateCommitMessage(
await client.updateChangeSubject(
prConfig.number,
change.change_id,
change.revisions[change.current_revision].commit.message,
prConfig.prTitle,
);
}
Expand Down Expand Up @@ -200,9 +200,9 @@ export async function createPr(prConfig: CreatePRConfig): Promise<Pr | null> {
}
//Workaround for "Known Problems.1"
if (pr.subject !== prConfig.prTitle) {
await client.updateCommitMessage(
await client.updateChangeSubject(
pr._number,
pr.change_id,
pr.revisions[pr.current_revision].commit.message,
prConfig.prTitle,
);
}
Expand Down
9 changes: 7 additions & 2 deletions lib/modules/platform/gerrit/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,26 @@ export interface GerritChange {
labels?: Record<string, GerritLabelInfo>;
reviewers?: Record<GerritReviewersType, GerritAccountInfo[]>;
messages?: GerritChangeMessageInfo[];
current_revision?: string;
current_revision: string;
/**
* All patch sets of this change as a map that maps the commit ID of the patch set to a RevisionInfo entity.
*/
revisions?: Record<string, GerritRevisionInfo>;
revisions: Record<string, GerritRevisionInfo>;
problems: unknown[];
}

export interface GerritCommitInfo {
message: string;
}

export interface GerritRevisionInfo {
uploader: GerritAccountInfo;
/**
* The Git reference for the patch set.
*/
ref: string;
actions?: Record<string, GerritActionInfo>;
commit: GerritCommitInfo;
}

export interface GerritChangeMessageInfo {
Expand Down

0 comments on commit 9841fa0

Please sign in to comment.