Skip post-submit task status update when build hasn't started (#2958)

Fixes: flutter/flutter#131419 1) Does not change task status when build hasn't started 2) Updates datastore only when task status changes 3) Adds tests
flutter · Jul 27, 2023 · edebd76 · edebd76
1 parent ee77264
commit edebd76
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 7 deletions.
diff --git a/app_dart/lib/src/model/appengine/task.dart b/app_dart/lib/src/model/appengine/task.dart
@@ -439,9 +439,6 @@ class Task extends Model<int> {
 
     if (build.status == Status.started) {
       return status = statusInProgress;
-    } else if (build.status == Status.scheduled) {
-      // Check for scheduled.
-      return status = statusNew;
     }
 
     switch (build.result) {
@@ -454,7 +451,8 @@ class Task extends Model<int> {
       case Result.failure:
         return status = statusFailed;
       default:
-        throw BadRequestException('${build.result} is unknown');
+        log.warning('keep task status as build result is ${build.result}.');
+        return status;
     }
   }
 

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,9 +80,15 @@ class PostsubmitLuciSubscription extends SubscriptionHandler {
     }
     log.fine('Found $task');
 
+    final String oldTaskStatus = task.status;
     task.updateFromBuild(build);
-    await datastore.insert(<Task>[task]);
-    log.fine('Updated datastore');
+    if (task.status != oldTaskStatus) {
+      await datastore.insert(<Task>[task]);
+      log.fine('Updated task status from $oldTaskStatus to ${task.status}');
+    } else {
+      log.fine('Skip updating task as status remains unchanged: ${task.status}');
+      return Body.empty;
+    }
 
     final Commit commit = await datastore.lookupByValue<Commit>(commitKey);
     final CiYaml ciYaml = await scheduler.getCiYaml(commit);

diff --git a/app_dart/test/model/task_test.dart b/app_dart/test/model/task_test.dart
@@ -179,6 +179,23 @@ void main() {
         task.updateFromBuild(build);
         expect(task.status, Task.statusCancelled);
       });
+
+      test('does not update when build result is null', () {
+        final pm.Build build = generatePushMessageBuild(
+          1,
+          status: pm.Status.scheduled,
+          result: null,
+        );
+        final Task task = generateTask(
+          1,
+          buildNumber: 1,
+          status: Task.statusNew,
+        );
+
+        expect(task.status, Task.statusNew);
+        task.updateFromBuild(build);
+        expect(task.status, Task.statusNew);
+      });
     });
   });
 

diff --git a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart
@@ -110,6 +110,29 @@ void main() {
     expect(task.endTimestamp, 1565049193786);
   });
 
+  test('keep task status when build is with null result', () async {
+    final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822');
+    final Task task = generateTask(
+      4507531199512576,
+      name: 'Linux A',
+      parent: commit,
+      status: Task.statusNew,
+    );
+    config.db.values[task.key] = task;
+    config.db.values[commit.key] = commit;
+    tester.message = createBuildbucketPushMessage(
+      'SCHEDULED',
+      builderName: 'Linux A',
+      result: null,
+      userData: '{\\"task_key\\":\\"${task.key.id}\\", \\"commit_key\\":\\"${task.key.parent?.id}\\"}',
+    );
+
+    expect(task.status, Task.statusNew);
+    expect(task.attempts, 1);
+    expect(await tester.post(handler), Body.empty);
+    expect(task.status, Task.statusNew);
+  });
+
   test('does not fail on empty user data', () async {
     tester.message = createBuildbucketPushMessage(
       'COMPLETED',

diff --git a/app_dart/test/src/utilities/entity_generators.dart b/app_dart/test/src/utilities/entity_generators.dart
@@ -157,7 +157,7 @@ push_message.Build generatePushMessageBuild(
   String bucket = 'prod',
   String name = 'Linux test_builder',
   push_message.Status? status = push_message.Status.completed,
-  push_message.Result result = push_message.Result.success,
+  push_message.Result? result = push_message.Result.success,
   List<String>? tags,
   int buildNumber = 1,
   DateTime? completedTimestamp,