fix failed state processing (#4921)

Fix failed state processing
Fixes: flutter/flutter#176982

Author: ievdokdm

app_dart/lib/src/service/firestore/unified_check_run.dart

@@ -230,21 +230,22 @@ final class UnifiedCheckRun {
         }
         valid = true;
       } else {
-        // "remaining" should go down if buildSuccessed of any attempt
-        // or buildFailed a first attempt.
+        // "remaining" should go down if build is succeeded or failed.
         // "failed_count" can go up or down depending on:
         //   attemptNumber > 1 && buildSuccessed: down (-1)
         //   attemptNumber = 1 && buildFailed: up (+1)
         // So if the test existed and either remaining or failed_count is changed;
         // the response is valid.
         status = state.status;
-        if (status.isBuildSuccessed) {
+        if (status.isBuildCompleted) {
           // Guard against going negative and log enough info so we can debug.
           if (remaining == 0) {
             throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}';
           }
           remaining = remaining - 1;
+        }
 
+        if (status.isBuildSuccessed) {
           // Only rollback the "failed" counter if this is a successful test run,
           // i.e. the test failed, the user requested a rerun, and now it passes.
           if (state.attemptNumber > 1) {

app_dart/lib/src/service/scheduler.dart

@@ -877,6 +877,30 @@ $s
     );
   }
 
+  Future<void> _requireActionForGuard({
+    required RepositorySlug slug,
+    required CheckRun lock,
+    required String headSha,
+    required String summary,
+    required String details,
+    String? detailsUrl,
+  }) async {
+    log.info('Failing merge group guard for merge group $headSha in $slug');
+    await _githubChecksService.githubChecksUtil.updateCheckRun(
+      _config,
+      slug,
+      lock,
+      status: CheckRunStatus.completed,
+      conclusion: CheckRunConclusion.actionRequired,
+      output: CheckRunOutput(
+        title: Config.kCiYamlCheckName,
+        summary: summary,
+        text: details,
+      ),
+      detailsUrl: detailsUrl,
+    );
+  }
+
   /// If [builderTriggerList] is specificed, return only builders that are contained in [presubmitTarget].
   /// Otherwise, return [presubmitTarget].
   List<Target> filterTargets(
@@ -1030,24 +1054,6 @@ $s
       return false;
     }
 
-    // If the number of failed checks is equal to the number of remaining checks, then all remaining checks have failed.
-    if (check.isUnifiedCheckRun &&
-        stagingConclusion.failed > 0 &&
-        stagingConclusion.failed == stagingConclusion.remaining) {
-      final guard = checkRunFromString(stagingConclusion.checkRunGuard!);
-      await failGuardForMergeGroup(
-        slug: check.slug,
-        lock: guard,
-        headSha: check.sha,
-        summary: kMergeQueueLockDescription,
-        details:
-            'For CI stage ${check.stage} ${stagingConclusion.failed} checks failed',
-        detailsUrl:
-            'https://flutter-dashboard.appspot.com/repo/${check.slug.name}/checkrun/${check.guardId}',
-      );
-      return true;
-    }
-
     // Are there tests remaining? Keep waiting.
     if (stagingConclusion.isPending) {
       log.info(
@@ -1063,6 +1069,7 @@ $s
       //   to the next stage. Let the author sort out what's up.
       // * If this is a merge group: kick the pull request out of the queue, and
       //   let the author sort it out.
+      // If its a unified check run we need to require action on the guard.
       if (check.isMergeGroup) {
         await _completeArtifacts(check.sha, false);
         final guard = checkRunFromString(stagingConclusion.checkRunGuard!);
@@ -1073,6 +1080,18 @@ $s
           summary: stagingConclusion.summary,
           details: stagingConclusion.details,
         );
+      } else if (check.isUnifiedCheckRun) {
+        final guard = checkRunFromString(stagingConclusion.checkRunGuard!);
+        await _requireActionForGuard(
+          slug: check.slug,
+          lock: guard,
+          headSha: check.sha,
+          summary: kMergeQueueLockDescription,
+          details:
+              'For CI stage ${check.stage} ${stagingConclusion.failed} checks failed',
+          detailsUrl:
+              'https://flutter-dashboard.appspot.com/repo/${check.slug.name}/checkrun/${check.guardId}',
+        );
       }
       return true;
     }

app_dart/test/service/firestore/unified_check_run_test.dart

@@ -267,7 +267,7 @@ void main() {
         );
 
         expect(result.result, PresubmitGuardConclusionResult.ok);
-        expect(result.remaining, 2);
+        expect(result.remaining, 1);
         expect(result.failed, 1);
       });
 

packages/cocoon_common/lib/task_status.dart

@@ -81,11 +81,7 @@ enum TaskStatus {
       this == TaskStatus.cancelled;
 
   /// Returns true if the build succeeded or some kind of failure occurred.
-  bool get isBuildCompleted =>
-      this == TaskStatus.succeeded ||
-      this == TaskStatus.failed ||
-      this == TaskStatus.infraFailure ||
-      this == TaskStatus.cancelled;
+  bool get isBuildCompleted => isBuildSuccessed || isBuildFailed;
 
   /// Returns the JSON representation of `this`.
   Object? toJson() => _schemaValue;