Fix backfill execution based on ToT target comparison.

app_dart/lib/src/request_handlers/scheduler/backfill_grid.dart

@@ -30,19 +30,20 @@ final class BackfillGrid {
   /// Creates an indexed data structure from the provided `(commit, [tasks])`.
   ///
   /// Some automatic filtering is applied, removing targets/tasks where:
-  /// - [tipOfTreeTargets] that does not use [BatchPolicy];
-  /// - [tipOfTreeTargets] without a task;
-  /// - task that does have a matching [tipOfTreeTargets].
+  /// - [postsubmitTargets] that does not use [BatchPolicy];
+  /// - [postsubmitTargets] without a task;
+  /// - task that does have a matching [postsubmitTargets].
   factory BackfillGrid.from(
     Iterable<(CommitRef, List<TaskRef>)> grid, {
-    required Iterable<Target> tipOfTreeTargets,
+    required Iterable<Target> postsubmitTargets,
   }) {
-    final totTargetsByName = {for (final t in tipOfTreeTargets) t.name: t};
+    final totTargetsByName = {for (final t in postsubmitTargets) t.name: t};
     final commitsByName = <String, CommitRef>{};
     final tasksByName = <String, List<TaskRef>>{};
     for (final (commit, tasks) in grid) {
       commitsByName[commit.sha] = commit;
       for (final task in tasks) {
+        (tasksByName[task.name] ??= []).add(task);
         // Must exist at ToT (in this Map) and must be BatchPolicy.
         if (totTargetsByName[task.name]?.schedulerPolicy is! BatchPolicy) {
           // Even if it existed, let's remove it at this point because it is no
@@ -51,7 +52,6 @@ final class BackfillGrid {
           totTargetsByName.remove(task.name);
           continue;
         }
-        (tasksByName[task.name] ??= []).add(task);
       }
     }
 
@@ -122,15 +122,11 @@ final class BackfillGrid {
   }
 
   @useResult
-  Target _validateColumnAndTarget(String name, List<TaskRef> column) {
+  Target? _validateColumnAndTarget(String name, List<TaskRef> column) {
     if (column.isEmpty) {
       throw StateError('A target ("$name") should never have 0 tasks');
     }
-    final target = _targetsByName[name];
-    if (target == null) {
-      throw StateError('A target ("$name") should have existed in the grid');
-    }
-    return target;
+    return _targetsByName[name];
   }
 
   /// Each task, ordered by column (task by task).
@@ -139,7 +135,7 @@ final class BackfillGrid {
   Iterable<(Target, List<TaskRef>)> get eligibleTasks sync* {
     for (final MapEntry(key: name, value: column) in _tasksByName.entries) {
       final target = _validateColumnAndTarget(name, column);
-      if (target.backfill) {
+      if (target != null && target.backfill) {
         yield (target, column);
       }
     }
@@ -151,7 +147,7 @@ final class BackfillGrid {
   Iterable<SkippableTask> get skippableTasks sync* {
     for (final MapEntry(key: name, value: column) in _tasksByName.entries) {
       final target = _validateColumnAndTarget(name, column);
-      if (target.backfill) {
+      if (target?.backfill == true) {
         continue;
       }
       for (final task in column) {
@@ -164,7 +160,7 @@ final class BackfillGrid {
             'A commit ("${task.commitSha}") should have existed in the grid',
           );
         }
-        yield SkippableTask._from(task, target: target, commit: commit);
+        yield SkippableTask._from(task, commit: commit);
       }
     }
   }
@@ -235,26 +231,18 @@ final class BackfillTask {
 /// A proposed task to be skipped as part of the backfill process.
 @immutable
 final class SkippableTask {
-  const SkippableTask._from(
-    this.task, {
-    required this.target,
-    required this.commit,
-  });
+  const SkippableTask._from(this.task, {required this.commit});
 
   /// The task itself.
   final TaskRef task;
 
-  /// Which [Target] (originating from `.ci.yaml`) defined this task.
-  final Target target;
-
   /// The commit this task is associated with.
   final CommitRef commit;
 
   @override
   String toString() {
     return 'SkippableTask ${const JsonEncoder.withIndent('  ').convert({
       'task': '$task', //
-      'target': '$target',
       'commit': '$commit',
     })}';
   }

app_dart/lib/src/request_handlers/scheduler/batch_backfiller.dart

@@ -106,26 +106,26 @@ final class BatchBackfiller extends RequestHandler {
         '${fsGrid.map((i) => i.tasks).expand((i) => i).length} tasks',
       );
 
-      // Download the ToT .ci.yaml targets.
-      final totCiYaml = await _ciYamlFetcher.getTipOfTreeCiYaml(slug: slug);
-
-      final totTargets = [
-        ...totCiYaml.postsubmitTargets(),
-        if (totCiYaml.isFusion)
-          ...totCiYaml.postsubmitTargets(type: CiType.fusionEngine),
+      // Download the current commits targets.
+      final currentCiYaml = await _ciYamlFetcher.getCiYamlByCommit(
+        fsGrid.first.commit,
+        postsubmit: true,
+      );
+      final currentTargets = [
+        ...currentCiYaml.postsubmitTargets(),
+        if (currentCiYaml.isFusion)
+          ...currentCiYaml.postsubmitTargets(type: CiType.fusionEngine),
       ];
-      if (totTargets.isEmpty) {
-        log.warn(
-          'Did not fetch any tip-of-tree targets. Backfill will do nothing!',
-        );
+      if (currentTargets.isEmpty) {
+        log.warn('Did not fetch any targets. Backfill will do nothing!');
       } else {
-        log.debug('Fetched ${totTargets.length} tip-of-tree targets');
+        log.debug('Fetched ${currentTargets.length} targets');
       }
 
       grid = BackfillGrid.from([
         for (final CommitAndTasks(:commit, :tasks) in fsGrid)
           (commit, [...tasks.map((t) => t.toRef())]),
-      ], tipOfTreeTargets: totTargets);
+      ], postsubmitTargets: currentTargets);
     }
     log.debug('Built a grid of ${grid.eligibleTasks.length} target columns');
 

app_dart/test/request_handlers/scheduler/backfill_grid_test.dart

@@ -24,7 +24,7 @@ void main() {
       [
         (commit, [task]),
       ],
-      tipOfTreeTargets: [target],
+      postsubmitTargets: [target],
     );
 
     expect(
@@ -51,7 +51,7 @@ void main() {
         (c1, [t1c1, t2c1]),
         (c2, [t1c2, t2c2]),
       ],
-      tipOfTreeTargets: [tg1, tg2],
+      postsubmitTargets: [tg1, tg2],
     );
 
     expect(
@@ -88,7 +88,7 @@ void main() {
         (c1, [t1c1, t2c1]),
         (c2, [t1c2, t2c2]),
       ],
-      tipOfTreeTargets: [tg1, tg2],
+      postsubmitTargets: [tg1, tg2],
     );
 
     expect(
@@ -119,7 +119,7 @@ void main() {
       [
         (commit, [taskExists, taskMissing]),
       ],
-      tipOfTreeTargets: [targetExists],
+      postsubmitTargets: [targetExists],
     );
 
     expect(
@@ -149,7 +149,7 @@ void main() {
       [
         (commit, [taskBatch, taskNonBatch]),
       ],
-      tipOfTreeTargets: [targetBatch, targetNonBatch],
+      postsubmitTargets: [targetBatch, targetNonBatch],
     );
 
     expect(
@@ -172,7 +172,7 @@ void main() {
       [
         (commit, [task]),
       ],
-      tipOfTreeTargets: [target],
+      postsubmitTargets: [target],
     );
 
     expect(

app_dart/test/request_handlers/scheduler/backfill_strategy_test.dart

@@ -88,7 +88,7 @@ void main() {
         //           Linux TASK_0           Linux TASK_1
         (commits[0], [taskNew       (0, 0),        taskNew (0, 1)]),
         (commits[1], [taskSucceeded (1, 0), taskInProgress (1, 1)]),
-      ], tipOfTreeTargets: [
+      ], postsubmitTargets: [
         targets[0],
         targets[1],
       ]);
@@ -112,7 +112,7 @@ void main() {
         //           Linux TASK_0            Linux TASK_1
         (commits[0], [taskNew       (0, 0),  taskNew (0, 1)]),
         (commits[1], [taskNew       (1, 0),  taskNew (1, 1)]),
-      ], tipOfTreeTargets: [
+      ], postsubmitTargets: [
         targets[0],
         targets[1],
       ]);
@@ -146,7 +146,7 @@ void main() {
         //           Linux TASK_0            Linux TASK_1
         (commits[0], [taskNew       (0, 0),  taskSucceeded (0, 1)]),
         (commits[1], [taskNew       (1, 0),  taskNew       (1, 1)]),
-      ], tipOfTreeTargets: [
+      ], postsubmitTargets: [
         targets[0],
         targets[1],
       ]);
@@ -195,7 +195,7 @@ void main() {
         (commits[1], [taskNew       (5, 0),  taskNew    (5, 1),    taskFailed (5, 2)]), // < 5
         (commits[1], [taskNew       (6, 0),  taskFailed (6, 1),    taskNew    (6, 2)]), // < 6
         (commits[1], [taskFailed    (7, 0),  taskNew    (7, 1),    taskNew    (7, 2)]), // < 7
-      ], tipOfTreeTargets: [
+      ], postsubmitTargets: [
         targets[0],
         targets[1],
         targets[2],
@@ -228,7 +228,7 @@ void main() {
         (commit, [
           generateFirestoreTask(1, commitSha: '123', name: targets[0].name).toRef()
         ])
-      ], tipOfTreeTargets: [
+      ], postsubmitTargets: [
         targets[0],
       ]);
       // dart format on

app_dart/test/request_handlers/scheduler/batch_backfiller_test.dart

@@ -212,8 +212,8 @@ void main() {
 
     // AFTER:
     expect(await visualizeFirestoreGrid(), [
-      '🧑‍💼 ⬜ 🟨 🟥 🟩',
-      '🧑‍💼 ⬜ 🟨 🟨 🟨',
+      '🧑‍💼 ⬛️ 🟨 🟥 🟩',
+      '🧑‍💼 ⬛️ 🟨 🟨 🟨',
     ]);
   });
 
@@ -379,6 +379,115 @@ void main() {
       reason: 'Should use a low priority for all executions',
     );
   });
+
+  test('skips targets that do not exist in ToT', () async {
+    ciYamlFetcher.setTotCiYamlFrom('''
+      enabled_branches:
+        - master
+
+      targets:
+        - name: Linux Will_Run
+    ''', engine: '');
+    ciYamlFetcher.setCiYamlFrom('''
+      enabled_branches:
+        - master
+
+      targets:
+        - name: Linux Will_Run
+        - name: Linux Will_Not_Run
+    ''', engine: '');
+    // Add a commit and two tasks.
+    // The second task will be skipped because it doesn't exist in ToT.
+    firestore.putDocument(
+      generateFirestoreCommit(
+        0,
+        branch: 'master',
+        createTimestamp: fakeNow.millisecondsSinceEpoch,
+      ),
+    );
+    firestore.putDocument(
+      generateFirestoreTask(0, name: 'Linux Will_Run', commitSha: '0'),
+    );
+    firestore.putDocument(
+      generateFirestoreTask(1, name: 'Linux Will_Not_Run', commitSha: '0'),
+    );
+
+    // BEFORE:
+    expect(
+      await visualizeFirestoreGrid(),
+      // dart format off
+      [
+      '🧑‍💼 ⬜ ⬜',
+      ],
+      // dart format on
+    );
+
+    await tester.get(handler);
+
+    // AFTER:
+    expect(
+      await visualizeFirestoreGrid(),
+      // dart format off
+      [
+      '🧑‍💼 🟨 ⬛️',
+      ],
+      // dart format on
+    );
+  });
+
+  test('uses target definition from the current commit', () async {
+    ciYamlFetcher.setTotCiYamlFrom('''
+      enabled_branches:
+        - master
+
+      targets:
+        - name: Linux Will_Run
+          properties:
+            is-tot: "true"
+    ''', engine: '');
+    ciYamlFetcher.setCiYamlFrom('''
+      enabled_branches:
+        - master
+
+      targets:
+        - name: Linux Will_Run
+          properties:
+            is-tot: "false"
+        - name: Linux Will_Not_Run
+    ''', engine: '');
+
+    // Add a commit and two tasks.
+    // The first task will have updated properties.
+    // The second task will be skipped because it doesn't exist in ToT.
+    firestore.putDocument(
+      generateFirestoreCommit(
+        0,
+        branch: 'master',
+        createTimestamp: fakeNow.millisecondsSinceEpoch,
+      ),
+    );
+    firestore.putDocument(
+      generateFirestoreTask(0, name: 'Linux Will_Run', commitSha: '0'),
+    );
+    firestore.putDocument(
+      generateFirestoreTask(1, name: 'Linux Will_Not_Run', commitSha: '0'),
+    );
+
+    // Run the backfiller.
+    await tester.get(handler);
+
+    // Lookup the scheduled tasks.
+    final tasks = fakeLuciBuildService.scheduledPostsubmitBuilds;
+    expect(tasks, [
+      isA<PendingTask>()
+          .having((t) => t.taskName, 'name', 'Linux Will_Run')
+          .having(
+            (t) => t.target.getProperties(),
+            'target.getProperties()',
+            containsPair('is-tot', isFalse),
+          ),
+    ]);
+  });
 }
 
 /// A very hermetic but dumb backfilling algorithm.