Skip to content

Stop using remaining_jobs and failed_jobs and use jobs statuses instead #5165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ievdokdm wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Stop using remaining_jobs and failed_jobs and use jobs statuses instead #5165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42df832
Remove remaining_jobs and failed_jobs from presubmit_guards
ievdokdm Oct 8, 2026
e87fdc4
fix data seeder
ievdokdm Oct 8, 2026
cf33486
format
ievdokdm Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 2 additions & 28 deletions app_dart/lib/src/model/firestore/presubmit_guard.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ library;

import 'dart:convert';

import 'package:cocoon_common/guard_status.dart';
import 'package:cocoon_common/task_status.dart';
import 'package:github/github.dart';
import 'package:googleapis/firestore/v1.dart' hide Status;
Expand Down Expand Up @@ -57,8 +58,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
static const fieldHeadSha = 'head_sha';
static const fieldAuthor = 'author';
static const fieldCreationTime = 'creation_time';
static const fieldRemainingJobs = 'remaining_jobs';
static const fieldFailedJobs = 'failed_jobs';
static const fieldJobs = 'jobs';

static AppDocumentId<PresubmitGuard> documentIdFor({
Expand Down Expand Up @@ -92,15 +91,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
return '$kDocumentParent/$collectionId/${docId.documentId}';
}

/// Returns the document ID for the given parameters.
// static String documentId({
// required RepositorySlug slug,
// required int prNum,
// required int checkRunId,
// required CiStage stage,
// }) =>
// '${slug.owner}_${slug.name}_${prNum}_${checkRunId}_${stage.name}';

@override
AppDocumentMetadata<PresubmitGuard> get runtimeMetadata => metadata;

Expand All @@ -117,7 +107,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
required String headSha,
required int creationTime,
required String author,
required int jobCount,
CheckRun? checkRunGuard,
}) {
return PresubmitGuard(
Expand All @@ -128,8 +117,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
stage: stage,
author: author,
creationTime: creationTime,
remainingJobs: jobCount,
failedJobs: 0,
checkRunGuard: checkRunGuard,
);
}
Expand All @@ -146,8 +133,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
required CiStage stage,
required int creationTime,
required String author,
required int remainingJobs,
required int failedJobs,
CheckRun? checkRunGuard,
Map<String, TaskStatus>? jobs,
}) {
Expand All @@ -161,8 +146,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
fieldCreationTime: creationTime.toValue(),
fieldAuthor: author.toValue(),
fieldCheckRun: json.encode(checkRun.toJson()).toValue(),
fieldRemainingJobs: remainingJobs.toValue(),
fieldFailedJobs: failedJobs.toValue(),
if (checkRunGuard != null)
fieldCheckRunGuard: json.encode(checkRunGuard.toJson()).toValue(),
if (jobs != null)
Expand All @@ -189,13 +172,12 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
String get commitSha => fields[fieldHeadSha]!.stringValue!;
String get author => fields[fieldAuthor]!.stringValue!;
int get creationTime => int.parse(fields[fieldCreationTime]!.integerValue!);
int get remainingJobs => int.parse(fields[fieldRemainingJobs]!.integerValue!);
int get failedJobs => int.parse(fields[fieldFailedJobs]!.integerValue!);
Map<String, TaskStatus> get jobs =>
fields[fieldJobs]?.mapValue?.fields?.map<String, TaskStatus>(
(k, v) => MapEntry(k, TaskStatus.from(v.stringValue!)),
) ??
<String, TaskStatus>{};
GuardStatus get status => GuardStatus.calculate(jobs.values);
CheckRun get checkRun {
final jsonData =
jsonDecode(fields[fieldCheckRun]!.stringValue!) as Map<String, Object?>;
Expand Down Expand Up @@ -270,14 +252,6 @@ final class PresubmitGuard extends AppDocument<PresubmitGuard> {
if (value.isFailure) key,
];

set remainingJobs(int remainingJobs) {
fields[fieldRemainingJobs] = remainingJobs.toValue();
}

set failedJobs(int failedJobs) {
fields[fieldFailedJobs] = failedJobs.toValue();
}

set jobs(Map<String, TaskStatus> jobs) {
fields[fieldJobs] = Value(
mapValue: MapValue(
Expand Down
63 changes: 30 additions & 33 deletions app_dart/lib/src/request_handlers/get_engine_artifacts_ready.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:cocoon_common/guard_status.dart';

import '../../cocoon_service.dart';
import '../model/firestore/ci_staging.dart';
import '../request_handling/exceptions.dart';
Expand Down Expand Up @@ -39,9 +41,6 @@ final class GetEngineArtifactsReady extends PublicApiRequestHandler {
throw const BadRequestException('Missing query parameter: "$_paramSha"');
}

var failed = 0;
var remaining = 0;
var found = false;
final ciStaging = await CiStaging.fromFirestoreOrNull(
firestoreService: _firestore,
documentName: CiStaging.documentNameFor(
Expand All @@ -51,43 +50,41 @@ final class GetEngineArtifactsReady extends PublicApiRequestHandler {
),
);
if (ciStaging != null) {
failed = ciStaging.failed;
remaining = ciStaging.remaining;
found = true;
}
// if not found in ciStaging document, check if there are any
// presubmit guards for this sha. This is needed for unified check-run flow.
if (!found) {
final guards = await UnifiedCheckRun.getPresubmitGuardsForCommitSha(
firestoreService: _firestore,
slug: Config.flutterSlug,
commitSha: commitSha,
);
if (guards.isNotEmpty) {
found = true;
// If Guard exists for `fusion` only consider that stage is successful
// since empty guard is not stored for `engine` like it is in ciStaging.
final engineGuard = guards
.where((g) => g.stage == CiStage.fusionEngineBuild)
.firstOrNull;
remaining = engineGuard?.remainingJobs ?? 0;
failed = engineGuard?.failedJobs ?? 0;
if (ciStaging.failed > 0) {
return Response.json(_GetEngineArtifactsResponse.failed);
}
}

if (!found) {
throw NotFoundException('No engine SHA found for "$commitSha"');
}
if (ciStaging.remaining > 0) {
return Response.json(_GetEngineArtifactsResponse.pending);
}

if (failed > 0) {
return Response.json(_GetEngineArtifactsResponse.failed);
return Response.json(_GetEngineArtifactsResponse.complete);
}

if (remaining > 0) {
return Response.json(_GetEngineArtifactsResponse.pending);
// if not found in ciStaging document, check if there are any
// presubmit guards for this sha. This is needed for unified check-run flow.
final guards = await UnifiedCheckRun.getPresubmitGuardsForCommitSha(
firestoreService: _firestore,
slug: Config.flutterSlug,
commitSha: commitSha,
);
if (guards.isNotEmpty) {
// If Guard exists for `fusion` only consider that stage is successful
// since empty guard is not stored for `engine` like it is in ciStaging.
final engineGuard = guards
.where((g) => g.stage == CiStage.fusionEngineBuild)
.firstOrNull;
return switch (engineGuard?.status) {
null || GuardStatus.succeeded => Response.json(
_GetEngineArtifactsResponse.complete,
),
GuardStatus.failed => Response.json(_GetEngineArtifactsResponse.failed),
GuardStatus.inProgress || GuardStatus.waitingForBackfill =>
Response.json(_GetEngineArtifactsResponse.pending),
};
}

return Response.json(_GetEngineArtifactsResponse.complete);
throw NotFoundException('No engine SHA found for "$commitSha"');
}
}

Expand Down
64 changes: 9 additions & 55 deletions app_dart/lib/src/request_handlers/get_presubmit_guard.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,8 @@ final class GetPresubmitGuard extends PublicApiRequestHandler {
// Consolidate metadata from the first record.
final first = guards.first;

var totalFailed = 0;
var totalRemaining = 0;
var totalBuilds = 0;
for (final g in guards) {
totalFailed += g.failedJobs;
totalRemaining += g.remainingJobs;
totalBuilds += g.jobs.length;
}

final guardStatus = GuardStatus.calculate(
failedBuilds: totalFailed,
remainingBuilds: totalRemaining,
totalBuilds: totalBuilds,
guards.expand((g) => g.jobs.values),
);

final response = rpc_model.PresubmitGuardResponse(
Expand Down Expand Up @@ -162,58 +151,23 @@ final class GetPresubmitGuard extends PublicApiRequestHandler {
}, statusCode: HttpStatus.notFound);
}

var totalFailed = 0;
var totalRemaining = 0;
var totalBuilds = 0;
for (final stage in ciStagings) {
totalFailed += stage.failed;
totalRemaining += stage.remaining;
totalBuilds += stage.total;
}

/// Sort oldest first using a Schwartzian Transform with records to parse createTimestamp exactly once.
final tasksWithTime = [
for (final t in tasks) (task: t, time: t.createTimestamp),
]..sort((a, b) => a.time.compareTo(b.time));
final sortedTasks = [for (final entry in tasksWithTime) entry.task];

final nonBringupTasks = <String, TaskStatus>{};
final taskStatusMap = <String, TaskStatus>{};
for (final t in sortedTasks) {
final taskName = t.taskName;
final oldStatus = taskStatusMap[taskName];
final newStatus = t.status;

taskStatusMap[taskName] = newStatus;

// Do not count bringup towards success/failure.
if (t.bringup) continue;
if (oldStatus == null) {
totalBuilds++;
if (newStatus.isFailure) {
totalFailed++;
} else if (newStatus.isBuildInProgress) {
totalRemaining++;
}
} else {
// Adjust failed builds count.
switch ((oldStatus.isFailure, newStatus.isFailure)) {
case (true, false):
totalFailed--;
case (false, true):
totalFailed++;
case _:
break;
}

// Adjust remaining builds count.
switch ((oldStatus.isBuildInProgress, newStatus.isBuildInProgress)) {
case (true, false):
totalRemaining--;
case (false, true):
totalRemaining++;
case _:
break;
}
if (!t.bringup) {
nonBringupTasks[taskName] = newStatus;
}
}

Expand Down Expand Up @@ -243,11 +197,11 @@ final class GetPresubmitGuard extends PublicApiRequestHandler {
),
];

final guardStatus = GuardStatus.calculate(
failedBuilds: totalFailed,
remainingBuilds: totalRemaining,
totalBuilds: totalBuilds,
);
final guardStatus = GuardStatus.calculate([
for (final ciStage in ciStagings)
...ciStage.checkRuns.values.map(ChecksExtension.fromTaskConclusion),
...nonBringupTasks.values,
]);

var checkRunId = -1;
if (ciStagings.isNotEmpty) {
Expand Down
16 changes: 1 addition & 15 deletions app_dart/lib/src/request_handlers/get_presubmit_guard_summaries.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,6 @@ final class GetPresubmitGuardSummaries extends PublicApiRequestHandler {
final sha = entry.key;
final shaGuards = entry.value;

final totalFailed = shaGuards.fold<int>(
0,
(int sum, PresubmitGuard g) => sum + g.failedJobs,
);
final totalRemaining = shaGuards.fold<int>(
0,
(int sum, PresubmitGuard g) => sum + g.remainingJobs,
);
final totalBuilds = shaGuards.fold<int>(
0,
(int sum, PresubmitGuard g) => sum + g.jobs.length,
);
final earliestCreationTime = shaGuards.fold<int>(
// assuming creation time is always in the past :)
DateTime.now().millisecondsSinceEpoch,
Expand All @@ -117,9 +105,7 @@ final class GetPresubmitGuardSummaries extends PublicApiRequestHandler {
headSha: sha,
creationTime: earliestCreationTime,
guardStatus: GuardStatus.calculate(
failedBuilds: totalFailed,
remainingBuilds: totalRemaining,
totalBuilds: totalBuilds,
shaGuards.expand((g) => g.jobs.values),
),
),
);
Expand Down
Loading
Loading