Fusion checks are not scheduled if any engine checks are failed so no need to look up for failed checks in both, only last guard might have failed checks.

ievdokdm · ievdokdm · commit 70b6bcb745b2 · 2026-01-22T18:06:09.000-08:00
diff --git a/app_dart/lib/src/service/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart
@@ -37,17 +37,21 @@ final class UnifiedCheckRun {
         config.flags.isUnifiedCheckRunFlowEnabledForUser(
           pullRequest.user!.login!,
         )) {
+      // Create the UnifiedCheckRun and UnifiedCheckRunBuilds.
       log.info(
         'Storing UnifiedCheckRun data for ${slug.fullName}#${pullRequest.number} as it enabled for user ${pullRequest.user!.login}.',
       );
-      // Create the UnifiedCheckRun and UnifiedCheckRunBuilds.
+      // We store the creation time of the guard since there might be several
+      // guards for the same PR created and each new one created after previous
+      // was succeeded so we are interested in a state of the last one.
+      final creationTime = DateTime.now().toUtc().microsecondsSinceEpoch;
       final guard = PresubmitGuard(
         checkRun: checkRun,
         commitSha: sha,
         slug: slug,
         pullRequestId: pullRequest.number!,
         stage: stage,
-        creationTime: pullRequest.createdAt!.microsecondsSinceEpoch,
+        creationTime: creationTime,
         author: pullRequest.user!.login!,
         remainingBuilds: tasks.length,
         failedBuilds: 0,
@@ -58,7 +62,7 @@ final class UnifiedCheckRun {
           PresubmitCheck.init(
             buildName: task,
             checkRunId: checkRun.id!,
-            creationTime: pullRequest.createdAt!.microsecondsSinceEpoch,
+            creationTime: creationTime,
           ),
       ];
       await firestoreService.writeViaTransaction(
@@ -89,63 +93,67 @@ final class UnifiedCheckRun {
     log.info('$logCrumb Re-Running failed checks.');
     final transaction = await firestoreService.beginTransaction();
 
-    final guards = await getPresubmitGuardsForCheckRun(
+    // New guard created only if previous is succeeded so failed checks might be
+    // only in last guard.
+    final guard = await getLastPresubmitGuardForCheckRun(
       firestoreService: firestoreService,
       slug: slug,
       pullRequestId: pullRequestId,
       checkRunId: checkRunId,
       transaction: transaction,
     );
 
-    for (final guard in guards) {
-      // Copy the failed build names to a local variable to avoid losing the
-      // failed build names after resetting the failed guard.builds.
-      final failedBuildNames = guard.failedBuildNames;
-      if (failedBuildNames.isNotEmpty) {
-        guard.failedBuilds = 0;
-        guard.remainingBuilds = failedBuildNames.length;
-        final builds = guard.builds;
-        for (final buildName in failedBuildNames) {
-          builds[buildName] = TaskStatus.waitingForBackfill;
-        }
-        guard.builds = builds;
-        final checks = [
-          for (final buildName in failedBuildNames)
-            PresubmitCheck.init(
-              buildName: buildName,
-              checkRunId: checkRunId,
-              creationTime: DateTime.now().toUtc().microsecondsSinceEpoch,
-              attemptNumber:
-                  ((await getLatestPresubmitCheck(
-                        firestoreService: firestoreService,
-                        checkRunId: checkRunId,
-                        buildName: buildName,
-                        transaction: transaction,
-                      ))?.attemptNumber ??
-                      0) +
-                  1, // Increment the latest attempt number.
-            ),
-        ];
-        try {
-          final response = await firestoreService.commit(
-            transaction,
-            documentsToWrites([...checks, guard]),
-          );
-          log.info(
-            '$logCrumb: results = ${response.writeResults?.map((e) => e.toJson())}',
-          );
-          return FailedChecksForRerun(
-            checkRunGuard: guard.checkRun,
-            checkNames: failedBuildNames,
-            stage: guard.stage,
-          );
-        } catch (e) {
-          log.info('$logCrumb: failed to update presubmit check', e);
-          rethrow;
-        }
+    if (guard == null) {
+      return null;
+    }
+
+    // Copy the failed build names to a local variable to avoid losing the
+    // failed build names after resetting the failed guard.builds.
+    final creationTime = DateTime.now().toUtc().microsecondsSinceEpoch;
+    final failedBuildNames = guard.failedBuildNames;
+    if (failedBuildNames.isNotEmpty) {
+      guard.failedBuilds = 0;
+      guard.remainingBuilds = failedBuildNames.length;
+      final builds = guard.builds;
+      for (final buildName in failedBuildNames) {
+        builds[buildName] = TaskStatus.waitingForBackfill;
+      }
+      guard.builds = builds;
+      final checks = [
+        for (final buildName in failedBuildNames)
+          PresubmitCheck.init(
+            buildName: buildName,
+            checkRunId: checkRunId,
+            creationTime: creationTime,
+            attemptNumber:
+                ((await getLatestPresubmitCheck(
+                      firestoreService: firestoreService,
+                      checkRunId: checkRunId,
+                      buildName: buildName,
+                      transaction: transaction,
+                    ))?.attemptNumber ??
+                    0) +
+                1, // Increment the latest attempt number.
+          ),
+      ];
+      try {
+        final response = await firestoreService.commit(
+          transaction,
+          documentsToWrites([...checks, guard]),
+        );
+        log.info(
+          '$logCrumb: results = ${response.writeResults?.map((e) => e.toJson())}',
+        );
+        return FailedChecksForRerun(
+          checkRunGuard: guard.checkRun,
+          checkNames: failedBuildNames,
+          stage: guard.stage,
+        );
+      } catch (e) {
+        log.info('$logCrumb: failed to update presubmit check', e);
+        rethrow;
       }
     }
-    return null;
   }
 
   /// Returns _all_ checks running against the specified github [checkRunId].
@@ -203,19 +211,19 @@ final class UnifiedCheckRun {
   }
 
   /// Returns [PresubmitGuard]s for the specified github [checkRunId].
-  static Future<List<PresubmitGuard>> getPresubmitGuardsForCheckRun({
+  static Future<PresubmitGuard?> getLastPresubmitGuardForCheckRun({
     required FirestoreService firestoreService,
     required RepositorySlug slug,
     required int pullRequestId,
     required int checkRunId,
     Transaction? transaction,
   }) async {
-    return await _queryPresubmitGuards(
+    return (await _queryPresubmitGuards(
       firestoreService: firestoreService,
       checkRunId: checkRunId,
       transaction: transaction,
-      orderMap: const {PresubmitGuard.fieldStage: kQueryOrderAscending},
-    );
+      limit: 1,
+    )).firstOrNull;
   }
 
   static Future<List<PresubmitGuard>> _queryPresubmitGuards({
diff --git a/app_dart/test/service/firestore/unified_check_run_test.dart b/app_dart/test/service/firestore/unified_check_run_test.dart
@@ -484,7 +484,7 @@ void main() {
       });
     });
 
-    test('getPresubmitGuardsForCheckRun returns correct guards', () async {
+    test('getLastPresubmitGuardForCheckRun returns newest guard', () async {
       final sha = 'sha';
       final slug = RepositorySlug('flutter', 'flutter');
       final checkRun = CheckRun.fromJson(const {
@@ -513,7 +513,7 @@ void main() {
         slug: slug,
         pullRequestId: 1,
         stage: CiStage.fusionTests,
-        creationTime: 1000,
+        creationTime: 2000,
         author: 'dash',
         remainingBuilds: 1,
         failedBuilds: 0,
@@ -527,17 +527,16 @@ void main() {
         ], exists: false),
       );
 
-      final guards = await UnifiedCheckRun.getPresubmitGuardsForCheckRun(
+      final guard = await UnifiedCheckRun.getLastPresubmitGuardForCheckRun(
         firestoreService: firestoreService,
         slug: slug,
         pullRequestId: 1,
         checkRunId: 123,
       );
 
-      expect(guards, hasLength(2));
-      expect(guards[0].stage, CiStage.fusionEngineBuild);
-      expect(guards[1].stage, CiStage.fusionTests);
-      expect(guards.every((g) => g.checkRunId == 123), isTrue);
+      expect(guard, isNotNull);
+      expect(guard!.stage, CiStage.fusionTests);
+      expect(guard.checkRunId, 123);
     });
   });
 }
diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart
@@ -1290,8 +1290,6 @@ targets:
         expect(updatedGuard.builds['Linux A'], TaskStatus.waitingForBackfill);
       });
 
-
-
       test('rerequested action reschedules failed checks for fusion', () async {
         final mockGithubChecksService = MockGithubChecksService();
         when(
@@ -1323,7 +1321,7 @@ targets:
           slug: slug,
           pullRequestId: 1,
           stage: CiStage.fusionTests,
-          creationTime: 1000,
+          creationTime: 2000,
           author: 'dash',
           builds: {'Linux A': TaskStatus.failed},
           failedBuilds: 1,
@@ -1333,7 +1331,7 @@ targets:
         final check = PresubmitCheck(
           buildName: 'Linux A',
           checkRunId: 1,
-          creationTime: 1000,
+          creationTime: 2000,
           status: TaskStatus.failed,
           attemptNumber: 1,
         );
