Merge branch 'flutter:main' into main

auto_submit/lib/model/auto_submit_query_result.dart

@@ -83,11 +83,14 @@ enum MergeableState {
 @JsonSerializable()
 class ContextNode {
   ContextNode({
+    this.createdAt,
     this.context,
     this.state,
     this.targetUrl,
   });
 
+  @JsonKey(name: 'createdAt')
+  DateTime? createdAt;
   String? context;
   String? state;
   @JsonKey(name: 'targetUrl')

auto_submit/lib/requests/graphql_queries.dart

@@ -57,6 +57,7 @@ query LabeledPullRequestWithReviews($sOwner: String!, $sName: String!, $sPrNumbe
             pushedDate
             status {
               contexts {
+                createdAt
                 context
                 state
                 targetUrl

auto_submit/lib/service/config.dart

@@ -48,6 +48,9 @@ class Config {
   /// Labels autosubmit looks for on pull requests
   static const String kAutosubmitLabel = 'autosubmit';
 
+  /// GitHub check stale threshold.
+  static const int kGitHubCheckStaleThreshold = 2; // hours
+
   // Labels the bot looks for on revert requests.
   // TODO (ricardoamador) https://github.com/flutter/flutter/issues/134845:
   // add a link to a one page doc outlining the workflow that happens here.

auto_submit/lib/validations/ci_successful.dart

@@ -79,7 +79,7 @@ class CiSuccessful extends Validation {
     }
 
     /// Validate if all checkRuns have succeeded.
-    allSuccess = validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess);
+    allSuccess = validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess, author);
 
     if (!allSuccess && failures.isEmpty) {
       return ValidationResult(allSuccess, Action.IGNORE_TEMPORARILY, '');
@@ -140,12 +140,13 @@ class CiSuccessful extends Validation {
     final String overrideTreeStatusLabel = config.overrideTreeStatusLabel;
     log.info('Validating name: ${slug.name}/$prNumber, statuses: $statuses');
 
+    final List<ContextNode> staleStatuses = <ContextNode>[];
     for (ContextNode status in statuses) {
       // How can name be null but presumed to not be null below when added to failure?
       final String? name = status.context;
 
       // If the account author is a roller account do not block merge on flutter-gold check.
-      if (config.rollerAccounts.contains(author.login!) && slug == Config.engineSlug && name == 'flutter-gold') {
+      if (isEngineRoller(author, slug) && name == 'flutter-gold') {
         log.info('Skipping status check for flutter-gold for ${slug.fullName}/$prNumber, pr author: $author.');
         continue;
       }
@@ -158,8 +159,16 @@ 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)) {
+          staleStatuses.add(status);
+        }
       }
     }
+    if (staleStatuses.isNotEmpty) {
+      log.warning(
+        'Pull request https://github.com/${slug.fullName}/pull/$prNumber from ${slug.name} repo auto roller has been running over ${Config.kGitHubCheckStaleThreshold} hours due to: ${staleStatuses.map((e) => e.context).toList()}',
+      );
+    }
 
     return allSuccess;
   }
@@ -174,12 +183,18 @@ class CiSuccessful extends Validation {
     List<github.CheckRun> checkRuns,
     Set<FailureDetail> failures,
     bool allSuccess,
+    Author author,
   ) {
     log.info('Validating name: ${slug.name}/$prNumber, checkRuns: $checkRuns');
 
+    final List<github.CheckRun> staleCheckRuns = <github.CheckRun>[];
     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 &&
@@ -193,7 +208,21 @@ class CiSuccessful extends Validation {
       }
       allSuccess = false;
     }
+    if (staleCheckRuns.isNotEmpty) {
+      log.warning(
+        'Pull request https://github.com/${slug.fullName}/pull/$prNumber from ${slug.name} repo auto roller has been running over ${Config.kGitHubCheckStaleThreshold} hours due to: ${staleCheckRuns.map((e) => e.name).toList()}',
+      );
+    }
 
     return allSuccess;
   }
+
+  // Treat any GitHub check run as stale if created over [Config.kGitHubCheckStaleThreshold] hours ago.
+  bool isStale(DateTime dateTime) {
+    return dateTime.compareTo(DateTime.now().subtract(const Duration(hours: Config.kGitHubCheckStaleThreshold))) < 0;
+  }
+
+  bool isEngineRoller(Author author, github.RepositorySlug slug) {
+    return config.rollerAccounts.contains(author.login!) && slug == Config.engineSlug;
+  }
 }

auto_submit/lib/validations/validation.dart

@@ -14,6 +14,7 @@ const CHANGES_REQUESTED_STATE = 'CHANGES_REQUESTED';
 /// GitHub status state.
 const STATUS_SUCCESS = 'SUCCESS';
 const STATUS_FAILURE = 'FAILURE';
+const STATUS_PENDING = 'PENDING';
 
 /// GitHub merge state.
 const UNKNOWN_MERGE_STATE = 'UNKNOWN';

auto_submit/test/model/auto_submit_query_result_test.dart

@@ -55,7 +55,9 @@ void main() {
       expect(commitNode.commit!.abbreviatedOid, '4009ecc');
       expect(commitNode.commit!.oid, '4009ecc0b6dbf5cb19cb97472147063e7368ec10');
       expect(commitNode.commit!.pushedDate, DateTime.parse('2022-05-11 22:35:03.000Z'));
-      expect(commitNode.commit!.status, isNull);
+      expect(commitNode.commit!.status, isNotNull);
+      final List<ContextNode> statuses = commitNode.commit!.status!.contexts!;
+      expect(statuses[0].createdAt, DateTime.parse('2023-12-01T23:29:12Z'));
     });
   });
 
