Schedule bringup true targets for guaranteed policy repos (flutter#3139)

Fixes: flutter/flutter#136361

Author: keyonghan

app_dart/lib/src/service/scheduler.dart

@@ -142,14 +142,10 @@ class Scheduler {
         policy = GuaranteedPolicy();
       }
       final int? priority = await policy.triggerPriority(task: task, datastore: datastore);
-      // Skip scheduling `bringup: true` targets. They may be newly created and
-      // their corresponding LUCI builder configs may not be ready yet considering we
-      // disabled the `ci_yaml roller` backfill. Existing `bringup: true` targets
-      // can rely on backfiller to get tasks scheduled and executed.
-      if (priority != null && !target.value.bringup) {
+      if (_shouldSchedule(priority, target.value.bringup, policy)) {
         // Mark task as in progress to ensure it isn't scheduled over
         task.status = Task.statusInProgress;
-        toBeScheduled.add(Tuple<Target, Task, int>(target, task, priority));
+        toBeScheduled.add(Tuple<Target, Task, int>(target, task, priority!));
       }
     }
 
@@ -169,6 +165,22 @@ class Scheduler {
     await _uploadToBigQuery(commit);
   }
 
+  /// Checks whether the task should be scheduled.
+  ///
+  /// Skips scheduling `bringup: true` targets for BatchPolicy. For BatchPolicy,
+  /// targets may be newly created and their corresponding LUCI builder configs may
+  /// not be ready yet considering we disabled the `ci_yaml roller` backfill.
+  /// Existing `bringup: true` targets can rely on backfiller to get tasks scheduled and executed.
+  bool _shouldSchedule(int? priority, bool bringup, SchedulerPolicy policy) {
+    if (priority == null) {
+      return false;
+    }
+    if (bringup) {
+      return policy is! BatchPolicy;
+    }
+    return true;
+  }
+
   /// Schedule all builds in batch requests instead of a single request.
   ///
   /// Each batch request contains [Config.batchSize] builds to be scheduled.

app_dart/test/service/scheduler_test.dart

@@ -408,7 +408,7 @@ targets:
         expect(db.values.values.whereType<Task>().where((Task task) => task.status == Task.statusInProgress).length, 1);
       });
 
-      test('skip scheduling bringup true targets', () async {
+      test('skip scheduling bringup true targets for BatchPolicy', () async {
         final PullRequest mergedPr = generatePullRequest();
 
         httpClient = MockClient((http.Request request) async {
@@ -423,7 +423,6 @@ targets:
   - name: Linux B
     scheduler: cocoon
     bringup: true
-
           ''',
               200,
             );
@@ -439,6 +438,37 @@ targets:
         expect(taskB.status, Task.statusNew);
       });
 
+      test('schedule bringup true targets for GuaranteedPolicy', () async {
+        final PullRequest mergedPr = generatePullRequest(repo: 'engine', branch: 'main');
+
+        httpClient = MockClient((http.Request request) async {
+          if (request.url.path.contains('.ci.yaml')) {
+            return http.Response(
+              '''
+enabled_branches:
+  - main
+targets:
+  - name: Linux A
+    scheduler: cocoon
+  - name: Linux B
+    scheduler: cocoon
+    bringup: true
+
+          ''',
+              200,
+            );
+          }
+          throw Exception('Failed to find ${request.url.path}');
+        });
+        await scheduler.addPullRequest(mergedPr);
+        expect(db.values.values.whereType<Commit>().length, 1);
+        expect(db.values.values.whereType<Task>().length, 2);
+        final Task taskA = db.values.values.whereType<Task>().where((task) => task.name == 'Linux A').single;
+        final Task taskB = db.values.values.whereType<Task>().where((task) => task.name == 'Linux B').single;
+        expect(taskA.status, Task.statusInProgress);
+        expect(taskB.status, Task.statusInProgress);
+      });
+
       test('does not schedule tasks against non-merged PRs', () async {
         final PullRequest notMergedPr = generatePullRequest(merged: false);
         await scheduler.addPullRequest(notMergedPr);