diff --git a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart index a237d5030..0a1e96e9e 100644 --- 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]); + 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]); - log.fine('Updated datastore from $oldTaskStatus to ${task.status}'); final Commit commit = await datastore.lookupByValue(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 index 3aa50e167..70d5793cc 100644 --- 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);