diff --git a/app_dart/lib/src/model/common/checks_extension.dart b/app_dart/lib/src/model/common/checks_extension.dart index 671e8763c..d6e2d7622 100644 --- a/app_dart/lib/src/model/common/checks_extension.dart +++ b/app_dart/lib/src/model/common/checks_extension.dart @@ -25,7 +25,7 @@ extension ChecksExtension on TaskStatus { TaskStatus.failed || TaskStatus.infraFailure => 'failure', TaskStatus.cancelled => 'cancelled', TaskStatus.skipped => 'skipped', - _ => 'neutral', + _ => '', }; } @@ -37,7 +37,7 @@ extension ChecksExtension on TaskStatus { TaskStatus.infraFailure => CheckRunConclusion.failure, TaskStatus.cancelled => CheckRunConclusion.cancelled, TaskStatus.skipped => CheckRunConclusion.skipped, - _ => CheckRunConclusion.neutral, + _ => CheckRunConclusion.empty, }; } diff --git a/app_dart/lib/src/model/common/presubmit_check_state.dart b/app_dart/lib/src/model/common/presubmit_check_state.dart index 64da23b7d..a058e524d 100644 --- a/app_dart/lib/src/model/common/presubmit_check_state.dart +++ b/app_dart/lib/src/model/common/presubmit_check_state.dart @@ -28,21 +28,6 @@ class PresubmitCheckState { this.endTime, this.summary, }); - - /// Returns true if the build is waiting for backfill or in progress. - bool get isBuildInProgress => - status == TaskStatus.waitingForBackfill || - status == TaskStatus.inProgress; - - /// Returns true if the build succeeded or was skipped. - bool get isBuildSuccessed => - status == TaskStatus.succeeded || status == TaskStatus.skipped; - - /// Returns true if the build failed, had an infra failure, or was cancelled. - bool get isBuildFailed => - status == TaskStatus.failed || - status == TaskStatus.infraFailure || - status == TaskStatus.cancelled; } extension BuildToPresubmitCheckState on bbv2.Build { diff --git a/app_dart/lib/src/service/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart index c432139c5..f8b797ddb 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -274,7 +274,7 @@ final class UnifiedCheckRun { // If build is in progress, we should only update appropriate checks with // that [TaskStatus] - if (state.isBuildInProgress) { + if (state.status.isBuildInProgress) { presubmitCheck.startTime = state.startTime!; } else { // "remaining" should go down if buildSuccessed of any attempt @@ -285,8 +285,8 @@ final class UnifiedCheckRun { // So if the test existed and either remaining or failed_count is changed; // the response is valid. - if (state.isBuildSuccessed || - state.attemptNumber == 1 && state.isBuildFailed) { + if (state.status.isBuildSuccessed || + state.attemptNumber == 1 && state.status.isBuildFailed) { // Guard against going negative and log enough info so we can debug. if (remaining == 0) { throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; @@ -297,7 +297,7 @@ final class UnifiedCheckRun { // Only rollback the "failed" counter if this is a successful test run, // i.e. the test failed, the user requested a rerun, and now it passes. - if (state.attemptNumber > 1 && state.isBuildSuccessed) { + if (state.attemptNumber > 1 && state.status.isBuildSuccessed) { log.info( '$logCrumb: conclusion flipped to positive - assuming test was re-run', ); @@ -309,7 +309,7 @@ final class UnifiedCheckRun { } // Only increment the "failed" counter if the conclusion failed for the first attempt. - if (state.attemptNumber == 1 && state.isBuildFailed) { + if (state.attemptNumber == 1 && state.status.isBuildFailed) { log.info('$logCrumb: test failed'); valid = true; failed = failed + 1; diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 303c367df..4a0771ebd 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -968,6 +968,11 @@ $s state: check.state, ); } else { + // for github flow check runs are processed only if the build succeeded or + // some kind of failure occurred. + if (!check.status.isBuildCompleted) { + return true; + } // Check runs are fired at every stage. However, at this point it is unknown // if this check run belongs in the engine build stage or in the test stage. // So first look for it in the engine stage, and if it's missing, look for diff --git a/packages/cocoon_common/lib/task_status.dart b/packages/cocoon_common/lib/task_status.dart index 9ec28f9a2..5ea506938 100644 --- a/packages/cocoon_common/lib/task_status.dart +++ b/packages/cocoon_common/lib/task_status.dart @@ -66,6 +66,27 @@ enum TaskStatus { /// Whether the status represents a running state. bool get isRunning => this == inProgress; + /// Returns true if the build is waiting for backfill or in progress. + bool get isBuildInProgress => + this == TaskStatus.waitingForBackfill || this == TaskStatus.inProgress; + + /// Returns true if the build succeeded or was skipped. + bool get isBuildSuccessed => + this == TaskStatus.succeeded || this == TaskStatus.skipped; + + /// Returns true if the build failed, had an infra failure, or was cancelled. + bool get isBuildFailed => + this == TaskStatus.failed || + this == TaskStatus.infraFailure || + this == TaskStatus.cancelled; + + /// Returns true if the build succeeded or some kind of failure occurred. + bool get isBuildCompleted => + this == TaskStatus.succeeded || + this == TaskStatus.failed || + this == TaskStatus.infraFailure || + this == TaskStatus.cancelled; + /// Returns the JSON representation of `this`. Object? toJson() => _schemaValue;