Restore auto rerun on failures (#2970)

Follow up of #2964, which exit early also for the failed case. It causes auto-retry skipped. This PR reorg the logics and restores the rerun logic back for the failed case.
flutter · Aug 1, 2023 · d2cfc6d · d2cfc6d
1 parent 24f2a77
commit d2cfc6d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 19 deletions.
diff --git a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart
@@ -80,21 +80,14 @@ class PostsubmitLuciSubscription extends SubscriptionHandler {
     }
     log.fine('Found $task');
 
-    // No need to process if
-    // 1) the build is `scheduled`. Task is marked as `In Progress`
-    //    whenever scheduled, either from scheduler/backfiller/rerun. We need to update
-    //    task in datastore only for
-    //    a) `started`: update info like builder number.
-    //    b) `completed`: update info like status.
-    // 2) the task is already complemeted.
-    if (build.status == Status.scheduled || Task.finishedStatusValues.contains(task.status)) {
+    if (_shouldUpdateTask(build, task)) {
+      final String oldTaskStatus = task.status;
+      task.updateFromBuild(build);
+      await datastore.insert(<Task>[task]);
+      log.fine('Updated datastore from $oldTaskStatus to ${task.status}');
+    } else {
       log.fine('skip processing for build with status scheduled or task with status finished.');
-      return Body.empty;
     }
-    final String oldTaskStatus = task.status;
-    task.updateFromBuild(build);
-    await datastore.insert(<Task>[task]);
-    log.fine('Updated datastore from $oldTaskStatus to ${task.status}');
 
     final Commit commit = await datastore.lookupByValue<Commit>(commitKey);
     final CiYaml ciYaml = await scheduler.getCiYaml(commit);
@@ -124,4 +117,16 @@ class PostsubmitLuciSubscription extends SubscriptionHandler {
 
     return Body.empty;
   }
+
+  // No need to update task in datastore if
+  // 1) the build is `scheduled`. Task is marked as `In Progress`
+  //    whenever scheduled, either from scheduler/backfiller/rerun. We need to update
+  //    task in datastore only for
+  //    a) `started`: update info like builder number.
+  //    b) `completed`: update info like status.
+  // 2) the task is already completed.
+  //    The task may have been marked as completed from test framework via update-task-status API.
+  bool _shouldUpdateTask(Build build, Task task) {
+    return build.status != Status.scheduled && !Task.finishedStatusValues.contains(task.status);
+  }
 }
diff --git a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart
@@ -172,7 +172,7 @@ void main() {
       4507531199512576,
       name: 'Linux A',
       parent: commit,
-      status: Task.statusNew,
+      status: Task.statusFailed,
     );
     config.db.values[task.key] = task;
     config.db.values[commit.key] = commit;
@@ -183,7 +183,7 @@ void main() {
       userData: '{\\"task_key\\":\\"${task.key.id}\\", \\"commit_key\\":\\"${task.key.parent?.id}\\"}',
     );
 
-    expect(task.status, Task.statusNew);
+    expect(task.status, Task.statusFailed);
     expect(task.attempts, 1);
     expect(await tester.post(handler), Body.empty);
     expect(task.status, Task.statusInProgress);
@@ -196,7 +196,7 @@ void main() {
       4507531199512576,
       name: 'Linux A',
       parent: commit,
-      status: Task.statusNew,
+      status: Task.statusInfraFailure,
     );
     config.db.values[task.key] = task;
     config.db.values[commit.key] = commit;
@@ -207,7 +207,7 @@ void main() {
       userData: '{\\"task_key\\":\\"${task.key.id}\\", \\"commit_key\\":\\"${task.key.parent?.id}\\"}',
     );
 
-    expect(task.status, Task.statusNew);
+    expect(task.status, Task.statusInfraFailure);
     expect(task.attempts, 1);
     expect(await tester.post(handler), Body.empty);
     expect(task.status, Task.statusInProgress);
@@ -220,7 +220,7 @@ void main() {
       4507531199512576,
       name: 'Linux A',
       parent: commit,
-      status: Task.statusNew,
+      status: Task.statusInfraFailure,
     );
     config.db.values[task.key] = task;
     config.db.values[commit.key] = commit;
@@ -231,7 +231,7 @@ void main() {
       userData: '{\\"task_key\\":\\"${task.key.id}\\", \\"commit_key\\":\\"${task.key.parent?.id}\\"}',
     );
 
-    expect(task.status, Task.statusNew);
+    expect(task.status, Task.statusInfraFailure);
     expect(task.attempts, 1);
     expect(await tester.post(handler), Body.empty);
     expect(task.status, Task.statusInProgress);