Skip to content

Commit

Permalink
Calculate tree build status based on Firestore task statuses (#3575)
Browse files Browse the repository at this point in the history
Part of flutter/flutter#142951

This PR:
1) keeps existing function `retrieveCommitStatus` and creates a new `retrieveCommitStatusFirestore` for `get_build_status` API. This is to avoid affecting other APIs.
2) keeps existing class `CommitStatus` and creates a new `CommitTasksStatus` to support firestore case, preparing to deprecate the `stage` field from the old Task status.
3) maps to the firestore commit/task fields from old Datastore models and updates all related tests.
  • Loading branch information
keyonghan committed Mar 18, 2024
1 parent 2477167 commit 02192ce
Show file tree
Hide file tree
Showing 4 changed files with 230 additions and 192 deletions.
96 changes: 61 additions & 35 deletions app_dart/lib/src/service/build_status_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import 'package:cocoon_service/cocoon_service.dart';
import 'package:github/github.dart';
import 'package:meta/meta.dart';

import '../model/appengine/commit.dart';
import '../model/appengine/commit.dart' as datastore;
import '../model/appengine/github_build_status_update.dart';
import '../model/appengine/stage.dart';
import '../model/appengine/task.dart';
import '../model/firestore/commit.dart';
import '../model/firestore/task.dart';
import 'datastore.dart';

/// Function signature for a [BuildStatusService] provider.
Expand Down Expand Up @@ -64,7 +65,7 @@ class BuildStatusService {
///
/// Tree status is only for [defaultBranches].
Future<BuildStatus?> calculateCumulativeStatus(RepositorySlug slug) async {
final List<CommitStatus> statuses = await retrieveCommitStatus(
final List<CommitTasksStatus> statuses = await retrieveCommitStatusFirestore(
limit: numberOfCommitsToReferenceForTreeStatus,
slug: slug,
).toList();
Expand All @@ -78,30 +79,28 @@ class BuildStatusService {
}

final List<String> failedTasks = <String>[];
for (CommitStatus status in statuses) {
for (Stage stage in status.stages) {
for (Task task in stage.tasks) {
/// If a task [isRelevantToLatestStatus] but has not run yet, we look
/// for a previous run of the task from the previous commit.
final bool isRelevantToLatestStatus = tasksInProgress.containsKey(task.name);

/// Tasks that are not relevant to the latest status will have a
/// null value in the map.
final bool taskInProgress = tasksInProgress[task.name] ?? true;

if (isRelevantToLatestStatus && taskInProgress) {
if (task.isFlaky! || _isSuccessful(task)) {
/// This task no longer needs to be checked to see if it causing
/// the build status to fail.
tasksInProgress[task.name!] = false;
} else if (_isFailed(task) || _isRerunning(task)) {
failedTasks.add(task.name!);

/// This task no longer needs to be checked to see if its causing
/// the build status to fail since its been
/// added to the failedTasks list.
tasksInProgress[task.name!] = false;
}
for (CommitTasksStatus status in statuses) {
for (Task task in status.tasks) {
/// If a task [isRelevantToLatestStatus] but has not run yet, we look
/// for a previous run of the task from the previous commit.
final bool isRelevantToLatestStatus = tasksInProgress.containsKey(task.taskName);

/// Tasks that are not relevant to the latest status will have a
/// null value in the map.
final bool taskInProgress = tasksInProgress[task.taskName] ?? true;

if (isRelevantToLatestStatus && taskInProgress) {
if (task.bringup! || _isSuccessful(task)) {
/// This task no longer needs to be checked to see if it causing
/// the build status to fail.
tasksInProgress[task.taskName!] = false;
} else if (_isFailed(task) || _isRerunning(task)) {
failedTasks.add(task.taskName!);

/// This task no longer needs to be checked to see if its causing
/// the build status to fail since its been
/// added to the failedTasks list.
tasksInProgress[task.taskName!] = false;
}
}
}
Expand All @@ -112,29 +111,44 @@ class BuildStatusService {
/// Creates a map of the tasks that need to be checked for the build status.
///
/// This is based on the most recent [CommitStatus] and all of its tasks.
Map<String, bool> _findTasksRelevantToLatestStatus(List<CommitStatus> statuses) {
Map<String, bool> _findTasksRelevantToLatestStatus(List<CommitTasksStatus> statuses) {
final Map<String, bool> tasks = <String, bool>{};

for (Stage stage in statuses.first.stages) {
for (Task task in stage.tasks) {
tasks[task.name!] = true;
}
for (Task task in statuses.first.tasks) {
tasks[task.taskName!] = true;
}

return tasks;
}

/// Retrieves the comprehensive status of every task that runs per commit.
///
/// The returned stream will be ordered by most recent commit first, then
/// the next newest, and so on.
Stream<CommitTasksStatus> retrieveCommitStatusFirestore({
required int limit,
int? timestamp,
String? branch,
required RepositorySlug slug,
}) async* {
final List<Commit> commits = await firestoreService.queryRecentCommits(
limit: limit,
timestamp: timestamp,
branch: branch,
slug: slug,
);
for (Commit commit in commits) {
final List<Task> tasks = await firestoreService.queryCommitTasks(commit.sha!);
yield CommitTasksStatus(commit, tasks);
}
}

Stream<CommitStatus> retrieveCommitStatus({
required int limit,
int? timestamp,
String? branch,
required RepositorySlug slug,
}) async* {
await for (Commit commit in datastoreService.queryRecentCommits(
await for (datastore.Commit commit in datastoreService.queryRecentCommits(
limit: limit,
timestamp: timestamp,
branch: branch,
Expand Down Expand Up @@ -165,13 +179,25 @@ class BuildStatusService {
///
/// Tasks may still be running, and thus their status is subject to change.
/// Put another way, this class holds information that is a snapshot in time.
@immutable
class CommitTasksStatus {
/// Creates a new [CommitTasksStatus].
const CommitTasksStatus(this.commit, this.tasks);

/// The commit against which all the tasks are run.
final Commit commit;

/// Tasks running against the commit.
final List<Task> tasks;
}

@immutable
class CommitStatus {
/// Creates a new [CommitStatus].
const CommitStatus(this.commit, this.stages);

/// The commit against which all the tasks in [stages] are run.
final Commit commit;
final datastore.Commit commit;

/// The partitioned stages, each of which holds a bucket of tasks that
/// belong in the stage.
Expand Down
5 changes: 4 additions & 1 deletion app_dart/lib/src/service/firestore.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ class FirestoreService {
final Map<String, Object> filterMap = <String, Object>{
'$kTaskCommitShaField =': commitSha,
};
final List<Document> documents = await query(kTaskCollectionId, filterMap);
final Map<String, String> orderMap = <String, String>{
kTaskCreateTimestampField: kQueryOrderDescending,
};
final List<Document> documents = await query(kTaskCollectionId, filterMap, orderMap: orderMap);
return documents.map((Document document) => Task.fromDocument(taskDocument: document)).toList();
}

Expand Down
Loading

0 comments on commit 02192ce

Please sign in to comment.