Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions app_dart/docs/cicd_flowchart.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# CICD Label Flowchart

This flowchart describes the logic for managing the `CICD` label in Cocoon for running presubmit CI.

```mermaid
flowchart TD
PR_Opened([Event: PR Opened]) --> Is_Draft{Is Draft?}

Is_Draft -- No --> Is_Privileged_Open{Is Privileged?}
Is_Draft -- Yes --> Create_Awaiting[Create Awaiting Check Run]

Is_Privileged_Open -- Yes --> Add_Label[Add CICD Label]
Add_Label --> Start_Pre[Start Presubmits]

Is_Privileged_Open -- No --> Create_Awaiting

Create_Awaiting --> State_Awaiting((State: Awaiting))
Start_Pre --> State_Running((State: Running))

State_Awaiting --> Label_Added([Event: CICD Label Added])
Label_Added --> Resolve_Awaiting[Resolve Awaiting Check Run]
Resolve_Awaiting --> Start_Pre

State_Running --> Changes_Pushed([Event: Changes Pushed])
Changes_Pushed --> Is_Privileged_Push{Is Privileged?}
Remove_Label --> Create_Awaiting

Is_Privileged_Push -- No --> Remove_Label[Remove CICD Label]

Is_Privileged_Push -- Yes --> Label_Present{Has CICD Label?}
Label_Present -- Yes --> Start_Pre
Label_Present -- No --> Create_Awaiting
```
69 changes: 44 additions & 25 deletions app_dart/lib/src/request_handlers/github/webhook_subscription.dart
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
/// retried with exponential backoff within that time period. The GoB mirror
/// should be caught up within that time frame via either the internal
/// mirroring service or [VacuumGithubCommits].
///
/// See docs/cicd_flowchart.md for the flowchart of CICD label management.
Future<Response> _handlePullRequest(String rawRequest) async {
final pullRequestEvent = _getPullRequestEvent(rawRequest);
if (pullRequestEvent == null) {
Expand All @@ -199,16 +201,20 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
final result = await _processPullRequestClosed(pullRequestEvent);
return result.toResponse();
case 'edited':
if (pullRequestEvent.changes != null &&
pullRequestEvent.changes!.base != null) {
await _addCICDForRollersAndMembers(pullRequestEvent);
}
await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
await _checkForTests(pullRequestEvent);
break;
case 'opened':
await _addCICDForRollersAndMembers(pullRequestEvent);
await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
final isPrivileged = await _isPrivilegedUser(pr, slug);
if (!pr.draft! && isPrivileged) {
final gitHubClient = await config.createGitHubClient(pullRequest: pr);
await gitHubClient.issues.addLabelsToIssue(slug, pr.number!, [
'CICD',
]);
log.debug('Added CICD label to PR ${pr.number}');
await _scheduleIfMergeable(pullRequestEvent);
} else {
await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
}
await _checkForTests(pullRequestEvent);
await _tryReleaseApproval(pullRequestEvent);
break;
Expand Down Expand Up @@ -242,7 +248,31 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
await _respondToPullRequestDequeued(pullRequestEvent);
break;
case 'synchronize':
await _addCICDForRollersAndMembers(pullRequestEvent);
final isPrivileged = await _isPrivilegedUser(pr, slug);
final hasLabel =
pr.labels?.any((l) => l.name == Config.kCicdLabel) ?? false;
final githubService = await config.createGithubService(slug);

if (isPrivileged) {
if (hasLabel) {
await _scheduleIfMergeable(pullRequestEvent);
} else {
await scheduler.createAwaitingCicdLabelCheckRun(
slug,
pr.head!.sha!,
);
}
} else {
if (hasLabel) {
await githubService.removeLabel(
slug,
pr.number!,
Config.kCicdLabel,
);
log.debug('Removed CICD label from PR ${pr.number}');
}
await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
}
break;
// Ignore the rest of the events.
case 'ready_for_review':
Expand Down Expand Up @@ -580,27 +610,16 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
);
}

