Skip to content

Commit 46dde1f

Browse files
authored
chore(prlint): update second comment to state exemption request has been submitted (#29000)
### Issue # (if applicable) Closes #28803 ### Reason for this change If a contributor requests an exemption or clarification for a PR, they will get a second message (due to the comment event) telling them again that they can request an exemption/clarification. This may confuse contributors, especially beginners. ### Description of changes Detect if an exempt request comment already exists, next comment would mention an request has been submitted and waiting for a maintainer's review. ### Description of how you validated changes unit tests *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 97e3827 commit 46dde1f

File tree

2 files changed

+76
-3
lines changed

2 files changed

+76
-3
lines changed

tools/@aws-cdk/prlint/lint.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ export class PullRequestLinter {
250250
* @param existingReview The review created by a previous run of the linter.
251251
*/
252252
private async createOrUpdatePRLinterReview(failureMessages: string[], existingReview?: Review): Promise<void> {
253-
const body = `The pull request linter fails with the following errors:${this.formatErrors(failureMessages)}`
253+
let body = `The pull request linter fails with the following errors:${this.formatErrors(failureMessages)}`
254254
+ '<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n'
255255
+ 'If you would like to request an exemption from the status checks or clarification on feedback,'
256256
+ ' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.';
@@ -265,6 +265,10 @@ export class PullRequestLinter {
265265
});
266266
}
267267

268+
const comments = await this.client.issues.listComments();
269+
if (comments.data.find(comment => comment.body?.includes("Exemption Request"))) {
270+
body += '\n\n✅ A exemption request has been requested. Please wait for a maintainer\'s review.';
271+
}
268272
await this.client.issues.createComment({
269273
...this.issueParams,
270274
body,

tools/@aws-cdk/prlint/test/lint.test.ts

+71-2
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,71 @@ describe('integration tests required on features', () => {
10391039
});
10401040
});
10411041

1042+
describe('with existing Exemption Request comment', () => {
1043+
test('valid exemption request comment', async () => {
1044+
const issue: Subset<linter.GitHubPr> = {
1045+
number: 1,
1046+
title: 'fix: some title',
1047+
body: '',
1048+
labels: [{ name: 'pr-linter/exempt-test' }],
1049+
user: {
1050+
login: 'author',
1051+
},
1052+
};
1053+
const prLinter = configureMock(issue, undefined, ['Exemption Request']);
1054+
await expect(prLinter.validatePullRequestTarget(SHA)).rejects.toThrow(
1055+
'The pull request linter fails with the following errors:' +
1056+
'\n\n\t❌ Fixes must contain a change to an integration test file and the resulting snapshot.' +
1057+
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
1058+
'If you would like to request an exemption from the status checks or clarification on feedback,' +
1059+
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.' +
1060+
'\n\n✅ A exemption request has been requested. Please wait for a maintainer\'s review.',
1061+
);
1062+
});
1063+
1064+
test('valid exemption request with additional context', async () => {
1065+
const issue: Subset<linter.GitHubPr> = {
1066+
number: 1,
1067+
title: 'fix: some title',
1068+
body: '',
1069+
labels: [{ name: 'pr-linter/exempt-test' }],
1070+
user: {
1071+
login: 'author',
1072+
},
1073+
};
1074+
const prLinter = configureMock(issue, undefined, ['Exemption Request: \nThe reason is blah blah blah.']);
1075+
await expect(prLinter.validatePullRequestTarget(SHA)).rejects.toThrow(
1076+
'The pull request linter fails with the following errors:' +
1077+
'\n\n\t❌ Fixes must contain a change to an integration test file and the resulting snapshot.' +
1078+
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
1079+
'If you would like to request an exemption from the status checks or clarification on feedback,' +
1080+
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.' +
1081+
'\n\n✅ A exemption request has been requested. Please wait for a maintainer\'s review.',
1082+
);
1083+
});
1084+
1085+
test('valid exemption request with middle exemption request', async () => {
1086+
const issue: Subset<linter.GitHubPr> = {
1087+
number: 1,
1088+
title: 'fix: some title',
1089+
body: '',
1090+
labels: [{ name: 'pr-linter/exempt-test' }],
1091+
user: {
1092+
login: 'author',
1093+
},
1094+
};
1095+
const prLinter = configureMock(issue, undefined, ['Random content - Exemption Request - hello world']);
1096+
await expect(prLinter.validatePullRequestTarget(SHA)).rejects.toThrow(
1097+
'The pull request linter fails with the following errors:' +
1098+
'\n\n\t❌ Fixes must contain a change to an integration test file and the resulting snapshot.' +
1099+
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
1100+
'If you would like to request an exemption from the status checks or clarification on feedback,' +
1101+
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.' +
1102+
'\n\n✅ A exemption request has been requested. Please wait for a maintainer\'s review.',
1103+
);
1104+
});
1105+
});
1106+
10421107
describe('metadata file changed', () => {
10431108
const files: linter.GitHubFile[] = [{
10441109
filename: 'packages/aws-cdk-lib/region-info/build-tools/metadata.ts',
@@ -1074,7 +1139,7 @@ describe('integration tests required on features', () => {
10741139
});
10751140
});
10761141

1077-
function configureMock(pr: Subset<linter.GitHubPr>, prFiles?: linter.GitHubFile[]): linter.PullRequestLinter {
1142+
function configureMock(pr: Subset<linter.GitHubPr>, prFiles?: linter.GitHubFile[], existingComments?: string[]): linter.PullRequestLinter {
10781143
const pullsClient = {
10791144
get(_props: { _owner: string, _repo: string, _pull_number: number, _user: { _login: string} }) {
10801145
return { data: pr };
@@ -1103,7 +1168,11 @@ function configureMock(pr: Subset<linter.GitHubPr>, prFiles?: linter.GitHubFile[
11031168
deleteComment() {},
11041169

11051170
listComments() {
1106-
return { data: [{ id: 1212121212, user: { login: 'aws-cdk-automation' }, body: 'The pull request linter fails with the following errors:' }] };
1171+
const data = [{ id: 1212121212, user: { login: 'aws-cdk-automation' }, body: 'The pull request linter fails with the following errors:' }];
1172+
if (existingComments) {
1173+
existingComments.forEach(comment => data.push({ id: 1212121211, user: { login: 'aws-cdk-automation' }, body: comment }));
1174+
}
1175+
return { data };
11071176
},
11081177

11091178
removeLabel: mockRemoveLabel,

0 commit comments

Comments
 (0)