@@ -114,7 +116,22 @@ const String dataString = '''
               "oid": "4009ecc0b6dbf5cb19cb97472147063e7368ec10",
               "committedDate": "2022-05-11T22:35:02Z",
               "pushedDate": "2022-05-11T22:35:03Z",
-              "status":null
+              "status": {
+                  "contexts": [
+                    {
+                      "createdAt": "2023-12-01T23:29:12Z",
+                      "context": "flutter-gold",
+                      "state": "SUCCESS",
+                      "targetUrl": "https://flutter-gold.skia.org/cl/github/139397"
+                    },
+                    {
+                      "createdAt": "2023-12-01T22:55:04Z",
+                      "context": "tree-status",
+                      "state": "SUCCESS",
+                      "targetUrl": "https://flutter-dashboard.appspot.com/#/build?repo=flutter"
+                    }
+                  ]
+                }
             }
           }
         ]

auto_submit/test/validations/ci_successful_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:convert';
+import 'dart:core';
 
 import 'ci_successful_test_data.dart';
 
@@ -67,7 +68,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isTrue);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isTrue,
+        );
         expect(failures, isEmpty);
       });
     });
@@ -78,7 +89,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isTrue);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isTrue,
+        );
         expect(failures, isEmpty);
       });
     });
@@ -89,7 +110,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isTrue);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isTrue,
+        );
         expect(failures, isEmpty);
       });
     });
@@ -100,7 +131,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isFalse);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isFalse,
+        );
         expect(failures, isNotEmpty);
         expect(failures.length, 1);
       });
@@ -112,7 +153,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isTrue);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isTrue,
+        );
         expect(failures, isEmpty);
       });
     });
@@ -123,7 +174,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isFalse);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isFalse,
+        );
         expect(failures, isNotEmpty);
         expect(failures.length, 1);
       });
@@ -137,7 +198,17 @@ void main() {
       const bool allSuccess = true;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isFalse);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isFalse,
+        );
         expect(failures, isEmpty);
       });
     });
@@ -148,7 +219,17 @@ void main() {
       const bool allSuccess = false;
 
       checkRunFuture.then((checkRuns) {
-        expect(ciSuccessful.validateCheckRuns(slug, prNumber, checkRuns, failures, allSuccess), isFalse);
+        expect(
+          ciSuccessful.validateCheckRuns(
+            slug,
+            prNumber,
+            checkRuns,
+            failures,
+            allSuccess,
+            Author(login: 'testAuthor'),
+          ),
+          isFalse,
+        );
         expect(failures, isNotEmpty);
         expect(failures.length, 1);
       });
@@ -512,4 +593,48 @@ void main() {
       );
     });
   });
+  group('Validate if a datetime is stale', () {
+    setUp(() {
+      githubService = FakeGithubService(client: MockGitHub());
+      config = FakeConfig(githubService: githubService);
+      ciSuccessful = CiSuccessful(config: config);
+    });
+
+    test('when it is stale', () async {
+      final bool isStale = ciSuccessful.isStale(DateTime.now().subtract(const Duration(hours: 3)));
+      expect(isStale, true);
+    });
+    test('when it is not stale', () async {
+      final bool isStale = ciSuccessful.isStale(DateTime.now().subtract(const Duration(hours: 1)));
+      expect(isStale, false);
+    });
+  });
+
+  group('Validate if an engine 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.isEngineRoller(
+        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'));
+      expect(isEngineRoller, false);
+    });
+    test('when it is not from engine', () async {
+      final bool isEngineRoller = ciSuccessful.isEngineRoller(
+        Author(login: 'engine-flutter-autoroll'),
+        github.RepositorySlug('flutter', 'flutter'),
+      );
+      expect(isEngineRoller, false);
+    });
+  });
 }