Future<void> _addCICDForRollersAndMembers(
PullRequestEvent pullRequestEvent,
) async {
final pr = pullRequestEvent.pullRequest!;
final slug = pr.base!.repo!.slug();
final author = pr.user!.login!;
final githubService = await config.createGithubService(slug);

/// Returns true if the user is a roller or a member of flutter-hackers.
Future<bool> _isPrivilegedUser(PullRequest pr, RepositorySlug slug) async {
final isRoller = config.rollerAccounts.contains(pr.user!.login);
final githubService = await config.createGithubService(slug);
final isFlutterHacker = await githubService.isTeamMember(
'flutter-hackers',
author,
slug.owner,
pr.user!.login!,
'flutter',
);
log.debug('isRoller=$isRoller, isFlutterHacker=$isFlutterHacker');

if (config.supportedRepos.contains(slug) && (isRoller || isFlutterHacker)) {
final gitHubClient = await config.createGitHubClient(pullRequest: pr);
await gitHubClient.issues.addLabelsToIssue(slug, pr.number!, ['CICD']);
log.debug('Added CICD label to PR $pr.number');
}
return isRoller || isFlutterHacker;
}

Future<void> _checkForTests(PullRequestEvent pullRequestEvent) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3755,7 +3755,7 @@ void foo() {
});

test(
'opened PR with adds CICD label if author is member of flutter-hackers',
'opened PR with adds CICD label and schedules tests if author is member of flutter-hackers',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
Expand All @@ -3766,7 +3766,8 @@ void foo() {

verify(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
);
).called(1);
expect(scheduler.triggerPresubmitTargetsCnt, 1);
},
);
test(
Expand Down Expand Up @@ -3861,50 +3862,87 @@ void foo() {
);

test(
'synchronize event with CICD label does not schedules tests',
'synchronize event by non-privileged user removes CICD label and creates awaiting checkrun',
() async {
githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
withCicdLabel: true,
);

await tester.post(webhook);

expect(githubService.removedLabels, [
(Config.flutterSlug, 123, Config.kCicdLabel),
]);
verify(
mockGithubChecksUtil.createCheckRun(
any,
any,
any,
'Awaiting CICD label',
output: anyNamed('output'),
),
).called(1);
expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

test(
'synchronize event adds CICD label if author is member of flutter-hackers',
'synchronize event by non-privileged user without label creates awaiting checkrun',
() async {
githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
login: 'test-flutter-hacker',
withCicdLabel: false,
);

await tester.post(webhook);

verify(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
);
mockGithubChecksUtil.createCheckRun(
any,
any,
any,
'Awaiting CICD label',
output: anyNamed('output'),
),
).called(1);
expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

test(
'synchronize event does not add CICD label if author is not member of flutter-hackers',
'synchronize event by privileged user with label schedules tests',
() async {
tester.message = generateGithubWebhookMessage(action: 'synchronize');
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
login: 'test-flutter-hacker',
withCicdLabel: true,
);

await tester.post(webhook);

expect(scheduler.triggerPresubmitTargetsCnt, 1);
verifyNever(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, any),
mockGithubChecksUtil.createCheckRun(
any,
any,
any,
'Awaiting CICD label',
output: anyNamed('output'),
),
);
},
);

test(
'opened PR without CICD label creates "Awaiting CICD label" checkrun',
'synchronize event by privileged user without label creates awaiting checkrun',
() async {
githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
tester.message = generateGithubWebhookMessage(
action: 'opened',
action: 'synchronize',
login: 'test-flutter-hacker',
withCicdLabel: false,
);

Expand All @@ -3919,16 +3957,17 @@ void foo() {
output: anyNamed('output'),
),
).called(1);
expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

test(
'opened PR with CICD label STILL creates "Awaiting CICD label" checkrun if not exists',
'opened PR without CICD label creates "Awaiting CICD label" checkrun',
() async {
githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: true,
withCicdLabel: false,
);

await tester.post(webhook);
Expand All @@ -3946,12 +3985,12 @@ void foo() {
);

test(
'edited PR without CICD label creates "Awaiting CICD label" checkrun if not exists',
'opened PR with CICD label STILL creates "Awaiting CICD label" checkrun if not exists',
() async {
githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
tester.message = generateGithubWebhookMessage(
action: 'edited',
withCicdLabel: false,
action: 'opened',
withCicdLabel: true,
);

await tester.post(webhook);
Expand Down
Loading