Flag stale PRs for engine to framework roller (#3338)

Following up of https://github.com/flutter/cocoon/pull/3332/files#r1423217355 This PR does: 1) supports engine to framework roller 2) change stale logic to focus on `queued` status. There are cases where GitHub check runs have finished a while ago, and we do not want to trigger alerts for them.
flutter · Dec 15, 2023 · d9ab885 · d9ab885
1 parent 3c38374
commit d9ab885
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 10 deletions.
diff --git a/auto_submit/lib/validations/ci_successful.dart b/auto_submit/lib/validations/ci_successful.dart
@@ -146,7 +146,7 @@ class CiSuccessful extends Validation {
       final String? name = status.context;
 
       // If the account author is a roller account do not block merge on flutter-gold check.
-      if (isEngineRoller(author, slug) && name == 'flutter-gold') {
+      if (isToEngineRoller(author, slug) && name == 'flutter-gold') {
         log.info('Skipping status check for flutter-gold for ${slug.fullName}/$prNumber, pr author: $author.');
         continue;
       }
@@ -159,7 +159,7 @@ class CiSuccessful extends Validation {
         if (status.state == STATUS_FAILURE && !notInAuthorsControl.contains(name)) {
           failures.add(FailureDetail(name!, status.targetUrl!));
         }
-        if (status.state == STATUS_PENDING && isStale(status.createdAt!) && isEngineRoller(author, slug)) {
+        if (status.state == STATUS_PENDING && isStale(status.createdAt!) && supportStale(author, slug)) {
           staleStatuses.add(status);
         }
       }
@@ -191,10 +191,6 @@ class CiSuccessful extends Validation {
     for (github.CheckRun checkRun in checkRuns) {
       final String? name = checkRun.name;
 
-      if (isStale(checkRun.startedAt) && isEngineRoller(author, slug)) {
-        staleCheckRuns.add(checkRun);
-      }
-
       if (checkRun.conclusion == github.CheckRunConclusion.skipped ||
           checkRun.conclusion == github.CheckRunConclusion.success ||
           (checkRun.status == github.CheckRunStatus.completed &&
@@ -205,6 +201,10 @@ class CiSuccessful extends Validation {
         // checkrun has failed.
         log.info('${slug.name}/$prNumber: CheckRun $name failed.');
         failures.add(FailureDetail(name!, checkRun.detailsUrl as String));
+      } else if (checkRun.status == github.CheckRunStatus.queued) {
+        if (isStale(checkRun.startedAt) && supportStale(author, slug)) {
+          staleCheckRuns.add(checkRun);
+        }
       }
       allSuccess = false;
     }
@@ -222,7 +222,19 @@ class CiSuccessful extends Validation {
     return dateTime.compareTo(DateTime.now().subtract(const Duration(hours: Config.kGitHubCheckStaleThreshold))) < 0;
   }
 
-  bool isEngineRoller(Author author, github.RepositorySlug slug) {
+  /// Perform stale check only on Engine related rolled PRs.
+  ///
+  /// This includes those rolled PRs from upstream to Engine repo and those
+  /// rolled PRs from Engine to Framework.
+  bool supportStale(Author author, github.RepositorySlug slug) {
+    return isToEngineRoller(author, slug) || isEngineToFrameworkRoller(author, slug);
+  }
+
+  bool isToEngineRoller(Author author, github.RepositorySlug slug) {
     return config.rollerAccounts.contains(author.login!) && slug == Config.engineSlug;
   }
+
+  bool isEngineToFrameworkRoller(Author author, github.RepositorySlug slug) {
+    return author.login! == 'engine-flutter-autoroll' && slug == Config.flutterSlug;
+  }
 }
diff --git a/auto_submit/test/validations/ci_successful_test.dart b/auto_submit/test/validations/ci_successful_test.dart
@@ -618,23 +618,53 @@ void main() {
     });
 
     test('when it is engine roller', () async {
-      final bool isEngineRoller = ciSuccessful.isEngineRoller(
+      final bool isEngineRoller = ciSuccessful.isToEngineRoller(
         Author(login: 'engine-flutter-autoroll'),
         github.RepositorySlug('flutter', 'engine'),
       );
       expect(isEngineRoller, true);
     });
     test('when it is not from roller', () async {
       final bool isEngineRoller =
-          ciSuccessful.isEngineRoller(Author(login: 'testAuthor'), github.RepositorySlug('flutter', 'engine'));
+          ciSuccessful.isToEngineRoller(Author(login: 'testAuthor'), github.RepositorySlug('flutter', 'engine'));
       expect(isEngineRoller, false);
     });
     test('when it is not from engine', () async {
-      final bool isEngineRoller = ciSuccessful.isEngineRoller(
+      final bool isEngineRoller = ciSuccessful.isToEngineRoller(
         Author(login: 'engine-flutter-autoroll'),
         github.RepositorySlug('flutter', 'flutter'),
       );
       expect(isEngineRoller, false);
     });
   });
+
+  group('Validate if an engine to framework roller', () {
+    setUp(() {
+      githubService = FakeGithubService(client: MockGitHub());
+      config = FakeConfig(githubService: githubService);
+      ciSuccessful = CiSuccessful(config: config);
+    });
+
+    test('when it is engine roller', () async {
+      final bool isEngineRoller = ciSuccessful.isEngineToFrameworkRoller(
+        Author(login: 'engine-flutter-autoroll'),
+        github.RepositorySlug('flutter', 'flutter'),
+      );
+      expect(isEngineRoller, true);
+    });
+    test('when it is not from roller', () async {
+      final bool isEngineRoller = ciSuccessful.isEngineToFrameworkRoller(
+        Author(login: 'testAuthor'),
+        github.RepositorySlug('flutter', 'flutter'),
+      );
+      expect(isEngineRoller, false);
+    });
+    test('when it is not from framework', () async {
+      final bool isEngineRoller = ciSuccessful.isEngineToFrameworkRoller(
+        Author(login: 'engine-flutter-autoroll'),
+        github.RepositorySlug('flutter', 'engine'),
+      );
+      expect(isEngineRoller, false);
+    });
+  });
 }